Fix debugger stepping behavior around match expressions

Previously, we would set up the source lines for `match` expressions so that the code generated to perform the test of the scrutinee was matched to the line of the arm that required the test and then jump from the arm block to the "next" block was matched to all of the lines in the `match` expression. While that makes sense, it has the side effect of causing strange stepping behavior in debuggers. I've changed the source information so that all of the generated tests are sourced to `match {scrutinee}` and the jumps are sourced to the last line of the block they are inside. This resolves the weird stepping behavior in all debuggers and resolves some instances of "ambiguous symbol" errors in WinDbg preventing the user from setting breakpoints at `match` expressions.
Dylan-DPC-zz · Aug 25, 2021 · 0a42dfc · 0a42dfc
1 parent a992a11
commit 0a42dfc
Show file tree

Hide file tree

Showing 92 changed files with 533 additions and 482 deletions.
diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs
@@ -21,7 +21,7 @@ use rustc_middle::mir::*;
 use rustc_middle::thir::{self, *};
 use rustc_middle::ty::{self, CanonicalUserTypeAnnotation, Ty};
 use rustc_span::symbol::Symbol;
-use rustc_span::Span;
+use rustc_span::{BytePos, Pos, Span};
 use rustc_target::abi::VariantIdx;
 use smallvec::{smallvec, SmallVec};
 
@@ -143,8 +143,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         let mut candidates =
             arm_candidates.iter_mut().map(|(_, candidate)| candidate).collect::<Vec<_>>();
 
-        let fake_borrow_temps =
-            self.lower_match_tree(block, scrutinee_span, match_has_guard, &mut candidates);
+        let match_start_span = span.shrink_to_lo().to(scrutinee.span);
+
+        let fake_borrow_temps = self.lower_match_tree(
+            block,
+            scrutinee_span,
+            match_start_span,
+            match_has_guard,
+            &mut candidates,
+        );
 
         self.lower_match_arms(
             destination,
@@ -224,6 +231,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         &mut self,
         block: BasicBlock,
         scrutinee_span: Span,
+        match_start_span: Span,
         match_has_guard: bool,
         candidates: &mut [&mut Candidate<'pat, 'tcx>],
     ) -> Vec<(Place<'tcx>, Local)> {
@@ -236,7 +244,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
 
         // This will generate code to test scrutinee_place and
         // branch to the appropriate arm block
-        self.match_candidates(scrutinee_span, block, &mut otherwise, candidates, &mut fake_borrows);
+        self.match_candidates(
+            match_start_span,
+            scrutinee_span,
+            block,
+            &mut otherwise,
+            candidates,
+            &mut fake_borrows,
+        );
 
         if let Some(otherwise_block) = otherwise {
             // See the doc comment on `match_candidates` for why we may have an
@@ -339,8 +354,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         // all the arm blocks will rejoin here
         let end_block = self.cfg.start_new_block();
 
+        let end_brace = self.source_info(
+            outer_source_info.span.with_lo(outer_source_info.span.hi() - BytePos::from_usize(1)),
+        );
         for arm_block in arm_end_blocks {
-            self.cfg.goto(unpack!(arm_block), outer_source_info, end_block);
+            let block = &self.cfg.basic_blocks[arm_block.0];
+            let last_location = block.statements.last().map(|s| s.source_info);
+
+            self.cfg.goto(unpack!(arm_block), last_location.unwrap_or(end_brace), end_block);
         }
 
         self.source_scope = outer_source_info.scope;
@@ -533,8 +554,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         set_match_place: bool,
     ) -> BlockAnd<()> {
         let mut candidate = Candidate::new(initializer.clone(), &irrefutable_pat, false);
-        let fake_borrow_temps =
-            self.lower_match_tree(block, irrefutable_pat.span, false, &mut [&mut candidate]);
+        let fake_borrow_temps = self.lower_match_tree(
+            block,
+            irrefutable_pat.span,
+            irrefutable_pat.span,
+            false,
+            &mut [&mut candidate],
+        );
         // For matches and function arguments, the place that is being matched
         // can be set when creating the variables. But the place for
         // let PATTERN = ... might not even exist until we do the assignment.
@@ -993,6 +1019,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
     fn match_candidates<'pat>(
         &mut self,
         span: Span,
+        scrutinee_span: Span,
         start_block: BasicBlock,
         otherwise_block: &mut Option<BasicBlock>,
         candidates: &mut [&mut Candidate<'pat, 'tcx>],
@@ -1022,6 +1049,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 }
                 self.match_simplified_candidates(
                     span,
+                    scrutinee_span,
                     start_block,
                     otherwise_block,
                     &mut *new_candidates,
@@ -1030,6 +1058,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             } else {
                 self.match_simplified_candidates(
                     span,
+                    scrutinee_span,
                     start_block,
                     otherwise_block,
                     candidates,
@@ -1042,6 +1071,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
     fn match_simplified_candidates(
         &mut self,
         span: Span,
+        scrutinee_span: Span,
         start_block: BasicBlock,
         otherwise_block: &mut Option<BasicBlock>,
         candidates: &mut [&mut Candidate<'_, 'tcx>],
@@ -1087,6 +1117,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         // Test for the remaining candidates.
         self.test_candidates_with_or(
             span,
+            scrutinee_span,
             unmatched_candidates,
             block,
             otherwise_block,
@@ -1257,6 +1288,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
     fn test_candidates_with_or(
         &mut self,
         span: Span,
+        scrutinee_span: Span,
         candidates: &mut [&mut Candidate<'_, 'tcx>],
         block: BasicBlock,
         otherwise_block: &mut Option<BasicBlock>,
@@ -1269,7 +1301,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         match *first_candidate.match_pairs[0].pattern.kind {
             PatKind::Or { .. } => (),
             _ => {
-                self.test_candidates(span, candidates, block, otherwise_block, fake_borrows);
+                self.test_candidates(
+                    span,
+                    scrutinee_span,
+                    candidates,
+                    block,
+                    otherwise_block,
+                    fake_borrows,
+                );
                 return;
             }
         }
@@ -1302,6 +1341,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
 
         self.match_candidates(
             span,
+            scrutinee_span,
             remainder_start,
             otherwise_block,
             remaining_candidates,
@@ -1330,6 +1370,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             otherwise
         };
         self.match_candidates(
+            or_span,
             or_span,
             candidate.pre_binding_block.unwrap(),
             otherwise,
@@ -1497,6 +1538,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
     fn test_candidates<'pat, 'b, 'c>(
         &mut self,
         span: Span,
+        scrutinee_span: Span,
         mut candidates: &'b mut [&'c mut Candidate<'pat, 'tcx>],
         block: BasicBlock,
         otherwise_block: &mut Option<BasicBlock>,
@@ -1591,6 +1633,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                         let candidate_start = this.cfg.start_new_block();
                         this.match_candidates(
                             span,
+                            scrutinee_span,
                             candidate_start,
                             remainder_start,
                             &mut *candidates,
@@ -1607,6 +1650,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 let remainder_start = remainder_start.unwrap_or_else(|| this.cfg.start_new_block());
                 this.match_candidates(
                     span,
+                    scrutinee_span,
                     remainder_start,
                     otherwise_block,
                     candidates,
@@ -1617,7 +1661,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             target_blocks
         };
 
-        self.perform_test(block, match_place, &test, make_target_blocks);
+        self.perform_test(span, scrutinee_span, block, match_place, &test, make_target_blocks);
     }
 
     /// Determine the fake borrows that are needed from a set of places that
@@ -1713,6 +1757,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         let fake_borrow_temps = self.lower_match_tree(
             block,
             pat.span,
+            pat.span,
             false,
             &mut [&mut guard_candidate, &mut otherwise_candidate],
         );

diff --git a/compiler/rustc_mir_build/src/build/matches/test.rs b/compiler/rustc_mir_build/src/build/matches/test.rs
@@ -19,6 +19,7 @@ use rustc_middle::ty::util::IntTypeExt;
 use rustc_middle::ty::{self, adjustment::PointerCast, Ty, TyCtxt};
 use rustc_span::def_id::DefId;
 use rustc_span::symbol::{sym, Symbol};
+use rustc_span::Span;
 use rustc_target::abi::VariantIdx;
 
 use std::cmp::Ordering;
@@ -151,6 +152,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
 
     pub(super) fn perform_test(
         &mut self,
+        match_start_span: Span,
+        scrutinee_span: Span,
         block: BasicBlock,
         place_builder: PlaceBuilder<'tcx>,
         test: &Test<'tcx>,
@@ -206,10 +209,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 debug!("num_enum_variants: {}, variants: {:?}", num_enum_variants, variants);
                 let discr_ty = adt_def.repr.discr_type().to_ty(tcx);
                 let discr = self.temp(discr_ty, test.span);
-                self.cfg.push_assign(block, source_info, discr, Rvalue::Discriminant(place));
+                self.cfg.push_assign(
+                    block,
+                    self.source_info(scrutinee_span),
+                    discr,
+                    Rvalue::Discriminant(place),
+                );
                 self.cfg.terminate(
                     block,
-                    source_info,
+                    self.source_info(match_start_span),
                     TerminatorKind::SwitchInt {
                         discr: Operand::Move(discr),
                         switch_ty: discr_ty,
@@ -246,7 +254,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                         targets: switch_targets,
                     }
                 };
-                self.cfg.terminate(block, source_info, terminator);
+                self.cfg.terminate(block, self.source_info(match_start_span), terminator);
             }
 
             TestKind::Eq { value, ty } => {

diff --git a/src/test/mir-opt/76803_regression.encode.SimplifyBranchSame.diff b/src/test/mir-opt/76803_regression.encode.SimplifyBranchSame.diff
@@ -7,18 +7,18 @@
       let mut _2: isize;                   // in scope 0 at $DIR/76803_regression.rs:12:9: 12:16
 
       bb0: {
-          _2 = discriminant(_1);           // scope 0 at $DIR/76803_regression.rs:12:9: 12:16
-          switchInt(move _2) -> [0_isize: bb2, otherwise: bb1]; // scope 0 at $DIR/76803_regression.rs:12:9: 12:16
+          _2 = discriminant(_1);           // scope 0 at $DIR/76803_regression.rs:11:11: 11:12
+          switchInt(move _2) -> [0_isize: bb2, otherwise: bb1]; // scope 0 at $DIR/76803_regression.rs:11:5: 11:12
       }
 
       bb1: {
           _0 = move _1;                    // scope 0 at $DIR/76803_regression.rs:13:14: 13:15
-          goto -> bb3;                     // scope 0 at $DIR/76803_regression.rs:11:5: 14:6
+          goto -> bb3;                     // scope 0 at $DIR/76803_regression.rs:13:14: 13:15
       }
 
       bb2: {
           discriminant(_0) = 1;            // scope 0 at $DIR/76803_regression.rs:12:20: 12:27
-          goto -> bb3;                     // scope 0 at $DIR/76803_regression.rs:11:5: 14:6
+          goto -> bb3;                     // scope 0 at $DIR/76803_regression.rs:12:20: 12:27
       }
 
       bb3: {

diff --git a/src/test/mir-opt/const_goto.issue_77355_opt.ConstGoto.diff b/src/test/mir-opt/const_goto.issue_77355_opt.ConstGoto.diff
@@ -10,10 +10,10 @@
 
       bb0: {
 -         StorageLive(_2);                 // scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
--         _3 = discriminant(_1);           // scope 0 at $DIR/const_goto.rs:12:22: 12:28
--         switchInt(move _3) -> [1_isize: bb2, 2_isize: bb2, otherwise: bb1]; // scope 0 at $DIR/const_goto.rs:12:22: 12:28
-+         _2 = discriminant(_1);           // scope 0 at $DIR/const_goto.rs:12:22: 12:28
-+         switchInt(move _2) -> [1_isize: bb2, 2_isize: bb2, otherwise: bb1]; // scope 0 at $DIR/const_goto.rs:12:22: 12:28
+-         _3 = discriminant(_1);           // scope 0 at $DIR/const_goto.rs:12:17: 12:20
+-         switchInt(move _3) -> [1_isize: bb2, 2_isize: bb2, otherwise: bb1]; // scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
++         _2 = discriminant(_1);           // scope 0 at $DIR/const_goto.rs:12:17: 12:20
++         switchInt(move _2) -> [1_isize: bb2, 2_isize: bb2, otherwise: bb1]; // scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
       }
 
       bb1: {

diff --git a/src/test/mir-opt/const_goto_const_eval_fail.f.ConstGoto.diff b/src/test/mir-opt/const_goto_const_eval_fail.f.ConstGoto.diff
@@ -10,35 +10,35 @@
           StorageLive(_1);                 // scope 0 at $DIR/const_goto_const_eval_fail.rs:7:11: 12:6
           StorageLive(_2);                 // scope 0 at $DIR/const_goto_const_eval_fail.rs:8:15: 8:16
           _2 = const A;                    // scope 0 at $DIR/const_goto_const_eval_fail.rs:8:15: 8:16
-          switchInt(_2) -> [1_i32: bb2, 2_i32: bb2, 3_i32: bb2, otherwise: bb1]; // scope 0 at $DIR/const_goto_const_eval_fail.rs:9:13: 9:14
+          switchInt(_2) -> [1_i32: bb2, 2_i32: bb2, 3_i32: bb2, otherwise: bb1]; // scope 0 at $DIR/const_goto_const_eval_fail.rs:8:9: 8:16
       }
 
       bb1: {
           _1 = const true;                 // scope 0 at $DIR/const_goto_const_eval_fail.rs:10:18: 10:22
-          goto -> bb3;                     // scope 0 at $DIR/const_goto_const_eval_fail.rs:8:9: 11:10
+          goto -> bb3;                     // scope 0 at $DIR/const_goto_const_eval_fail.rs:10:18: 10:22
       }
 
       bb2: {
           _1 = const B;                    // scope 0 at $DIR/const_goto_const_eval_fail.rs:9:26: 9:27
--         goto -> bb3;                     // scope 0 at $DIR/const_goto_const_eval_fail.rs:8:9: 11:10
-+         switchInt(_1) -> [false: bb4, otherwise: bb3]; // scope 0 at $DIR/const_goto_const_eval_fail.rs:13:9: 13:14
+-         goto -> bb3;                     // scope 0 at $DIR/const_goto_const_eval_fail.rs:9:26: 9:27
++         switchInt(_1) -> [false: bb4, otherwise: bb3]; // scope 0 at $DIR/const_goto_const_eval_fail.rs:7:5: 12:6
       }
 
       bb3: {
--         switchInt(_1) -> [false: bb5, otherwise: bb4]; // scope 0 at $DIR/const_goto_const_eval_fail.rs:13:9: 13:14
+-         switchInt(_1) -> [false: bb5, otherwise: bb4]; // scope 0 at $DIR/const_goto_const_eval_fail.rs:7:5: 12:6
 -     }
 - 
 -     bb4: {
           _0 = const 2_u64;                // scope 0 at $DIR/const_goto_const_eval_fail.rs:14:17: 14:18
--         goto -> bb6;                     // scope 0 at $DIR/const_goto_const_eval_fail.rs:7:5: 15:6
-+         goto -> bb5;                     // scope 0 at $DIR/const_goto_const_eval_fail.rs:7:5: 15:6
+-         goto -> bb6;                     // scope 0 at $DIR/const_goto_const_eval_fail.rs:14:17: 14:18
++         goto -> bb5;                     // scope 0 at $DIR/const_goto_const_eval_fail.rs:14:17: 14:18
       }
 
 -     bb5: {
 +     bb4: {
           _0 = const 1_u64;                // scope 0 at $DIR/const_goto_const_eval_fail.rs:13:18: 13:19
--         goto -> bb6;                     // scope 0 at $DIR/const_goto_const_eval_fail.rs:7:5: 15:6
-+         goto -> bb5;                     // scope 0 at $DIR/const_goto_const_eval_fail.rs:7:5: 15:6
+-         goto -> bb6;                     // scope 0 at $DIR/const_goto_const_eval_fail.rs:13:18: 13:19
++         goto -> bb5;                     // scope 0 at $DIR/const_goto_const_eval_fail.rs:13:18: 13:19
       }
 
 -     bb6: {

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
@@ -29,7 +29,7 @@
       }
 
       bb2: {
-          switchInt(((_3 as Some).0: bool)) -> [false: bb1, otherwise: bb3]; // scope 0 at $DIR/discriminant.rs:11:26: 11:30
+          switchInt(((_3 as Some).0: bool)) -> [false: bb1, otherwise: bb3]; // scope 0 at $DIR/discriminant.rs:11:21: 11:31
       }
 
       bb3: {

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
@@ -29,7 +29,7 @@
       }
 
       bb2: {
-          switchInt(((_3 as Some).0: bool)) -> [false: bb1, otherwise: bb3]; // scope 0 at $DIR/discriminant.rs:11:26: 11:30
+          switchInt(((_3 as Some).0: bool)) -> [false: bb1, otherwise: bb3]; // scope 0 at $DIR/discriminant.rs:11:21: 11:31
       }
 
       bb3: {

diff --git a/src/test/mir-opt/const_prop/switch_int.main.ConstProp.diff b/src/test/mir-opt/const_prop/switch_int.main.ConstProp.diff
@@ -8,8 +8,8 @@
       bb0: {
           StorageLive(_1);                 // scope 0 at $DIR/switch_int.rs:7:11: 7:12
           _1 = const 1_i32;                // scope 0 at $DIR/switch_int.rs:7:11: 7:12
--         switchInt(_1) -> [1_i32: bb2, otherwise: bb1]; // scope 0 at $DIR/switch_int.rs:8:9: 8:10
-+         switchInt(const 1_i32) -> [1_i32: bb2, otherwise: bb1]; // scope 0 at $DIR/switch_int.rs:8:9: 8:10
+-         switchInt(_1) -> [1_i32: bb2, otherwise: bb1]; // scope 0 at $DIR/switch_int.rs:7:5: 7:12
++         switchInt(const 1_i32) -> [1_i32: bb2, otherwise: bb1]; // scope 0 at $DIR/switch_int.rs:7:5: 7:12
       }
 
       bb1: {

diff --git a/src/test/mir-opt/const_prop/switch_int.main.SimplifyBranches-after-const-prop.diff b/src/test/mir-opt/const_prop/switch_int.main.SimplifyBranches-after-const-prop.diff
@@ -8,8 +8,8 @@
       bb0: {
           StorageLive(_1);                 // scope 0 at $DIR/switch_int.rs:7:11: 7:12
           _1 = const 1_i32;                // scope 0 at $DIR/switch_int.rs:7:11: 7:12
--         switchInt(const 1_i32) -> [1_i32: bb2, otherwise: bb1]; // scope 0 at $DIR/switch_int.rs:8:9: 8:10
-+         goto -> bb2;                     // scope 0 at $DIR/switch_int.rs:8:9: 8:10
+-         switchInt(const 1_i32) -> [1_i32: bb2, otherwise: bb1]; // scope 0 at $DIR/switch_int.rs:7:5: 7:12
++         goto -> bb2;                     // scope 0 at $DIR/switch_int.rs:7:5: 7:12
       }
 
       bb1: {