Unrolled build for rust-lang#121784

Rollup merge of rust-lang#121784 - Zalathar:if-or-converge, r=Nadrieril Make the success arms of `if lhs || rhs` meet up in a separate block Extracted from rust-lang#118305, where this is necessary to avoid introducing a bug when injecting marker statements into the then/else arms. --- In the previous code (rust-lang#111752), the success block of `lhs` would jump directly to the success block of `rhs`. However, `rhs_success_block` could already contain statements that are specific to the RHS, and the direct goto causes them to be executed in the LHS success path as well. This patch therefore creates a fresh block that the LHS and RHS success blocks can both jump to. --- I think the reason we currently get away with this is that `rhs_success_block` usually doesn't contain anything other than StorageDead statements for locals used by the RHS, and those statements don't seem to cause problems in the LHS success path (which never makes those locals live). But if we start adding meaningful statements for branch coverage (or MC/DC coverage), it's important to keep the LHS and RHS blocks separate.
rust-lang-ci · Mar 1, 2024 · ea5c627 · ea5c627
2 parents 17edace + a7832b1
commit ea5c627
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 50 deletions.
diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs
@@ -93,8 +93,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     variable_source_info,
                     true,
                 ));
-                this.cfg.goto(lhs_success_block, variable_source_info, rhs_success_block);
-                rhs_success_block.unit()
+
+                // Make the LHS and RHS success arms converge to a common block.
+                // (We can't just make LHS goto RHS, because `rhs_success_block`
+                // might contain statements that we don't want on the LHS path.)
+                let success_block = this.cfg.start_new_block();
+                this.cfg.goto(lhs_success_block, variable_source_info, success_block);
+                this.cfg.goto(rhs_success_block, variable_source_info, success_block);
+                success_block.unit()
             }
             ExprKind::Unary { op: UnOp::Not, arg } => {
                 let local_scope = this.local_scope();

diff --git a/tests/mir-opt/building/logical_or_in_conditional.test_complex.built.after.mir b/tests/mir-opt/building/logical_or_in_conditional.test_complex.built.after.mir
@@ -19,7 +19,7 @@ fn test_complex() -> () {
     bb0: {
         StorageLive(_1);
         StorageLive(_2);
-        _2 = E::f() -> [return: bb1, unwind: bb37];
+        _2 = E::f() -> [return: bb1, unwind: bb38];
     }
 
     bb1: {
@@ -34,7 +34,7 @@ fn test_complex() -> () {
     }
 
     bb3: {
-        goto -> bb22;
+        goto -> bb23;
     }
 
     bb4: {
@@ -51,7 +51,7 @@ fn test_complex() -> () {
 
     bb7: {
         StorageLive(_4);
-        _4 = always_true() -> [return: bb8, unwind: bb37];
+        _4 = always_true() -> [return: bb8, unwind: bb38];
     }
 
     bb8: {
@@ -73,7 +73,7 @@ fn test_complex() -> () {
     }
 
     bb11: {
-        drop(_7) -> [return: bb13, unwind: bb37];
+        drop(_7) -> [return: bb13, unwind: bb38];
     }
 
     bb12: {
@@ -83,11 +83,11 @@ fn test_complex() -> () {
     bb13: {
         StorageDead(_7);
         StorageDead(_6);
-        goto -> bb19;
+        goto -> bb20;
     }
 
     bb14: {
-        drop(_7) -> [return: bb15, unwind: bb37];
+        drop(_7) -> [return: bb15, unwind: bb38];
     }
 
     bb15: {
@@ -107,106 +107,110 @@ fn test_complex() -> () {
     }
 
     bb17: {
-        drop(_10) -> [return: bb19, unwind: bb37];
+        drop(_10) -> [return: bb19, unwind: bb38];
     }
 
     bb18: {
-        goto -> bb20;
+        goto -> bb21;
     }
 
     bb19: {
         StorageDead(_10);
         StorageDead(_9);
-        _1 = const ();
-        goto -> bb23;
+        goto -> bb20;
     }
 
     bb20: {
-        drop(_10) -> [return: bb21, unwind: bb37];
+        _1 = const ();
+        goto -> bb24;
     }
 
     bb21: {
-        StorageDead(_10);
-        StorageDead(_9);
-        goto -> bb22;
+        drop(_10) -> [return: bb22, unwind: bb38];
     }
 
     bb22: {
-        _1 = const ();
+        StorageDead(_10);
+        StorageDead(_9);
         goto -> bb23;
     }
 
     bb23: {
+        _1 = const ();
+        goto -> bb24;
+    }
+
+    bb24: {
         StorageDead(_8);
         StorageDead(_5);
         StorageDead(_4);
         StorageDead(_2);
         StorageDead(_1);
         StorageLive(_11);
-        _11 = always_true() -> [return: bb24, unwind: bb37];
-    }
-
-    bb24: {
-        switchInt(move _11) -> [0: bb26, otherwise: bb25];
+        _11 = always_true() -> [return: bb25, unwind: bb38];
     }
 
     bb25: {
-        goto -> bb35;
+        switchInt(move _11) -> [0: bb27, otherwise: bb26];
     }
 
     bb26: {
-        goto -> bb27;
+        goto -> bb36;
     }
 
     bb27: {
-        StorageLive(_12);
-        _12 = E::f() -> [return: bb28, unwind: bb37];
+        goto -> bb28;
     }
 
     bb28: {
-        PlaceMention(_12);
-        _13 = discriminant(_12);
-        switchInt(move _13) -> [1: bb32, otherwise: bb30];
+        StorageLive(_12);
+        _12 = E::f() -> [return: bb29, unwind: bb38];
     }
 
     bb29: {
-        FakeRead(ForMatchedPlace(None), _12);
-        unreachable;
+        PlaceMention(_12);
+        _13 = discriminant(_12);
+        switchInt(move _13) -> [1: bb33, otherwise: bb31];
     }
 
     bb30: {
-        goto -> bb35;
+        FakeRead(ForMatchedPlace(None), _12);
+        unreachable;
     }
 
     bb31: {
-        goto -> bb29;
+        goto -> bb36;
     }
 
     bb32: {
-        falseEdge -> [real: bb34, imaginary: bb30];
+        goto -> bb30;
     }
 
     bb33: {
-        goto -> bb30;
+        falseEdge -> [real: bb35, imaginary: bb31];
     }
 
     bb34: {
-        _0 = const ();
-        goto -> bb36;
+        goto -> bb31;
     }
 
     bb35: {
         _0 = const ();
-        goto -> bb36;
+        goto -> bb37;
     }
 
     bb36: {
+        _0 = const ();
+        goto -> bb37;
+    }
+
+    bb37: {
         StorageDead(_11);
         StorageDead(_12);
         return;
     }
 
-    bb37 (cleanup): {
+    bb38 (cleanup): {
         resume;
     }
 }
diff --git a/tests/mir-opt/building/logical_or_in_conditional.test_or.built.after.mir b/tests/mir-opt/building/logical_or_in_conditional.test_or.built.after.mir
@@ -20,7 +20,7 @@ fn test_or() -> () {
     }
 
     bb1: {
-        drop(_3) -> [return: bb3, unwind: bb12];
+        drop(_3) -> [return: bb3, unwind: bb13];
     }
 
     bb2: {
@@ -30,11 +30,11 @@ fn test_or() -> () {
     bb3: {
         StorageDead(_3);
         StorageDead(_2);
-        goto -> bb8;
+        goto -> bb9;
     }
 
     bb4: {
-        drop(_3) -> [return: bb5, unwind: bb12];
+        drop(_3) -> [return: bb5, unwind: bb13];
     }
 
     bb5: {
@@ -50,38 +50,42 @@ fn test_or() -> () {
     }
 
     bb6: {
-        drop(_6) -> [return: bb8, unwind: bb12];
+        drop(_6) -> [return: bb8, unwind: bb13];
     }
 
     bb7: {
-        goto -> bb9;
+        goto -> bb10;
     }
 
     bb8: {
         StorageDead(_6);
         StorageDead(_5);
-        _0 = const ();
-        goto -> bb11;
+        goto -> bb9;
     }
 
     bb9: {
-        drop(_6) -> [return: bb10, unwind: bb12];
+        _0 = const ();
+        goto -> bb12;
     }
 
     bb10: {
+        drop(_6) -> [return: bb11, unwind: bb13];
+    }
+
+    bb11: {
         StorageDead(_6);
         StorageDead(_5);
         _0 = const ();
-        goto -> bb11;
+        goto -> bb12;
     }
 
-    bb11: {
+    bb12: {
         StorageDead(_4);
         StorageDead(_1);
         return;
     }
 
-    bb12 (cleanup): {
+    bb13 (cleanup): {
         resume;
     }
 }