Extract Spaces refactor / cleanup #7470
Conversation
| if matches!(loc_ann.extract_spaces().item, TypeAnnotation::Wildcard) | ||
| && matches!(ext_problem_kind, ExtensionTypeKind::TagUnion) | ||
| if matches!( | ||
| loc_ann.extract_spaces(env.arena).item, |
There was a problem hiding this comment.
This should probably be changed to use loc_ann.without_spaces(), since we don't care about the spaces part. Ditto for other cases where we discard the spaces. This way we can avoid unnecessary allocation.
I'm ok doing that in this PR or in a separate PR.
| @@ -21,6 +21,7 @@ pub struct Buf<'a> { | |||
| beginning_of_line: bool, | |||
| line_indent: u16, | |||
| flags: MigrationFlags, | |||
| pub arena: &'a Bump, | |||
There was a problem hiding this comment.
You actually don't need this - you can just access self.text.bump()
There was a problem hiding this comment.
I had ownership issue using that....
| match self { | ||
| $t::SpaceBefore(item, before) => { | ||
| match item { |
There was a problem hiding this comment.
Rather than doing a strict 2-level match here, we should change to doing a recursive call, similar to what happens in expr_lift_spaces (and similar functions) from the formatter.
There was a problem hiding this comment.
Is this not recursive? We call extract spaces on the item on line 2354
| AbilityImpls::SpaceAfter(AbilityImpls::AbilityImpls(inner), after) => Spaces { | ||
| before, | ||
| item: *inner, | ||
| after, | ||
| }, | ||
| AbilityImpls::SpaceAfter(_, _) => todo!(), | ||
| sb @ AbilityImpls::SpaceBefore(_, _) => { |
There was a problem hiding this comment.
Is there a way to use impl_extract_spaces! to handle this type?
Have extract_spaces method of ExtractSpaces trait take an arena so that: 1. We can remove a lot of `todo!()`s. 2. Recursively collapse several layers of SpaceBefore/SpaceAfter into a single Spaces. 3. Avoid some fuzzer panics (but not all). Rename new snapshot
gamebox
force-pushed
the
extract-spaces-refactor
branch
from
006c17b to
8dea8a2
Compare
|
@joshuawarner32 This still has one failure - one that i introduce because I was worried about |
Have extract_spaces method of ExtractSpaces trait take an arena so that:
todo!()s.