From c6f2d49ff8be6b42bba94e7dc96e0fff8b5ca580 Mon Sep 17 00:00:00 2001 From: Chenguang Wang Date: Wed, 9 Dec 2020 18:56:27 -0800 Subject: [PATCH 1/3] fix issue #78496 --- .../src/transform/early_otherwise_branch.rs | 28 +++++++++++++++++++ src/test/ui/mir/issue-78496.rs | 16 +++++++++++ 2 files changed, 44 insertions(+) create mode 100644 src/test/ui/mir/issue-78496.rs diff --git a/compiler/rustc_mir/src/transform/early_otherwise_branch.rs b/compiler/rustc_mir/src/transform/early_otherwise_branch.rs index f91477911a489..5829c52293530 100644 --- a/compiler/rustc_mir/src/transform/early_otherwise_branch.rs +++ b/compiler/rustc_mir/src/transform/early_otherwise_branch.rs @@ -283,6 +283,34 @@ impl<'a, 'tcx> Helper<'a, 'tcx> { return None; } + // when one place is the projection of the other, it's not safe to calculate their discriminant values sequentially. + // for example, this should not be optimized: + // + // ```rust + // enum E<'a> { Empty, Some(&'a E<'a>), } + // let Some(Some(_)) = e; + // ``` + // + // ```mir + // bb0: { + // _2 = discriminant(*_1) + // switchInt(move _2) -> [...] + // } + // bb1: { + // _3 = discriminant(*(((*_1) as Some).0: &E)) + // switchInt(move _3) -> [...] + // } + // ``` + let discr_place = discr_info.place_of_adt_discr_read; + let this_discr_place = this_bb_discr_info.place_of_adt_discr_read; + if discr_place.local == this_discr_place.local + && (discr_place.projection.starts_with(this_discr_place.projection) + || this_discr_place.projection.starts_with(discr_place.projection)) + { + trace!("NO: one target is the projection of another"); + return None; + } + // if we reach this point, the optimization applies, and we should be able to optimize this case // store the info that is needed to apply the optimization diff --git a/src/test/ui/mir/issue-78496.rs b/src/test/ui/mir/issue-78496.rs new file mode 100644 index 0000000000000..cc45945a2b8d5 --- /dev/null +++ b/src/test/ui/mir/issue-78496.rs @@ -0,0 +1,16 @@ +// run-pass +// compile-flags: -Z mir-opt-level=2 -C opt-level=0 + +// example from #68867 +pub enum E<'a> { + Empty, + Some(&'a E<'a>), +} + +fn f(e: &E) -> u32 { + if let E::Some(E::Some(_)) = e { 1 } else { 2 } +} + +fn main() { + assert_eq!(f(&E::Empty), 2); +} From 78c0680b3febef5aaa2330d8b0a7879fd44e9eba Mon Sep 17 00:00:00 2001 From: Chenguang Wang Date: Wed, 9 Dec 2020 19:50:11 -0800 Subject: [PATCH 2/3] update comments --- .../rustc_mir/src/transform/early_otherwise_branch.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_mir/src/transform/early_otherwise_branch.rs b/compiler/rustc_mir/src/transform/early_otherwise_branch.rs index 5829c52293530..f28a4b4671571 100644 --- a/compiler/rustc_mir/src/transform/early_otherwise_branch.rs +++ b/compiler/rustc_mir/src/transform/early_otherwise_branch.rs @@ -283,7 +283,7 @@ impl<'a, 'tcx> Helper<'a, 'tcx> { return None; } - // when one place is the projection of the other, it's not safe to calculate their discriminant values sequentially. + // when the second place is a projection of the first one, it's not safe to calculate their discriminant values sequentially. // for example, this should not be optimized: // // ```rust @@ -294,18 +294,17 @@ impl<'a, 'tcx> Helper<'a, 'tcx> { // ```mir // bb0: { // _2 = discriminant(*_1) - // switchInt(move _2) -> [...] + // switchInt(_2) -> [...] // } // bb1: { // _3 = discriminant(*(((*_1) as Some).0: &E)) - // switchInt(move _3) -> [...] + // switchInt(_3) -> [...] // } // ``` let discr_place = discr_info.place_of_adt_discr_read; let this_discr_place = this_bb_discr_info.place_of_adt_discr_read; if discr_place.local == this_discr_place.local - && (discr_place.projection.starts_with(this_discr_place.projection) - || this_discr_place.projection.starts_with(discr_place.projection)) + && this_discr_place.projection.starts_with(discr_place.projection) { trace!("NO: one target is the projection of another"); return None; From 3812f70355132b53092e826c9d1a753dfd8a1874 Mon Sep 17 00:00:00 2001 From: Chenguang Wang Date: Wed, 9 Dec 2020 20:11:32 -0800 Subject: [PATCH 3/3] fix test case issue ref --- src/test/ui/mir/issue-78496.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/ui/mir/issue-78496.rs b/src/test/ui/mir/issue-78496.rs index cc45945a2b8d5..1b0687cfac3f6 100644 --- a/src/test/ui/mir/issue-78496.rs +++ b/src/test/ui/mir/issue-78496.rs @@ -1,7 +1,7 @@ // run-pass // compile-flags: -Z mir-opt-level=2 -C opt-level=0 -// example from #68867 +// example from #78496 pub enum E<'a> { Empty, Some(&'a E<'a>),