internal compiler error: two identical projections #137467
Assignees
Labels
A-patterns
Relating to patterns and pattern matching
C-bug
Category: This is a bug.
I-ICE
Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️
P-medium
Medium priority
regression-from-stable-to-stable
Performance or correctness regression from one stable version to another.
S-bug-has-test
Status: This bug is tracked inside the repo by a `known-bug` test.
S-has-mcve
Status: A Minimal Complete and Verifiable Example has been found for this issue
T-compiler
Relevant to the compiler team, which will review and decide on the PR/issue.
Comments
HomelikeBrick42
added
C-bug
rustbot
added
the
needs-triage
|
Minimized: enum Camera {
Normal { base_transform: i32 },
Volume { transform: i32 },
}
fn draw_ui(camera: &mut Camera) {
|| {
let (Camera::Normal {
base_transform: _transform,
}
| Camera::Volume {
transform: _transform,
}) = camera;
};
}Error output (edition 2024)Error output (edition 2015) |
|
A variation with a slightly different ICE: enum Camera {
Normal { base_transform: i32 },
Volume { transform: i32 },
}
fn draw_ui(camera: &mut Camera) {
|| {
let (Camera::Normal {
base_transform: _,
}
| Camera::Volume {
transform: _,
}) = camera;
};
}Error output (edition 2024)Compiles without errors on edition 2015. |
|
This compiled fine in 1.55 (It causes ICEs of varying kinds starting from 1.56), so I guess.... @rustbot label +regression-from-stable-to-stable |
rustbot
added
regression-from-stable-to-stable
|
Note: the exact ICE appears to depend on the edition used, presumably due to the edition 2021 closure capturing changes. |
jieyouxu
added
S-has-mcve
|
@rustbot claim |
|
What seems to be happening here is that the or-pattern allows something that otherwise wouldn't be possible: an irrefutable pattern that matches on a particular variant of an enum. Because of this, the upvar inference attempts to capture only a particular field of an enum variant. The "two identical projections" assertion triggers because that part of the code doesn't expect enum variants, and so discards the This suggests the following interesting variation to the test case: enum Camera {
Normal { base_transform: i32 },
Volume { meow: i32, transform: i32 },
}
fn draw_ui(camera: &mut Camera) {
|| {
let (Camera::Normal {
base_transform: _transform,
}
| Camera::Volume {
transform: _transform,
..
}) = camera;
};
} |
|
Looks like I will need to modify // compiles on stable
enum Void {}
enum Test {
A(i32, i32),
B(Void),
}
fn florb(mut x: Test) {
let mut f = || {
let Test::A(ref mut i, _) = x;
*i += 2;
};
let mut g = || {
let Test::A(_, ref mut i) = x;
*i *= 2;
};
f();
g();
}
|
|
WG-prioritization assigning priority (Zulip discussion). @rustbot label -I-prioritize +P-medium |
rustbot
added
P-medium
|
I have established that fn meow(x: (u32, u32, u32)) {
let f = || {
let ((0, a, _) | (_, _, a)) = x;
};
} |
matthiaskrgr
added
the
S-bug-has-test
bors
added a commit
to rust-lang-ci/rust
that referenced
this issue
Mar 26, 2025
ExprUseVisitor: properly report discriminant reads This PR fixes rust-lang#137467. In order to do so, it needs to introduce a small breaking change surrounding the interaction of closure captures with matching against enums with uninhabited variants. Yes – to fix an ICE! ## Background The current upvar inference code handles patterns in two parts: - `ExprUseVisitor::walk_pat` finds the *bindings* being done by the pattern and captures the relevant parts - `ExprUseVisitor::maybe_read_scrutinee` determines whether matching against the pattern will at any point require inspecting a discriminant, and if so, captures *the entire scrutinee*. It also has some weird logic around bindings, deciding to also capture the entire scrutinee if *pretty much any binding exists in the pattern*, with some weird behavior like rust-lang#137553. Nevertheless, something like `|| let (a, _) = x;` will only capture `x.0`, because `maybe_read_scrutinee` does not run for irrefutable patterns at all. This causes issues like rust-lang#137467, where the closure wouldn't be capturing enough, because an irrefutable or-pattern can still require inspecting a discriminant, and the match lowering would then panic, because it couldn't find an appropriate upvar in the closure. My thesis is that this is not a reasonable implementation. To that end, I intend to merge the functionality of both these parts into `walk_pat`, which will bring upvar inference closer to what the MIR lowering actually needs – both in making sure that necessary variables get captured, fixing rust-lang#137467, and in reducing the cases where redundant variables do – fixing rust-lang#137553. This PR introduces the necessary logic into `walk_pat`, fixing rust-lang#137467. A subsequent PR will remove `maybe_read_scrutinee` entirely, which should now be redundant, fixing rust-lang#137553. The latter is still pending, as my current revision doesn't handle opaque types correctly for some reason I haven't looked into yet. ## The breaking change The following example, adapted from the testsuite, compiles on current stable, but will not compile with this PR: ```rust #[derive(Clone, Copy, PartialEq, Eq, Debug)] enum Void {} pub fn main() { let mut r = Result::<Void, (u32, u32)>::Err((0, 0)); let mut f = || { let Err((ref mut a, _)) = r; *a = 1; }; let mut g = || { //~^ ERROR: cannot borrow `r` as mutable more than once at a time let Err((_, ref mut b)) = r; *b = 2; }; f(); g(); assert_eq!(r, Err((1, 2))); } ``` The issue is that, to determine that matching against `Err` here doesn't require inspecting the discriminant, we need to query the `InhabitedPredicate` of the types involved. However, as upvar inference is done during typechecking, the relevant type might not yet be fully inferred. Because of this, performing such a check hits this assertion: https://github.com/rust-lang/rust/blob/43f0014ef0f242418674f49052ed39b70f73bc1c/compiler/rustc_middle/src/ty/inhabitedness/mod.rs#L121 The code used to compile fine, but only because the compiler incorrectly assumed that patterns used within a `let` cannot possibly be inspecting any discriminants. ## Is the breaking change necessary? One other option would be to double down, and introduce a deliberate semantics difference between `let $pat = $expr;` and `match $expr { $pat => ... }`, that syntactically determines whether the pattern is in an irrefutable position, instead of querying the types. **This would not eliminate the breaking change,** but it would limit it to more contrived examples, such as ```rust let ((true, Err((ref mut a, _, _))) | (false, Err((_, ref mut a, _)))) = x; ``` The cost here, would be the complexity added with very little benefit. ## Other notes - I performed various cleanups while working on this. The last commit of the PR is the interesting one. - Due to the temporary duplication of logic between `maybe_read_scrutinee` and `walk_pat`, some of the `#[rustc_capture_analysis]` tests report duplicate messages before deduplication. This is harmless.
bors
added a commit
to rust-lang-ci/rust
that referenced
this issue
Mar 26, 2025
ExprUseVisitor: murder maybe_read_scrutinee in cold blood This PR fixes rust-lang#137467. In order to do so, it needs to introduce a small breaking change surrounding the interaction of closure captures with matching against enums with uninhabited variants. Yes – to fix an ICE! ## Background The current upvar inference code handles patterns in two parts: - `ExprUseVisitor::walk_pat` finds the *bindings* being done by the pattern and captures the relevant parts - `ExprUseVisitor::maybe_read_scrutinee` determines whether matching against the pattern will at any point require inspecting a discriminant, and if so, captures *the entire scrutinee*. It also has some weird logic around bindings, deciding to also capture the entire scrutinee if *pretty much any binding exists in the pattern*, with some weird behavior like rust-lang#137553. Nevertheless, something like `|| let (a, _) = x;` will only capture `x.0`, because `maybe_read_scrutinee` does not run for irrefutable patterns at all. This causes issues like rust-lang#137467, where the closure wouldn't be capturing enough, because an irrefutable or-pattern can still require inspecting a discriminant, and the match lowering would then panic, because it couldn't find an appropriate upvar in the closure. My thesis is that this is not a reasonable implementation. To that end, I intend to merge the functionality of both these parts into `walk_pat`, which will bring upvar inference closer to what the MIR lowering actually needs – both in making sure that necessary variables get captured, fixing rust-lang#137467, and in reducing the cases where redundant variables do – fixing rust-lang#137553. This PR introduces the necessary logic into `walk_pat`, fixing rust-lang#137467. A subsequent PR will remove `maybe_read_scrutinee` entirely, which should now be redundant, fixing rust-lang#137553. The latter is still pending, as my current revision doesn't handle opaque types correctly for some reason I haven't looked into yet. ## The breaking change The following example, adapted from the testsuite, compiles on current stable, but will not compile with this PR: ```rust #[derive(Clone, Copy, PartialEq, Eq, Debug)] enum Void {} pub fn main() { let mut r = Result::<Void, (u32, u32)>::Err((0, 0)); let mut f = || { let Err((ref mut a, _)) = r; *a = 1; }; let mut g = || { //~^ ERROR: cannot borrow `r` as mutable more than once at a time let Err((_, ref mut b)) = r; *b = 2; }; f(); g(); assert_eq!(r, Err((1, 2))); } ``` The issue is that, to determine that matching against `Err` here doesn't require inspecting the discriminant, we need to query the `InhabitedPredicate` of the types involved. However, as upvar inference is done during typechecking, the relevant type might not yet be fully inferred. Because of this, performing such a check hits this assertion: https://github.com/rust-lang/rust/blob/43f0014ef0f242418674f49052ed39b70f73bc1c/compiler/rustc_middle/src/ty/inhabitedness/mod.rs#L121 The code used to compile fine, but only because the compiler incorrectly assumed that patterns used within a `let` cannot possibly be inspecting any discriminants. ## Is the breaking change necessary? One other option would be to double down, and introduce a deliberate semantics difference between `let $pat = $expr;` and `match $expr { $pat => ... }`, that syntactically determines whether the pattern is in an irrefutable position, instead of querying the types. **This would not eliminate the breaking change,** but it would limit it to more contrived examples, such as ```rust let ((true, Err((ref mut a, _, _))) | (false, Err((_, ref mut a, _)))) = x; ``` The cost here, would be the complexity added with very little benefit. ## Other notes - I performed various cleanups while working on this. The last commit of the PR is the interesting one. - Due to the temporary duplication of logic between `maybe_read_scrutinee` and `walk_pat`, some of the `#[rustc_capture_analysis]` tests report duplicate messages before deduplication. This is harmless.
This was referenced Mar 27, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Code
I dont really want to make a MRE right now, so this is the entire project
https://github.com/HomelikeBrick42/4dStuff/tree/rustc_bug
Meta
rustc --version --verbose:Error output
The text was updated successfully, but these errors were encountered: