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.

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();

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

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