Make the success arms of if lhs || rhs meet up in a separate block

Zalathar · Zalathar · commit a7832b14b14e · 2024-02-29T20:24:43.000+11:00
In the previous code, 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.
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;
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -20,7 +20,7 @@ fn test_or() -> () {`
`20`	`20`	`}`
`21`	`21`
`22`	`22`	`bb1: {`
`23`		`- drop(_3) -> [return: bb3, unwind: bb12];`
	`23`	`+ drop(_3) -> [return: bb3, unwind: bb13];`
`24`	`24`	`}`
`25`	`25`
`26`	`26`	`bb2: {`
`@@ -30,11 +30,11 @@ fn test_or() -> () {`
`30`	`30`	`bb3: {`
`31`	`31`	`StorageDead(_3);`
`32`	`32`	`StorageDead(_2);`
`33`		`- goto -> bb8;`
	`33`	`+ goto -> bb9;`
`34`	`34`	`}`
`35`	`35`
`36`	`36`	`bb4: {`
`37`		`- drop(_3) -> [return: bb5, unwind: bb12];`
	`37`	`+ drop(_3) -> [return: bb5, unwind: bb13];`
`38`	`38`	`}`
`39`	`39`
`40`	`40`	`bb5: {`
`@@ -50,38 +50,42 @@ fn test_or() -> () {`
`50`	`50`	`}`
`51`	`51`
`52`	`52`	`bb6: {`
`53`		`- drop(_6) -> [return: bb8, unwind: bb12];`
	`53`	`+ drop(_6) -> [return: bb8, unwind: bb13];`
`54`	`54`	`}`
`55`	`55`
`56`	`56`	`bb7: {`
`57`		`- goto -> bb9;`
	`57`	`+ goto -> bb10;`
`58`	`58`	`}`
`59`	`59`
`60`	`60`	`bb8: {`
`61`	`61`	`StorageDead(_6);`
`62`	`62`	`StorageDead(_5);`
`63`		`- _0 = const ();`
`64`		`- goto -> bb11;`
	`63`	`+ goto -> bb9;`
`65`	`64`	`}`
`66`	`65`
`67`	`66`	`bb9: {`
`68`		`- drop(_6) -> [return: bb10, unwind: bb12];`
	`67`	`+ _0 = const ();`
	`68`	`+ goto -> bb12;`
`69`	`69`	`}`
`70`	`70`
`71`	`71`	`bb10: {`
	`72`	`+ drop(_6) -> [return: bb11, unwind: bb13];`
	`73`	`+ }`
	`74`	`+`
	`75`	`+ bb11: {`
`72`	`76`	`StorageDead(_6);`
`73`	`77`	`StorageDead(_5);`
`74`	`78`	`_0 = const ();`
`75`		`- goto -> bb11;`
	`79`	`+ goto -> bb12;`
`76`	`80`	`}`
`77`	`81`
`78`		`- bb11: {`
	`82`	`+ bb12: {`
`79`	`83`	`StorageDead(_4);`
`80`	`84`	`StorageDead(_1);`
`81`	`85`	`return;`
`82`	`86`	`}`
`83`	`87`
`84`		`- bb12 (cleanup): {`
	`88`	`+ bb13 (cleanup): {`
`85`	`89`	`resume;`
`86`	`90`	`}`
`87`	`91`	`}`