Auto merge of #97931 - xldenis:fix-if-let-source-scopes, r=nagisa

Fix `SourceScope` for `if let` bindings. Fixes #97799. I'm not sure how to test this properly, is there any way to observe the difference in behavior apart from `ui` tests? I'm worried that they would be overlooked in the case of a regression.
rust-lang · Jun 20, 2022 · 9a0b774 · 9a0b774
2 parents 17c6bde + a434e4c
commit 9a0b774
Show file tree

Hide file tree

Showing 16 changed files with 330 additions and 177 deletions.
diff --git a/compiler/rustc_mir_build/src/build/expr/into.rs b/compiler/rustc_mir_build/src/build/expr/into.rs
@@ -63,15 +63,27 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                         (if_then_scope, then_source_info),
                         LintLevel::Inherited,
                         |this| {
+                            let source_info = if this.is_let(cond) {
+                                let variable_scope = this.new_source_scope(
+                                    then_expr.span,
+                                    LintLevel::Inherited,
+                                    None,
+                                );
+                                this.source_scope = variable_scope;
+                                SourceInfo { span: then_expr.span, scope: variable_scope }
+                            } else {
+                                this.source_info(then_expr.span)
+                            };
                             let (then_block, else_block) =
                                 this.in_if_then_scope(condition_scope, |this| {
                                     let then_blk = unpack!(this.then_else_break(
                                         block,
                                         &this.thir[cond],
                                         Some(condition_scope),
                                         condition_scope,
-                                        then_expr.span,
+                                        source_info
                                     ));
+
                                     this.expr_into_dest(destination, then_blk, then_expr)
                                 });
                             then_block.and(else_block)
@@ -97,7 +109,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             ExprKind::Let { expr, ref pat } => {
                 let scope = this.local_scope();
                 let (true_block, false_block) = this.in_if_then_scope(scope, |this| {
-                    this.lower_let_expr(block, &this.thir[expr], pat, scope, expr_span)
+                    this.lower_let_expr(block, &this.thir[expr], pat, scope, None, expr_span)
                 });
 
                 this.cfg.push_assign_constant(
@@ -575,4 +587,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
 
         block_and
     }
+
+    fn is_let(&self, expr: ExprId) -> bool {
+        match self.thir[expr].kind {
+            ExprKind::Let { .. } => true,
+            ExprKind::Scope { value, .. } => self.is_let(value),
+            _ => false,
+        }
+    }
 }
diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs
@@ -40,7 +40,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         expr: &Expr<'tcx>,
         temp_scope_override: Option<region::Scope>,
         break_scope: region::Scope,
-        variable_scope_span: Span,
+        variable_source_info: SourceInfo,
     ) -> BlockAnd<()> {
         let this = self;
         let expr_span = expr.span;
@@ -52,15 +52,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     &this.thir[lhs],
                     temp_scope_override,
                     break_scope,
-                    variable_scope_span,
+                    variable_source_info,
                 ));
 
                 let rhs_then_block = unpack!(this.then_else_break(
                     lhs_then_block,
                     &this.thir[rhs],
                     temp_scope_override,
                     break_scope,
-                    variable_scope_span,
+                    variable_source_info,
                 ));
 
                 rhs_then_block.unit()
@@ -73,13 +73,18 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                         &this.thir[value],
                         temp_scope_override,
                         break_scope,
-                        variable_scope_span,
+                        variable_source_info,
                     )
                 })
             }
-            ExprKind::Let { expr, ref pat } => {
-                this.lower_let_expr(block, &this.thir[expr], pat, break_scope, variable_scope_span)
-            }
+            ExprKind::Let { expr, ref pat } => this.lower_let_expr(
+                block,
+                &this.thir[expr],
+                pat,
+                break_scope,
+                Some(variable_source_info.scope),
+                variable_source_info.span,
+            ),
             _ => {
                 let temp_scope = temp_scope_override.unwrap_or_else(|| this.local_scope());
                 let mutability = Mutability::Mut;
@@ -1772,6 +1777,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         expr: &Expr<'tcx>,
         pat: &Pat<'tcx>,
         else_target: region::Scope,
+        source_scope: Option<SourceScope>,
         span: Span,
     ) -> BlockAnd<()> {
         let expr_span = expr.span;
@@ -1797,7 +1803,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         let otherwise_post_guard_block = otherwise_candidate.pre_binding_block.unwrap();
         self.break_for_else(otherwise_post_guard_block, else_target, self.source_info(expr_span));
 
-        self.declare_bindings(None, pat.span.to(span), pat, ArmHasGuard(false), opt_expr_place);
+        self.declare_bindings(
+            source_scope,
+            pat.span.to(span),
+            pat,
+            ArmHasGuard(false),
+            opt_expr_place,
+        );
+
         let post_guard_block = self.bind_pattern(
             self.source_info(pat.span),
             guard_candidate,
@@ -1969,12 +1982,18 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     Guard::If(e) => {
                         let e = &this.thir[e];
                         guard_span = e.span;
-                        this.then_else_break(block, e, None, match_scope, arm_span)
+                        this.then_else_break(
+                            block,
+                            e,
+                            None,
+                            match_scope,
+                            this.source_info(arm_span),
+                        )
                     }
                     Guard::IfLet(ref pat, scrutinee) => {
                         let s = &this.thir[scrutinee];
                         guard_span = s.span;
-                        this.lower_let_expr(block, s, pat, match_scope, arm_span)
+                        this.lower_let_expr(block, s, pat, match_scope, None, arm_span)
                     }
                 });
 

diff --git a/src/test/debuginfo/lexical-scope-in-if-let.rs b/src/test/debuginfo/lexical-scope-in-if-let.rs
@@ -0,0 +1,100 @@
+// compile-flags:-g
+
+// === GDB TESTS ==================================================================================
+
+// gdb-command:run
+// gdb-command:info locals
+// gdb-check:a = 123
+
+// gdb-command:continue
+// gdb-command:info locals
+// gdb-check:x = 42
+// gdb-check:a = 123
+
+// gdb-command:continue
+// gdb-command:info locals
+// gdb-check:y = true
+// gdb-check:b = 456
+// gdb-check:x = 42
+// gdb-check:a = 123
+
+// gdb-command:continue
+// gdb-command:info locals
+// gdb-check:z = 10
+// gdb-check:c = 789
+// gdb-check:y = true
+// gdb-check:b = 456
+// gdb-check:x = 42
+// gdb-check:a = 123
+
+// === LLDB TESTS =================================================================================
+
+// lldb-command:run
+// lldb-command:frame variable
+// lldb-check:(int) a = 123
+
+// lldb-command:continue
+// lldb-command:frame variable
+// lldb-check:(int) a = 123 (int) x = 42
+
+// lldb-command:continue
+// lldb-command:frame variable
+// lldb-check:(int) a = 123 (int) x = 42 (int) b = 456 (bool) y = true
+
+// lldb-command:continue
+// lldb-command:frame variable
+// lldb-check:(int) a = 123 (int) x = 42 (int) b = 456 (bool) y = true (int) c = 789 (int) z = 10
+
+// === CDB TESTS ==================================================================================
+
+// cdb-command: g
+// cdb-command: dv
+// cdb-check:[...]a = 0n123
+
+// cdb-command: g
+// cdb-command: dv
+// cdb-check:[...]a = 0n123
+// cdb-check:[...]x = 0n42
+
+// cdb-command: g
+// cdb-command: dv
+// cdb-check:[...]y = true
+// cdb-check:[...]b = 0n456
+// cdb-check:[...]a = 0n123
+// cdb-check:[...]x = 0n42
+
+// cdb-command: g
+// cdb-command: dv
+// cdb-check:[...]z = 0n10
+// cdb-check:[...]c = 0n789
+// cdb-check:[...]y = true
+// cdb-check:[...]b = 0n456
+// cdb-check:[...]a = 0n123
+// cdb-check:[...]x = 0n42
+
+fn main() {
+    let a = id(123);
+
+    zzz(); // #break
+
+    if let Some(x) = id(Some(42)) {
+        zzz(); // #break
+
+        let b = id(456);
+
+        if let Ok(y) = id::<Result<bool, ()>>(Ok(true)) {
+            zzz(); // #break
+
+            let c = id(789);
+
+            if let (z, 42) = id((10, 42)) {
+                zzz(); // #break
+            }
+        }
+    }
+}
+
+#[inline(never)]
+fn id<T>(value: T) -> T { value }
+
+fn zzz() { }
diff --git a/src/test/mir-opt/const_prop/discriminant.main.ConstProp.32bit.diff b/src/test/mir-opt/const_prop/discriminant.main.ConstProp.32bit.diff
@@ -10,26 +10,28 @@
       scope 1 {
           debug x => _1;                   // in scope 1 at $DIR/discriminant.rs:11:9: 11:10
       }
+      scope 2 {
+      }
 
       bb0: {
           StorageLive(_1);                 // scope 0 at $DIR/discriminant.rs:11:9: 11:10
           StorageLive(_2);                 // scope 0 at $DIR/discriminant.rs:11:13: 11:64
-          StorageLive(_3);                 // scope 0 at $DIR/discriminant.rs:11:34: 11:44
-          Deinit(_3);                      // scope 0 at $DIR/discriminant.rs:11:34: 11:44
-          ((_3 as Some).0: bool) = const true; // scope 0 at $DIR/discriminant.rs:11:34: 11:44
-          discriminant(_3) = 1;            // scope 0 at $DIR/discriminant.rs:11:34: 11:44
--         _4 = discriminant(_3);           // scope 0 at $DIR/discriminant.rs:11:21: 11:31
--         switchInt(move _4) -> [1_isize: bb1, otherwise: bb3]; // scope 0 at $DIR/discriminant.rs:11:21: 11:31
-+         _4 = const 1_isize;              // scope 0 at $DIR/discriminant.rs:11:21: 11:31
-+         switchInt(const 1_isize) -> [1_isize: bb1, otherwise: bb3]; // scope 0 at $DIR/discriminant.rs:11:21: 11:31
+          StorageLive(_3);                 // scope 2 at $DIR/discriminant.rs:11:34: 11:44
+          Deinit(_3);                      // scope 2 at $DIR/discriminant.rs:11:34: 11:44
+          ((_3 as Some).0: bool) = const true; // scope 2 at $DIR/discriminant.rs:11:34: 11:44
+          discriminant(_3) = 1;            // scope 2 at $DIR/discriminant.rs:11:34: 11:44
+-         _4 = discriminant(_3);           // scope 2 at $DIR/discriminant.rs:11:21: 11:31
+-         switchInt(move _4) -> [1_isize: bb1, otherwise: bb3]; // scope 2 at $DIR/discriminant.rs:11:21: 11:31
++         _4 = const 1_isize;              // scope 2 at $DIR/discriminant.rs:11:21: 11:31
++         switchInt(const 1_isize) -> [1_isize: bb1, otherwise: bb3]; // scope 2 at $DIR/discriminant.rs:11:21: 11:31
       }
 
       bb1: {
-          switchInt(((_3 as Some).0: bool)) -> [false: bb3, otherwise: bb2]; // scope 0 at $DIR/discriminant.rs:11:21: 11:31
+          switchInt(((_3 as Some).0: bool)) -> [false: bb3, otherwise: bb2]; // scope 2 at $DIR/discriminant.rs:11:21: 11:31
       }
 
       bb2: {
-          _2 = const 42_i32;               // scope 0 at $DIR/discriminant.rs:11:47: 11:49
+          _2 = const 42_i32;               // scope 2 at $DIR/discriminant.rs:11:47: 11:49
           goto -> bb4;                     // scope 0 at $DIR/discriminant.rs:11:13: 11:64
       }
 

diff --git a/src/test/mir-opt/const_prop/discriminant.main.ConstProp.64bit.diff b/src/test/mir-opt/const_prop/discriminant.main.ConstProp.64bit.diff
@@ -10,26 +10,28 @@
       scope 1 {
           debug x => _1;                   // in scope 1 at $DIR/discriminant.rs:11:9: 11:10
       }
+      scope 2 {
+      }
 
       bb0: {
           StorageLive(_1);                 // scope 0 at $DIR/discriminant.rs:11:9: 11:10
           StorageLive(_2);                 // scope 0 at $DIR/discriminant.rs:11:13: 11:64
-          StorageLive(_3);                 // scope 0 at $DIR/discriminant.rs:11:34: 11:44
-          Deinit(_3);                      // scope 0 at $DIR/discriminant.rs:11:34: 11:44
-          ((_3 as Some).0: bool) = const true; // scope 0 at $DIR/discriminant.rs:11:34: 11:44
-          discriminant(_3) = 1;            // scope 0 at $DIR/discriminant.rs:11:34: 11:44
--         _4 = discriminant(_3);           // scope 0 at $DIR/discriminant.rs:11:21: 11:31
--         switchInt(move _4) -> [1_isize: bb1, otherwise: bb3]; // scope 0 at $DIR/discriminant.rs:11:21: 11:31
-+         _4 = const 1_isize;              // scope 0 at $DIR/discriminant.rs:11:21: 11:31
-+         switchInt(const 1_isize) -> [1_isize: bb1, otherwise: bb3]; // scope 0 at $DIR/discriminant.rs:11:21: 11:31
+          StorageLive(_3);                 // scope 2 at $DIR/discriminant.rs:11:34: 11:44
+          Deinit(_3);                      // scope 2 at $DIR/discriminant.rs:11:34: 11:44
+          ((_3 as Some).0: bool) = const true; // scope 2 at $DIR/discriminant.rs:11:34: 11:44
+          discriminant(_3) = 1;            // scope 2 at $DIR/discriminant.rs:11:34: 11:44
+-         _4 = discriminant(_3);           // scope 2 at $DIR/discriminant.rs:11:21: 11:31
+-         switchInt(move _4) -> [1_isize: bb1, otherwise: bb3]; // scope 2 at $DIR/discriminant.rs:11:21: 11:31
++         _4 = const 1_isize;              // scope 2 at $DIR/discriminant.rs:11:21: 11:31
++         switchInt(const 1_isize) -> [1_isize: bb1, otherwise: bb3]; // scope 2 at $DIR/discriminant.rs:11:21: 11:31
       }
 
       bb1: {
-          switchInt(((_3 as Some).0: bool)) -> [false: bb3, otherwise: bb2]; // scope 0 at $DIR/discriminant.rs:11:21: 11:31
+          switchInt(((_3 as Some).0: bool)) -> [false: bb3, otherwise: bb2]; // scope 2 at $DIR/discriminant.rs:11:21: 11:31
       }
 
       bb2: {
-          _2 = const 42_i32;               // scope 0 at $DIR/discriminant.rs:11:47: 11:49
+          _2 = const 42_i32;               // scope 2 at $DIR/discriminant.rs:11:47: 11:49
           goto -> bb4;                     // scope 0 at $DIR/discriminant.rs:11:13: 11:64
       }
 

diff --git a/src/test/mir-opt/early_otherwise_branch_soundness.no_downcast.EarlyOtherwiseBranch.diff b/src/test/mir-opt/early_otherwise_branch_soundness.no_downcast.EarlyOtherwiseBranch.diff
@@ -7,22 +7,24 @@
       let mut _2: isize;                   // in scope 0 at $DIR/early_otherwise_branch_soundness.rs:13:20: 13:30
       let mut _3: isize;                   // in scope 0 at $DIR/early_otherwise_branch_soundness.rs:13:12: 13:31
       let mut _4: &E;                      // in scope 0 at $DIR/early_otherwise_branch_soundness.rs:12:16: 12:17
+      scope 1 {
+      }
 
       bb0: {
-          _3 = discriminant((*_1));        // scope 0 at $DIR/early_otherwise_branch_soundness.rs:13:12: 13:31
-          switchInt(move _3) -> [1_isize: bb1, otherwise: bb3]; // scope 0 at $DIR/early_otherwise_branch_soundness.rs:13:12: 13:31
+          _3 = discriminant((*_1));        // scope 1 at $DIR/early_otherwise_branch_soundness.rs:13:12: 13:31
+          switchInt(move _3) -> [1_isize: bb1, otherwise: bb3]; // scope 1 at $DIR/early_otherwise_branch_soundness.rs:13:12: 13:31
       }
 
       bb1: {
-          StorageLive(_4);                 // scope 0 at $DIR/early_otherwise_branch_soundness.rs:13:12: 13:31
-          _4 = move (((*_1) as Some).0: &E); // scope 0 at $DIR/early_otherwise_branch_soundness.rs:13:12: 13:31
-          _2 = discriminant((*_4));        // scope 0 at $DIR/early_otherwise_branch_soundness.rs:13:12: 13:31
-          StorageDead(_4);                 // scope 0 at $DIR/early_otherwise_branch_soundness.rs:13:12: 13:31
-          switchInt(move _2) -> [1_isize: bb2, otherwise: bb3]; // scope 0 at $DIR/early_otherwise_branch_soundness.rs:13:12: 13:31
+          StorageLive(_4);                 // scope 1 at $DIR/early_otherwise_branch_soundness.rs:13:12: 13:31
+          _4 = move (((*_1) as Some).0: &E); // scope 1 at $DIR/early_otherwise_branch_soundness.rs:13:12: 13:31
+          _2 = discriminant((*_4));        // scope 1 at $DIR/early_otherwise_branch_soundness.rs:13:12: 13:31
+          StorageDead(_4);                 // scope 1 at $DIR/early_otherwise_branch_soundness.rs:13:12: 13:31
+          switchInt(move _2) -> [1_isize: bb2, otherwise: bb3]; // scope 1 at $DIR/early_otherwise_branch_soundness.rs:13:12: 13:31
       }
 
       bb2: {
-          _0 = const 1_u32;                // scope 0 at $DIR/early_otherwise_branch_soundness.rs:13:38: 13:39
+          _0 = const 1_u32;                // scope 1 at $DIR/early_otherwise_branch_soundness.rs:13:38: 13:39
           goto -> bb4;                     // scope 0 at $DIR/early_otherwise_branch_soundness.rs:13:5: 13:52
       }