internal: Remove mutable syntax tree usages from add_missing_match_arms assist
#19155
Conversation
rustbot
added
the
S-waiting-on-review
| Test::A => ${1:todo!()}, | ||
| Test::B => ${2:todo!()}, | ||
| Test::C => ${3:todo!()},$0 | ||
| }); | ||
| }"#, |
There was a problem hiding this comment.
The indentation got a bit better
| } | ||
| } | ||
| } | ||
|
|
||
| impl ast::LetStmt { |
There was a problem hiding this comment.
These impls utilizes mutable syntax trees and as we are not using these anywhere after this PR, they should be removed, I think
| @@ -1377,6 +1375,7 @@ fn main() { | |||
| ); | |||
| } | |||
|
|
|||
| #[ignore = "FIXME"] | |||
There was a problem hiding this comment.
I just ignored this and the following test temporarily to discuss the details.
Question: this test and the following test contain comments inside match arm lists and without the in-place style mutable syntax tree things it's very hard to preserve them, as we are building new match arm lists rather than editing in-place.
Maybe we can implement some logic to collect comments or whitespace tokens inside the match arms and feed them into match arm list building, but that might be very flacky and verbose 🤔
Should I tread that path or seek other alternatives?
edit: Oh, I forgot that I can insert AST nodes 😅 Nevermind.
There was a problem hiding this comment.
This would be "trivially" fixed once we swap to the new trivia model, so if we regress in in that regard its fine once we fix that when we switch to the new trivia model.
There was a problem hiding this comment.
I've tried inserting new arms in place instead of replacing the whole match arm list but indentations, newlines and commas aren't works well in this case, and making them work in every cases seems quite verbose and flacky.
So, I'm adding some comments addressing this regression
|
BTW as many of the interesting features/issues are currently blocked on salsa migration and next-gen solver, I'll mainly focus on migrating assists for a while. |
ShoyuVanilla
force-pushed
the
migrate-missing-match-arms
branch
from
7166a8e to
7d74f2b
Compare
Part of #18285
#18459 has removed some of mutable syntax tree usages but there were some leftovers