mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Rollup merge of #121784 - Zalathar:if-or-converge, r=Nadrieril

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

Extracted from #118305, where this is necessary to avoid introducing a bug when injecting marker statements into the then/else arms.

---

In the previous code (#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.
This commit is contained in:
Matthias Krüger 2024-03-01 17:51:30 +01:00 committed by GitHub
commit 1a4c93e3ed
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 69 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
compiler/rustc_mir_build/src/build/matches/mod.rs
View File

@ -93,8 +93,14 @@ pub(crate) fn then_else_break(
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();

84
tests/mir-opt/building/logical_or_in_conditional.test_complex.built.after.mir
View File

@ -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: {
PlaceMention(_12);
_13 = discriminant(_12);
switchInt(move _13) -> [1: bb33, otherwise: bb31];
}
bb30: {
FakeRead(ForMatchedPlace(None), _12);
unreachable;
}
bb30: {
goto -> bb35;
}
bb31: {
goto -> bb29;
goto -> bb36;
}
bb32: {
falseEdge -> [real: bb34, imaginary: bb30];
}
bb33: {
goto -> bb30;
}
bb33: {
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;
}
}

30
tests/mir-opt/building/logical_or_in_conditional.test_or.built.after.mir
View File

@ -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: {
StorageDead(_6);
StorageDead(_5);
_0 = const ();
goto -> bb11;
drop(_6) -> [return: bb11, unwind: bb13];
}
bb11: {
StorageDead(_6);
StorageDead(_5);
_0 = const ();
goto -> bb12;
}
bb12: {
StorageDead(_4);
StorageDead(_1);
return;
}
bb12 (cleanup): {
bb13 (cleanup): {
resume;
}
}