fix: Crash when computing diagnostics with MIR and error types#22707
fix: Crash when computing diagnostics with MIR and error types#22707Wilfred wants to merge 2 commits into
Conversation
rustbot
added
the
S-waiting-on-review
|
This has been the second most common LSP request panic I've seen recently (after #22705). |
Wilfred
force-pushed
the
mir_crash_projection
branch
from
34d8146 to
319ddaa
Compare
This comment has been minimized.
This comment has been minimized.
|
OK, I've pushed the validation into MIR lowering, does this make sense? I've also split the PR into two commits, this is arguably two distinct fixes. |
| let place_ty = cond_place.ty(&self.result, &self.infcx, self.env).ty; | ||
| let mut ocx = ObligationCtxt::new(&self.infcx); | ||
| let normalized_ty = ocx | ||
| .structurally_normalize_ty(&ObligationCause::dummy(), self.env, place_ty) |
There was a problem hiding this comment.
I don't see why we should normalize here. Inference already deeply normalizes at the end, and we already fail lowering if the type isn't a tuple. Do you have a test case where we fail only without normalization?
There was a problem hiding this comment.
Yep, the test on this commit fails prior to this change:
thread 'tests::mir_alias_projection_recovery_does_not_panic' (58661203) panicked at crates/hir-ty/src/mir.rs:1277:22:
can't project out of Alias(AliasTy { args: [#0], kind: Projection { def_id: TypeAliasId("A") }, .. })
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
There was a problem hiding this comment.
This is a projection on an alias, something we cannot normalize before monomorphization. It's not normalization that solves this, and it's not the lack of it that causes the problem. The root of the problem is mismatch between the place type and the pattern type.
The code has a type error; we should not try to lower MIR for such code, but unfortunately that's something we do today. Still, no normalization is needed: just fetching the place type is enough.
Projection assumes the tuple is big enough for the field access, so return a MIR lowering error earlier if the tuple isn't valid. AI disclosure: Some Fable 5 and GPT 5.5 usage.
Ensure we normalise the type first, so projection always gets a valid tuple. Previously we panicked with:
can't project out of Alias(AliasTy { kind: Projection { def_id: TypeAliasId("A") }, .. })
AI disclosure: Some Fable 5 and GPT 5.5 usage.
Wilfred
force-pushed
the
mir_crash_projection
branch
from
319ddaa to
4632e79
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. |
3 participants
Previously MIR would panic if we tried to access out-of-bounds tuple elements or fields in a value with an error type. This would produce an error of the form:
Instead, handle error types gracefully, and add unit tests.
AI disclosure: Some Fable 5 and GPT 5.5 usage, any errors are of course mine :)