Skip to content

Commit bea112b

Browse files
committed
Revert "Auto merge of #84797 - richkadel:cover-unreachable-statements, r=tmandry"
This reverts commit e5f83d2, reversing changes made to ac888e8.
1 parent ba8d7e2 commit bea112b

21 files changed

+65
-102
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(tcx, body);
50+
simplify_cfg(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(tcx, body);
29+
simplify_cfg(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(tcx, body);
167+
simplify_cfg(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(tcx, &mut body);
967+
simplify::remove_dead_blocks(&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(tcx, body);
1140+
simplify::remove_dead_blocks(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(tcx, body);
60+
remove_dead_blocks(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(tcx, body);
170+
simplify_cfg(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(tcx, body)
41+
simplify::remove_dead_blocks(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(tcx, body);
39+
simplify_cfg(body);
4040
}
4141
}
4242
}

compiler/rustc_mir/src/transform/simplify.rs

+5-37
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
3030
use crate::transform::MirPass;
3131
use rustc_index::vec::{Idx, IndexVec};
32-
use rustc_middle::mir::coverage::*;
3332
use rustc_middle::mir::visit::{MutVisitor, MutatingUseContext, PlaceContext, Visitor};
3433
use rustc_middle::mir::*;
3534
use rustc_middle::ty::TyCtxt;
@@ -47,9 +46,9 @@ impl SimplifyCfg {
4746
}
4847
}
4948

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

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

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

@@ -287,7 +286,7 @@ impl<'a, 'tcx> CfgSimplifier<'a, 'tcx> {
287286
}
288287
}
289288

290-
pub fn remove_dead_blocks(tcx: TyCtxt<'tcx>, body: &mut Body<'_>) {
289+
pub fn remove_dead_blocks(body: &mut Body<'_>) {
291290
let reachable = traversal::reachable_as_bitset(body);
292291
let num_blocks = body.basic_blocks().len();
293292
if num_blocks == reachable.count() {
@@ -307,11 +306,6 @@ pub fn remove_dead_blocks(tcx: TyCtxt<'tcx>, body: &mut Body<'_>) {
307306
}
308307
used_blocks += 1;
309308
}
310-
311-
if tcx.sess.instrument_coverage() {
312-
save_unreachable_coverage(basic_blocks, used_blocks);
313-
}
314-
315309
basic_blocks.raw.truncate(used_blocks);
316310

317311
for block in basic_blocks {
@@ -321,32 +315,6 @@ pub fn remove_dead_blocks(tcx: TyCtxt<'tcx>, body: &mut Body<'_>) {
321315
}
322316
}
323317

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-
}
350318
pub struct SimplifyLocals;
351319

352320
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(tcx, body);
561+
simplify::remove_dead_blocks(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(tcx, body);
63+
simplify::remove_dead_blocks(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,7 +12,6 @@
1212
12| 1| if b {
1313
13| 1| println!("non_async_func println in block");
1414
14| 1| }
15-
^0
1615
15| 1|}
1716
16| |
1817
17| |

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

+2-6
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
5| 1| if true {
66
6| 1| countdown = 10;
77
7| 1| }
8-
^0
98
8| |
109
9| | const B: u32 = 100;
1110
10| 1| let x = if countdown > 7 {
@@ -25,7 +24,6 @@
2524
24| 1| if true {
2625
25| 1| countdown = 10;
2726
26| 1| }
28-
^0
2927
27| |
3028
28| 1| if countdown > 7 {
3129
29| 1| countdown -= 4;
@@ -44,7 +42,6 @@
4442
41| 1| if true {
4543
42| 1| countdown = 10;
4644
43| 1| }
47-
^0
4845
44| |
4946
45| 1| if countdown > 7 {
5047
46| 1| countdown -= 4;
@@ -57,14 +54,13 @@
5754
53| | } else {
5855
54| 0| return;
5956
55| | }
60-
56| 0| }
61-
57| |
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.
6259
58| |
6360
59| 1| let mut countdown = 0;
6461
60| 1| if true {
6562
61| 1| countdown = 1;
6663
62| 1| }
67-
^0
6864
63| |
6965
64| 1| let z = if countdown > 7 {
7066
^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| 0|//! assert_eq!(1, 2);
12+
11| |//! 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| 0| assert_eq!(1, 2);
87+
77| | 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| 0| }
23-
23| 0|
24-
24| 0| let _ = Firework { strength: 1000 };
25-
25| 0|
26-
26| 0| Ok(())
22+
22| | }
23+
23| |
24+
24| | let _ = Firework { strength: 1000 };
25+
25| |
26+
26| | 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| 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(())
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(())
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| 0| while true {
13-
13| 0| }
12+
12| | while true {
13+
13| | }
1414
14| 1| }
15-
15| 1| write!(f, "cool")?;
16-
^0
17-
16| 0| } else {
18-
17| 0| }
15+
15| 1| write!(f, "error")?;
16+
^0
17+
16| | } else {
18+
17| | }
1919
18| |
2020
19| 10| for i in 0..10 {
2121
20| 10| if true {
2222
21| 10| if false {
23-
22| 0| while true {}
23+
22| | while true {}
2424
23| 10| }
25-
24| 10| write!(f, "cool")?;
26-
^0
27-
25| 0| } else {
28-
26| 0| }
25+
24| 10| write!(f, "error")?;
26+
^0
27+
25| | } else {
28+
26| | }
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| 0| } else {
39+
37| | } else {
4040
38| 1| if false {
41-
39| 0| while true {}
41+
39| | while true {}
4242
40| 1| }
43-
41| 1| write!(f, "cool")?;
44-
^0
43+
41| 1| write!(f, "error")?;
44+
^0
4545
42| | }
4646
43| 10| for i in 0..10 {
4747
44| 10| if false {
48-
45| 0| } else {
48+
45| | } else {
4949
46| 10| if false {
50-
47| 0| while true {}
50+
47| | while true {}
5151
48| 10| }
52-
49| 10| write!(f, "cool")?;
53-
^0
52+
49| 10| write!(f, "error")?;
53+
^0
5454
50| | }
5555
51| | }
5656
52| 1| Ok(())
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
1| 1|fn main() {
22
2| 1| if false {
3-
3| 0| loop {}
3+
3| | 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-
}
57-
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.
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-
}
34-
35-
36-
37-
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`).
3838

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

0 commit comments

Comments
 (0)