Skip to content

Commit a611f8d

Browse files
authored
Rollup merge of #79882 - wecing:master, r=oli-obk
Fix issue #78496 EarlyOtherwiseBranch finds MIR structures like: ``` bb0: { ... _2 = discriminant(X) ... switchInt(_2) -> [1_isize: bb1, otherwise: bb3] } bb1: { ... _3 = discriminant(Y) ... switchInt(_3) -> [1_isize: bb2, otherwise: bb3] } bb2: {...} bb3: {...} ``` And transforms them into something like: ``` bb0: { ... _2 = discriminant(X) _3 = discriminant(Y) _4 = Eq(_2, _3) switchInt(_4) -> [true: bb4, otherwise: bb3] } bb2: {...} // unchanged bb3: {...} // unchanged bb4: { switchInt(_2) -> [1_isize: bb2, otherwise: bb3] } ``` But that is not always a safe thing to do -- sometimes the early `otherwise` branch is necessary so the later block could assume the value of `discriminant(X)`. I am not totally sure what's the best way to detect that, but fixing #78496 should be easy -- we just check if `X` is a sub-expression of `Y`. A more precise test might be to check if `Y` contains a `Downcast(1)` of `X`, but I think this might be good enough. Fix #78496
2 parents 3d42c00 + 3812f70 commit a611f8d

File tree

2 files changed

+43
-0
lines changed

2 files changed

+43
-0
lines changed

compiler/rustc_mir/src/transform/early_otherwise_branch.rs

+27
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,33 @@ impl<'a, 'tcx> Helper<'a, 'tcx> {
284284
return None;
285285
}
286286

287+
// when the second place is a projection of the first one, it's not safe to calculate their discriminant values sequentially.
288+
// for example, this should not be optimized:
289+
//
290+
// ```rust
291+
// enum E<'a> { Empty, Some(&'a E<'a>), }
292+
// let Some(Some(_)) = e;
293+
// ```
294+
//
295+
// ```mir
296+
// bb0: {
297+
// _2 = discriminant(*_1)
298+
// switchInt(_2) -> [...]
299+
// }
300+
// bb1: {
301+
// _3 = discriminant(*(((*_1) as Some).0: &E))
302+
// switchInt(_3) -> [...]
303+
// }
304+
// ```
305+
let discr_place = discr_info.place_of_adt_discr_read;
306+
let this_discr_place = this_bb_discr_info.place_of_adt_discr_read;
307+
if discr_place.local == this_discr_place.local
308+
&& this_discr_place.projection.starts_with(discr_place.projection)
309+
{
310+
trace!("NO: one target is the projection of another");
311+
return None;
312+
}
313+
287314
// if we reach this point, the optimization applies, and we should be able to optimize this case
288315
// store the info that is needed to apply the optimization
289316

src/test/ui/mir/issue-78496.rs

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// run-pass
2+
// compile-flags: -Z mir-opt-level=2 -C opt-level=0
3+
4+
// example from #78496
5+
pub enum E<'a> {
6+
Empty,
7+
Some(&'a E<'a>),
8+
}
9+
10+
fn f(e: &E) -> u32 {
11+
if let E::Some(E::Some(_)) = e { 1 } else { 2 }
12+
}
13+
14+
fn main() {
15+
assert_eq!(f(&E::Empty), 2);
16+
}

0 commit comments

Comments
 (0)