Fix crashes in assists due to .unwrap() calls in SyntaxFactory - #22759
Conversation
rustbot
added
the
S-waiting-on-review
There was a problem hiding this comment.
I think this is the wrong way to guard agains this. The code rightfully assumes that the make module constructs well formed code if given well formed code. So I guess the issue we are running into now is the way the make module works, by stringifying and then re-parsing, which makes this entire assumption fairly brittle as different parser recovery states can kick in.
I wonder how far away we are from finally changing this now that we have removed the mutable APIs
|
I agree with @Veykril that we should not make |
d6bec93 to
9822056
Compare
This comment has been minimized.
This comment has been minimized.
9822056 to
4d014d3
Compare
We will, the only pre-requisite (AFAICT), for this gonna be moving trivia inside the tokens, and making our grammar generator smarter. Currently, working on removing mutable rowan API's. |
0f13176 to
918f5a6
Compare
|
OK, I've done another pass on this: using I think this small and sensible now, but let me know if you'd like other changes. |
|
yea that looks better to me :) |
|
There seems to be a similar PR: #21578 |
918f5a6 to
c17becc
Compare
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
c17becc to
6faf4c7
Compare
|
OK, done another pass on this. I've got one regression test, and just changed the logic to use As with #22782, I am seeing this cause crashes for a few users (8 people recently) even without invoking the assist, so I think it's worth porting to Let me know what you think. |
What about your second regression test? I mentioned it in #22759 (comment) |
These functions would panic when they try to re-parse code that isn't valid in a top level function. For parameters, this is `..`, which is valid in a closure but not a function (requires an annotation). For tail block expressions, this is `let var = expr`, which is valid in a tail position but not a function body (requires a semicolon). I've checked both quote! invocations against the rust.ungram file and I think the structures are correct. Include a test of an assist that previously caused a panic. AI disclosure: Written with a little help by GPT-5.5.
6faf4c7 to
0da74dd
Compare
|
Added the second regression test, sorry for the slow response. |
rustbot
removed
the
S-waiting-on-review
6 participants
These functions would panic when they try to re-parse code that isn't
valid in a top level function.
For parameters, this is
.., which is valid in a closure but not afunction (requires an annotation). For tail block expressions, this is
let var = expr, which is valid in a tail position but not a functionbody (requires a semicolon).
I've checked both quote! invocations against the rust.ungram file and
I think the structures are correct.
Include a test of an assist that previously caused a panic.
AI disclosure: Written with a little help by GPT-5.5.