From 48c1c1d19043120e9e89b3186d5d5286142f3066 Mon Sep 17 00:00:00 2001 From: Ding Xiang Fei Date: Fri, 16 Sep 2022 19:02:45 +0800 Subject: [PATCH 1/3] avoid duplicating StorageLive in let-else --- compiler/rustc_mir_build/src/build/block.rs | 3 +- .../rustc_mir_build/src/build/matches/mod.rs | 39 +++++++++++++++---- 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/block.rs b/compiler/rustc_mir_build/src/build/block.rs index 7ab7870c4642f..4bab583c96018 100644 --- a/compiler/rustc_mir_build/src/build/block.rs +++ b/compiler/rustc_mir_build/src/build/block.rs @@ -232,7 +232,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { pattern, UserTypeProjections::none(), &mut |this, _, _, _, node, span, _, _| { - this.storage_live_binding(block, node, span, OutsideGuard, false); + this.storage_live_binding(block, node, span, OutsideGuard, true); + this.schedule_drop_for_binding(node, span, OutsideGuard); }, ); let failure = unpack!( diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index e0727725f68d5..82067ceebfd6d 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -371,6 +371,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { Some(arm.span), Some(arm.scope), Some(match_scope), + false, ); if let Some(source_scope) = scope { @@ -416,6 +417,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { arm_span: Option, arm_scope: Option, match_scope: Option, + storages_alive: bool, ) -> BasicBlock { if candidate.subcandidates.is_empty() { // Avoid generating another `BasicBlock` when we only have one @@ -429,6 +431,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { arm_span, match_scope, true, + storages_alive, ) } else { // It's helpful to avoid scheduling drops multiple times to save @@ -466,6 +469,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { arm_span, match_scope, schedule_drops, + storages_alive, ); if arm_scope.is_none() { schedule_drops = false; @@ -641,6 +645,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { None, None, None, + false, ) .unit() } @@ -1813,6 +1818,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { None, None, None, + false, ); post_guard_block.unit() @@ -1836,6 +1842,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { arm_span: Option, match_scope: Option, schedule_drops: bool, + storages_alive: bool, ) -> BasicBlock { debug!("bind_and_guard_matched_candidate(candidate={:?})", candidate); @@ -2051,7 +2058,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { self.cfg.push_fake_read(post_guard_block, guard_end, cause, Place::from(local_id)); } assert!(schedule_drops, "patterns with guards must schedule drops"); - self.bind_matched_candidate_for_arm_body(post_guard_block, true, by_value_bindings); + self.bind_matched_candidate_for_arm_body( + post_guard_block, + true, + by_value_bindings, + storages_alive, + ); post_guard_block } else { @@ -2065,6 +2077,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { .iter() .flat_map(|(bindings, _)| bindings) .chain(&candidate.bindings), + storages_alive, ); block } @@ -2154,6 +2167,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { block: BasicBlock, schedule_drops: bool, bindings: impl IntoIterator>, + storages_alive: bool, ) where 'tcx: 'b, { @@ -2163,13 +2177,20 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // Assign each of the bindings. This may trigger moves out of the candidate. for binding in bindings { let source_info = self.source_info(binding.span); - let local = self.storage_live_binding( - block, - binding.var_id, - binding.span, - OutsideGuard, - schedule_drops, - ); + let local = if storages_alive { + // Here storages are already alive, probably because this is a binding + // from let-else. + // We just need to schedule drop for the value. + self.var_local_id(binding.var_id, OutsideGuard).into() + } else { + self.storage_live_binding( + block, + binding.var_id, + binding.span, + OutsideGuard, + schedule_drops, + ) + }; if schedule_drops { self.schedule_drop_for_binding(binding.var_id, binding.span, OutsideGuard); } @@ -2300,6 +2321,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { None, None, None, + true, ); // This block is for the failure case let failure = this.bind_pattern( @@ -2311,6 +2333,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { None, None, None, + true, ); this.break_for_else(failure, *let_else_scope, this.source_info(initializer_span)); matching.unit() From d510ba3bc3b6d41e36cb40b64af65637de39ff8b Mon Sep 17 00:00:00 2001 From: Ding Xiang Fei Date: Fri, 16 Sep 2022 19:54:41 +0800 Subject: [PATCH 2/3] add mir-opt test --- src/test/mir-opt/issue-101867.rs | 9 +++ .../mir-opt/issue_101867.main.mir_map.0.mir | 75 +++++++++++++++++++ 2 files changed, 84 insertions(+) create mode 100644 src/test/mir-opt/issue-101867.rs create mode 100644 src/test/mir-opt/issue_101867.main.mir_map.0.mir diff --git a/src/test/mir-opt/issue-101867.rs b/src/test/mir-opt/issue-101867.rs new file mode 100644 index 0000000000000..98f9aa6ef40ac --- /dev/null +++ b/src/test/mir-opt/issue-101867.rs @@ -0,0 +1,9 @@ +#![feature(let_else)] + +// EMIT_MIR issue_101867.main.mir_map.0.mir +fn main() { + let x: Option = Some(1); + let Some(y) = x else { + panic!(); + }; +} diff --git a/src/test/mir-opt/issue_101867.main.mir_map.0.mir b/src/test/mir-opt/issue_101867.main.mir_map.0.mir new file mode 100644 index 0000000000000..98501ac8c9d84 --- /dev/null +++ b/src/test/mir-opt/issue_101867.main.mir_map.0.mir @@ -0,0 +1,75 @@ +// MIR for `main` 0 mir_map + +| User Type Annotations +| 0: user_ty: Canonical { max_universe: U0, variables: [], value: Ty(std::option::Option) }, span: $DIR/issue-101867.rs:5:12: 5:22, inferred_ty: std::option::Option +| 1: user_ty: Canonical { max_universe: U0, variables: [], value: Ty(std::option::Option) }, span: $DIR/issue-101867.rs:5:12: 5:22, inferred_ty: std::option::Option +| +fn main() -> () { + let mut _0: (); // return place in scope 0 at $DIR/issue-101867.rs:+0:11: +0:11 + let _1: std::option::Option as UserTypeProjection { base: UserType(0), projs: [] }; // in scope 0 at $DIR/issue-101867.rs:+1:9: +1:10 + let mut _2: !; // in scope 0 at $DIR/issue-101867.rs:+2:26: +4:6 + let _3: (); // in scope 0 at $SRC_DIR/std/src/panic.rs:LL:COL + let mut _4: !; // in scope 0 at $SRC_DIR/std/src/panic.rs:LL:COL + let mut _6: isize; // in scope 0 at $DIR/issue-101867.rs:+2:9: +2:16 + scope 1 { + debug x => _1; // in scope 1 at $DIR/issue-101867.rs:+1:9: +1:10 + let _5: u8; // in scope 1 at $DIR/issue-101867.rs:+2:14: +2:15 + scope 2 { + debug y => _5; // in scope 2 at $DIR/issue-101867.rs:+2:14: +2:15 + } + } + + bb0: { + StorageLive(_1); // scope 0 at $DIR/issue-101867.rs:+1:9: +1:10 + _1 = Option::::Some(const 1_u8); // scope 0 at $DIR/issue-101867.rs:+1:25: +1:32 + FakeRead(ForLet(None), _1); // scope 0 at $DIR/issue-101867.rs:+1:9: +1:10 + AscribeUserType(_1, o, UserTypeProjection { base: UserType(1), projs: [] }); // scope 0 at $DIR/issue-101867.rs:+1:12: +1:22 + StorageLive(_5); // scope 1 at $DIR/issue-101867.rs:+2:14: +2:15 + FakeRead(ForMatchedPlace(None), _1); // scope 1 at $DIR/issue-101867.rs:+2:19: +2:20 + _6 = discriminant(_1); // scope 1 at $DIR/issue-101867.rs:+2:19: +2:20 + switchInt(move _6) -> [1_isize: bb4, otherwise: bb3]; // scope 1 at $DIR/issue-101867.rs:+2:9: +2:16 + } + + bb1: { + StorageLive(_3); // scope 1 at $SRC_DIR/std/src/panic.rs:LL:COL + StorageLive(_4); // scope 1 at $SRC_DIR/std/src/panic.rs:LL:COL + _4 = begin_panic::<&str>(const "explicit panic") -> bb7; // scope 1 at $SRC_DIR/std/src/panic.rs:LL:COL + // mir::Constant + // + span: $SRC_DIR/std/src/panic.rs:LL:COL + // + literal: Const { ty: fn(&str) -> ! {begin_panic::<&str>}, val: Value() } + // mir::Constant + // + span: $SRC_DIR/std/src/panic.rs:LL:COL + // + literal: Const { ty: &str, val: Value(Slice(..)) } + } + + bb2: { + StorageDead(_4); // scope 1 at $SRC_DIR/std/src/panic.rs:LL:COL + StorageDead(_3); // scope 1 at $DIR/issue-101867.rs:+3:16: +3:17 + unreachable; // scope 1 at $DIR/issue-101867.rs:+2:26: +4:6 + } + + bb3: { + goto -> bb6; // scope 1 at $DIR/issue-101867.rs:+2:19: +2:20 + } + + bb4: { + falseEdge -> [real: bb5, imaginary: bb3]; // scope 1 at $DIR/issue-101867.rs:+2:9: +2:16 + } + + bb5: { + _5 = ((_1 as Some).0: u8); // scope 1 at $DIR/issue-101867.rs:+2:14: +2:15 + _0 = const (); // scope 0 at $DIR/issue-101867.rs:+0:11: +5:2 + StorageDead(_5); // scope 1 at $DIR/issue-101867.rs:+5:1: +5:2 + StorageDead(_1); // scope 0 at $DIR/issue-101867.rs:+5:1: +5:2 + return; // scope 0 at $DIR/issue-101867.rs:+5:2: +5:2 + } + + bb6: { + StorageDead(_5); // scope 1 at $DIR/issue-101867.rs:+5:1: +5:2 + goto -> bb1; // scope 0 at $DIR/issue-101867.rs:+0:11: +5:2 + } + + bb7 (cleanup): { + resume; // scope 0 at $DIR/issue-101867.rs:+0:1: +5:2 + } +} From eb36f5ee5b71cbe3eb356f8e56e9c9a69b6d649d Mon Sep 17 00:00:00 2001 From: Ding Xiang Fei Date: Sun, 18 Sep 2022 12:18:34 +0800 Subject: [PATCH 3/3] add miri test via const fn --- src/test/mir-opt/issue-101867.rs | 2 +- src/test/ui/let-else/const-fn.rs | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/let-else/const-fn.rs diff --git a/src/test/mir-opt/issue-101867.rs b/src/test/mir-opt/issue-101867.rs index 98f9aa6ef40ac..931396e2171f0 100644 --- a/src/test/mir-opt/issue-101867.rs +++ b/src/test/mir-opt/issue-101867.rs @@ -1,4 +1,4 @@ -#![feature(let_else)] +#![cfg_attr(bootstrap, feature(let_else))] // EMIT_MIR issue_101867.main.mir_map.0.mir fn main() { diff --git a/src/test/ui/let-else/const-fn.rs b/src/test/ui/let-else/const-fn.rs new file mode 100644 index 0000000000000..336b0b4b72ad5 --- /dev/null +++ b/src/test/ui/let-else/const-fn.rs @@ -0,0 +1,19 @@ +// run-pass +// issue #101932 + +#![cfg_attr(bootstrap, feature(let_else))] + +const fn foo(a: Option) -> i32 { + let Some(a) = a else { + return 42 + }; + + a + 1 +} + +fn main() { + const A: i32 = foo(None); + const B: i32 = foo(Some(1)); + + println!("{} {}", A, B); +}