Skip to content

Commit e5f83d2

Browse files
borsbors
committed
Auto merge of #84797 - richkadel:cover-unreachable-statements, r=tmandry
Report coverage `0` of dead blocks Fixes: #84018 With `-Z instrument-coverage`, coverage reporting of dead blocks (for example, blocks dropped because a conditional branch is dropped, based on const evaluation) is now supported. If `instrument-coverage` is enabled, `simplify::remove_dead_blocks()` finds all dropped coverage `Statement`s and adds their `code_region`s as `Unreachable` coverage `Statement`s to the `START_BLOCK`, so they are still included in the coverage map. Check out the resulting changes in the test coverage reports in this PR (in [commit 1](0b0d293)). r? `@tmandry` cc: `@wesleywiser`
2 parents ac888e8 + 0b0d293 commit e5f83d2

21 files changed

+102
-65
lines changed

‎compiler/rustc_mir/src/transform/const_goto.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ impl<'tcx> MirPass<'tcx> for ConstGoto {
4747
// if we applied optimizations, we potentially have some cfg to cleanup to
4848
// make it easier for further passes
4949
if should_simplify {
50-
simplify_cfg(body);
50+
simplify_cfg(tcx, body);
5151
simplify_locals(body, tcx);
5252
}
5353
}

‎compiler/rustc_mir/src/transform/deduplicate_blocks.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ impl<'tcx> MirPass<'tcx> for DeduplicateBlocks {
2626
if has_opts_to_apply {
2727
let mut opt_applier = OptApplier { tcx, duplicates };
2828
opt_applier.visit_body(body);
29-
simplify_cfg(body);
29+
simplify_cfg(tcx, body);
3030
}
3131
}
3232
}

‎compiler/rustc_mir/src/transform/early_otherwise_branch.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ impl<'tcx> MirPass<'tcx> for EarlyOtherwiseBranch {
164164
// Since this optimization adds new basic blocks and invalidates others,
165165
// clean up the cfg to make it nicer for other passes
166166
if should_cleanup {
167-
simplify_cfg(body);
167+
simplify_cfg(tcx, body);
168168
}
169169
}
170170
}

‎compiler/rustc_mir/src/transform/generator.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -964,7 +964,7 @@ fn create_generator_drop_shim<'tcx>(
964964

965965
// Make sure we remove dead blocks to remove
966966
// unrelated code from the resume part of the function
967-
simplify::remove_dead_blocks(&mut body);
967+
simplify::remove_dead_blocks(tcx, &mut body);
968968

969969
dump_mir(tcx, None, "generator_drop", &0, &body, |_, _| Ok(()));
970970

@@ -1137,7 +1137,7 @@ fn create_generator_resume_function<'tcx>(
11371137

11381138
// Make sure we remove dead blocks to remove
11391139
// unrelated code from the drop part of the function
1140-
simplify::remove_dead_blocks(body);
1140+
simplify::remove_dead_blocks(tcx, body);
11411141

11421142
dump_mir(tcx, None, "generator_resume", &0, body, |_, _| Ok(()));
11431143
}

‎compiler/rustc_mir/src/transform/inline.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ impl<'tcx> MirPass<'tcx> for Inline {
5757
if inline(tcx, body) {
5858
debug!("running simplify cfg on {:?}", body.source);
5959
CfgSimplifier::new(body).simplify();
60-
remove_dead_blocks(body);
60+
remove_dead_blocks(tcx, body);
6161
}
6262
}
6363
}

‎compiler/rustc_mir/src/transform/match_branches.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ impl<'tcx> MirPass<'tcx> for MatchBranchSimplification {
167167
}
168168

169169
if should_cleanup {
170-
simplify_cfg(body);
170+
simplify_cfg(tcx, body);
171171
}
172172
}
173173
}

‎compiler/rustc_mir/src/transform/multiple_return_terminators.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,6 @@ impl<'tcx> MirPass<'tcx> for MultipleReturnTerminators {
3838
}
3939
}
4040

41-
simplify::remove_dead_blocks(body)
41+
simplify::remove_dead_blocks(tcx, body)
4242
}
4343
}

‎compiler/rustc_mir/src/transform/remove_unneeded_drops.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ impl<'tcx> MirPass<'tcx> for RemoveUnneededDrops {
3636
// if we applied optimizations, we potentially have some cfg to cleanup to
3737
// make it easier for further passes
3838
if should_simplify {
39-
simplify_cfg(body);
39+
simplify_cfg(tcx, body);
4040
}
4141
}
4242
}

‎compiler/rustc_mir/src/transform/simplify.rs

+37-5
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
3030
use crate::transform::MirPass;
3131
use rustc_index::vec::{Idx, IndexVec};
32+
use rustc_middle::mir::coverage::*;
3233
use rustc_middle::mir::visit::{MutVisitor, MutatingUseContext, PlaceContext, Visitor};
3334
use rustc_middle::mir::*;
3435
use rustc_middle::ty::TyCtxt;
@@ -46,9 +47,9 @@ impl SimplifyCfg {
4647
}
4748
}
4849

49-
pub fn simplify_cfg(body: &mut Body<'_>) {
50+
pub fn simplify_cfg(tcx: TyCtxt<'tcx>, body: &mut Body<'_>) {
5051
CfgSimplifier::new(body).simplify();
51-
remove_dead_blocks(body);
52+
remove_dead_blocks(tcx, body);
5253

5354
// FIXME: Should probably be moved into some kind of pass manager
5455
body.basic_blocks_mut().raw.shrink_to_fit();
@@ -59,9 +60,9 @@ impl<'tcx> MirPass<'tcx> for SimplifyCfg {
5960
Cow::Borrowed(&self.label)
6061
}
6162

62-
fn run_pass(&self, _tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
63+
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
6364
debug!("SimplifyCfg({:?}) - simplifying {:?}", self.label, body.source);
64-
simplify_cfg(body);
65+
simplify_cfg(tcx, body);
6566
}
6667
}
6768

@@ -286,7 +287,7 @@ impl<'a, 'tcx> CfgSimplifier<'a, 'tcx> {
286287
}
287288
}
288289

289-
pub fn remove_dead_blocks(body: &mut Body<'_>) {
290+
pub fn remove_dead_blocks(tcx: TyCtxt<'tcx>, body: &mut Body<'_>) {
290291
let reachable = traversal::reachable_as_bitset(body);
291292
let num_blocks = body.basic_blocks().len();
292293
if num_blocks == reachable.count() {
@@ -306,6 +307,11 @@ pub fn remove_dead_blocks(body: &mut Body<'_>) {
306307
}
307308
used_blocks += 1;
308309
}
310+
311+
if tcx.sess.instrument_coverage() {
312+
save_unreachable_coverage(basic_blocks, used_blocks);
313+
}
314+
309315
basic_blocks.raw.truncate(used_blocks);
310316

311317
for block in basic_blocks {
@@ -315,6 +321,32 @@ pub fn remove_dead_blocks(body: &mut Body<'_>) {
315321
}
316322
}
317323

324+
fn save_unreachable_coverage(
325+
basic_blocks: &mut IndexVec<BasicBlock, BasicBlockData<'_>>,
326+
first_dead_block: usize,
327+
) {
328+
// retain coverage info for dead blocks, so coverage reports will still
329+
// report `0` executions for the uncovered code regions.
330+
let mut dropped_coverage = Vec::new();
331+
for dead_block in first_dead_block..basic_blocks.len() {
332+
for statement in basic_blocks[BasicBlock::new(dead_block)].statements.iter() {
333+
if let StatementKind::Coverage(coverage) = &statement.kind {
334+
if let Some(code_region) = &coverage.code_region {
335+
dropped_coverage.push((statement.source_info, code_region.clone()));
336+
}
337+
}
338+
}
339+
}
340+
for (source_info, code_region) in dropped_coverage {
341+
basic_blocks[START_BLOCK].statements.push(Statement {
342+
source_info,
343+
kind: StatementKind::Coverage(box Coverage {
344+
kind: CoverageKind::Unreachable,
345+
code_region: Some(code_region),
346+
}),
347+
})
348+
}
349+
}
318350
pub struct SimplifyLocals;
319351

320352
impl<'tcx> MirPass<'tcx> for SimplifyLocals {

‎compiler/rustc_mir/src/transform/simplify_try.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -558,7 +558,7 @@ impl<'tcx> MirPass<'tcx> for SimplifyBranchSame {
558558

559559
if did_remove_blocks {
560560
// We have dead blocks now, so remove those.
561-
simplify::remove_dead_blocks(body);
561+
simplify::remove_dead_blocks(tcx, body);
562562
}
563563
}
564564
}

‎compiler/rustc_mir/src/transform/unreachable_prop.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ impl MirPass<'_> for UnreachablePropagation {
6060
}
6161

6262
if replaced {
63-
simplify::remove_dead_blocks(body);
63+
simplify::remove_dead_blocks(tcx, body);
6464
}
6565
}
6666
}

‎src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.async2.txt

+1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
12| 1| if b {
1313
13| 1| println!("non_async_func println in block");
1414
14| 1| }
15+
^0
1516
15| 1|}
1617
16| |
1718
17| |

‎src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.conditions.txt

+6-2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
5| 1| if true {
66
6| 1| countdown = 10;
77
7| 1| }
8+
^0
89
8| |
910
9| | const B: u32 = 100;
1011
10| 1| let x = if countdown > 7 {
@@ -24,6 +25,7 @@
2425
24| 1| if true {
2526
25| 1| countdown = 10;
2627
26| 1| }
28+
^0
2729
27| |
2830
28| 1| if countdown > 7 {
2931
29| 1| countdown -= 4;
@@ -42,6 +44,7 @@
4244
41| 1| if true {
4345
42| 1| countdown = 10;
4446
43| 1| }
47+
^0
4548
44| |
4649
45| 1| if countdown > 7 {
4750
46| 1| countdown -= 4;
@@ -54,13 +57,14 @@
5457
53| | } else {
5558
54| 0| return;
5659
55| | }
57-
56| | } // Note: closing brace shows uncovered (vs. `0` for implicit else) because condition literal
58-
57| | // `true` was const-evaluated. The compiler knows the `if` block will be executed.
60+
56| 0| }
61+
57| |
5962
58| |
6063
59| 1| let mut countdown = 0;
6164
60| 1| if true {
6265
61| 1| countdown = 1;
6366
62| 1| }
67+
^0
6468
63| |
6569
64| 1| let z = if countdown > 7 {
6670
^0

‎src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.doctest.txt

+2-2
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
8| 1|//! assert_eq!(1, 1);
1010
9| |//! } else {
1111
10| |//! // this is not!
12-
11| |//! assert_eq!(1, 2);
12+
11| 0|//! assert_eq!(1, 2);
1313
12| |//! }
1414
13| 1|//! ```
1515
14| |//!
@@ -84,7 +84,7 @@
8484
74| 1| if true {
8585
75| 1| assert_eq!(1, 1);
8686
76| | } else {
87-
77| | assert_eq!(1, 2);
87+
77| 0| assert_eq!(1, 2);
8888
78| | }
8989
79| 1|}
9090
80| |

‎src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.drop_trait.txt

+5-5
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,11 @@
1919
19| 1| if true {
2020
20| 1| println!("Exiting with error...");
2121
21| 1| return Err(1);
22-
22| | }
23-
23| |
24-
24| | let _ = Firework { strength: 1000 };
25-
25| |
26-
26| | Ok(())
22+
22| 0| }
23+
23| 0|
24+
24| 0| let _ = Firework { strength: 1000 };
25+
25| 0|
26+
26| 0| Ok(())
2727
27| 1|}
2828
28| |
2929
29| |// Expected program output:

‎src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.generics.txt

+9-9
Original file line numberDiff line numberDiff line change
@@ -52,15 +52,15 @@
5252
30| 1| if true {
5353
31| 1| println!("Exiting with error...");
5454
32| 1| return Err(1);
55-
33| | } // The remaining lines below have no coverage because `if true` (with the constant literal
56-
34| | // `true`) is guaranteed to execute the `then` block, which is also guaranteed to `return`.
57-
35| | // Thankfully, in the normal case, conditions are not guaranteed ahead of time, and as shown
58-
36| | // in other tests, the lines below would have coverage (which would show they had `0`
59-
37| | // executions, assuming the condition still evaluated to `true`).
60-
38| |
61-
39| | let _ = Firework { strength: 1000 };
62-
40| |
63-
41| | Ok(())
55+
33| 0| }
56+
34| 0|
57+
35| 0|
58+
36| 0|
59+
37| 0|
60+
38| 0|
61+
39| 0| let _ = Firework { strength: 1000 };
62+
40| 0|
63+
41| 0| Ok(())
6464
42| 1|}
6565
43| |
6666
44| |// Expected program output:

‎src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.loops_branches.txt

+19-19
Original file line numberDiff line numberDiff line change
@@ -9,23 +9,23 @@
99
9| 1| fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
1010
10| 1| if true {
1111
11| 1| if false {
12-
12| | while true {
13-
13| | }
12+
12| 0| while true {
13+
13| 0| }
1414
14| 1| }
15-
15| 1| write!(f, "error")?;
16-
^0
17-
16| | } else {
18-
17| | }
15+
15| 1| write!(f, "cool")?;
16+
^0
17+
16| 0| } else {
18+
17| 0| }
1919
18| |
2020
19| 10| for i in 0..10 {
2121
20| 10| if true {
2222
21| 10| if false {
23-
22| | while true {}
23+
22| 0| while true {}
2424
23| 10| }
25-
24| 10| write!(f, "error")?;
26-
^0
27-
25| | } else {
28-
26| | }
25+
24| 10| write!(f, "cool")?;
26+
^0
27+
25| 0| } else {
28+
26| 0| }
2929
27| | }
3030
28| 1| Ok(())
3131
29| 1| }
@@ -36,21 +36,21 @@
3636
34| |impl std::fmt::Display for DisplayTest {
3737
35| 1| fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
3838
36| 1| if false {
39-
37| | } else {
39+
37| 0| } else {
4040
38| 1| if false {
41-
39| | while true {}
41+
39| 0| while true {}
4242
40| 1| }
43-
41| 1| write!(f, "error")?;
44-
^0
43+
41| 1| write!(f, "cool")?;
44+
^0
4545
42| | }
4646
43| 10| for i in 0..10 {
4747
44| 10| if false {
48-
45| | } else {
48+
45| 0| } else {
4949
46| 10| if false {
50-
47| | while true {}
50+
47| 0| while true {}
5151
48| 10| }
52-
49| 10| write!(f, "error")?;
53-
^0
52+
49| 10| write!(f, "cool")?;
53+
^0
5454
50| | }
5555
51| | }
5656
52| 1| Ok(())

‎src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.tight_inf_loop.txt

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
1| 1|fn main() {
22
2| 1| if false {
3-
3| | loop {}
3+
3| 0| loop {}
44
4| 1| }
55
5| 1|}
66

‎src/test/run-make-fulldeps/coverage/conditions.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@ fn main() {
5353
} else {
5454
return;
5555
}
56-
} // Note: closing brace shows uncovered (vs. `0` for implicit else) because condition literal
57-
// `true` was const-evaluated. The compiler knows the `if` block will be executed.
56+
}
57+
5858

5959
let mut countdown = 0;
6060
if true {

‎src/test/run-make-fulldeps/coverage/generics.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,11 @@ fn main() -> Result<(),u8> {
3030
if true {
3131
println!("Exiting with error...");
3232
return Err(1);
33-
} // The remaining lines below have no coverage because `if true` (with the constant literal
34-
// `true`) is guaranteed to execute the `then` block, which is also guaranteed to `return`.
35-
// Thankfully, in the normal case, conditions are not guaranteed ahead of time, and as shown
36-
// in other tests, the lines below would have coverage (which would show they had `0`
37-
// executions, assuming the condition still evaluated to `true`).
33+
}
34+
35+
36+
37+
3838

3939
let _ = Firework { strength: 1000 };
4040

0 commit comments

Comments
 (0)