Auto merge of rust-lang#101894 - dingxiangfei2009:let-else-avoid-duplicate-storage-live, r=oli-obk

Avoid duplicating StorageLive in let-else

cc `@est31`

Fix rust-lang#101867
Fix rust-lang#101932

rust-lang#101410 introduced directives to activate storages of bindings in let-else earlier. However, since it is using the machinery of `match` and friends for pattern matching and binding, those storages are activated for the second time. This PR adjusts this behavior and avoid the duplicated activation for let-else statements.

Author: bors

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!(

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<Span>,
         arm_scope: Option<region::Scope>,
         match_scope: Option<region::Scope>,
+        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<Span>,
         match_scope: Option<region::Scope>,
         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<Item = &'b Binding<'tcx>>,
+        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()

src/test/mir-opt/issue-101867.rs

@@ -0,0 +1,9 @@
+#![cfg_attr(bootstrap, feature(let_else))]
+
+// EMIT_MIR issue_101867.main.mir_map.0.mir
+fn main() {
+    let x: Option<u8> = Some(1);
+    let Some(y) = x else {
+        panic!();
+    };
+}

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<u8>) }, span: $DIR/issue-101867.rs:5:12: 5:22, inferred_ty: std::option::Option<u8>
+| 1: user_ty: Canonical { max_universe: U0, variables: [], value: Ty(std::option::Option<u8>) }, span: $DIR/issue-101867.rs:5:12: 5:22, inferred_ty: std::option::Option<u8>
+|
+fn main() -> () {
+    let mut _0: ();                      // return place in scope 0 at $DIR/issue-101867.rs:+0:11: +0:11
+    let _1: std::option::Option<u8> 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::<u8>::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(<ZST>) }
+                                         // 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
+    }
+}

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>) -> 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);
+}