1
Fork 0

Revert "Auto merge of #84797 - richkadel:cover-unreachable-statements, r=tmandry"

This reverts commit e5f83d24ae, reversing
changes made to ac888e8675.
This commit is contained in:
Rich Kadel 2021-05-11 12:47:08 -07:00
parent ba8d7e2cb7
commit bea112ba07
21 changed files with 65 additions and 102 deletions

View file

@ -47,7 +47,7 @@ impl<'tcx> MirPass<'tcx> for ConstGoto {
// if we applied optimizations, we potentially have some cfg to cleanup to // if we applied optimizations, we potentially have some cfg to cleanup to
// make it easier for further passes // make it easier for further passes
if should_simplify { if should_simplify {
simplify_cfg(tcx, body); simplify_cfg(body);
simplify_locals(body, tcx); simplify_locals(body, tcx);
} }
} }

View file

@ -26,7 +26,7 @@ impl<'tcx> MirPass<'tcx> for DeduplicateBlocks {
if has_opts_to_apply { if has_opts_to_apply {
let mut opt_applier = OptApplier { tcx, duplicates }; let mut opt_applier = OptApplier { tcx, duplicates };
opt_applier.visit_body(body); opt_applier.visit_body(body);
simplify_cfg(tcx, body); simplify_cfg(body);
} }
} }
} }

View file

@ -164,7 +164,7 @@ impl<'tcx> MirPass<'tcx> for EarlyOtherwiseBranch {
// Since this optimization adds new basic blocks and invalidates others, // Since this optimization adds new basic blocks and invalidates others,
// clean up the cfg to make it nicer for other passes // clean up the cfg to make it nicer for other passes
if should_cleanup { if should_cleanup {
simplify_cfg(tcx, body); simplify_cfg(body);
} }
} }
} }

View file

@ -964,7 +964,7 @@ fn create_generator_drop_shim<'tcx>(
// Make sure we remove dead blocks to remove // Make sure we remove dead blocks to remove
// unrelated code from the resume part of the function // unrelated code from the resume part of the function
simplify::remove_dead_blocks(tcx, &mut body); simplify::remove_dead_blocks(&mut body);
dump_mir(tcx, None, "generator_drop", &0, &body, |_, _| Ok(())); dump_mir(tcx, None, "generator_drop", &0, &body, |_, _| Ok(()));
@ -1137,7 +1137,7 @@ fn create_generator_resume_function<'tcx>(
// Make sure we remove dead blocks to remove // Make sure we remove dead blocks to remove
// unrelated code from the drop part of the function // unrelated code from the drop part of the function
simplify::remove_dead_blocks(tcx, body); simplify::remove_dead_blocks(body);
dump_mir(tcx, None, "generator_resume", &0, body, |_, _| Ok(())); dump_mir(tcx, None, "generator_resume", &0, body, |_, _| Ok(()));
} }

View file

@ -57,7 +57,7 @@ impl<'tcx> MirPass<'tcx> for Inline {
if inline(tcx, body) { if inline(tcx, body) {
debug!("running simplify cfg on {:?}", body.source); debug!("running simplify cfg on {:?}", body.source);
CfgSimplifier::new(body).simplify(); CfgSimplifier::new(body).simplify();
remove_dead_blocks(tcx, body); remove_dead_blocks(body);
} }
} }
} }

View file

@ -167,7 +167,7 @@ impl<'tcx> MirPass<'tcx> for MatchBranchSimplification {
} }
if should_cleanup { if should_cleanup {
simplify_cfg(tcx, body); simplify_cfg(body);
} }
} }
} }

View file

@ -38,6 +38,6 @@ impl<'tcx> MirPass<'tcx> for MultipleReturnTerminators {
} }
} }
simplify::remove_dead_blocks(tcx, body) simplify::remove_dead_blocks(body)
} }
} }

View file

@ -36,7 +36,7 @@ impl<'tcx> MirPass<'tcx> for RemoveUnneededDrops {
// if we applied optimizations, we potentially have some cfg to cleanup to // if we applied optimizations, we potentially have some cfg to cleanup to
// make it easier for further passes // make it easier for further passes
if should_simplify { if should_simplify {
simplify_cfg(tcx, body); simplify_cfg(body);
} }
} }
} }

View file

@ -29,7 +29,6 @@
use crate::transform::MirPass; use crate::transform::MirPass;
use rustc_index::vec::{Idx, IndexVec}; use rustc_index::vec::{Idx, IndexVec};
use rustc_middle::mir::coverage::*;
use rustc_middle::mir::visit::{MutVisitor, MutatingUseContext, PlaceContext, Visitor}; use rustc_middle::mir::visit::{MutVisitor, MutatingUseContext, PlaceContext, Visitor};
use rustc_middle::mir::*; use rustc_middle::mir::*;
use rustc_middle::ty::TyCtxt; use rustc_middle::ty::TyCtxt;
@ -47,9 +46,9 @@ impl SimplifyCfg {
} }
} }
pub fn simplify_cfg(tcx: TyCtxt<'tcx>, body: &mut Body<'_>) { pub fn simplify_cfg(body: &mut Body<'_>) {
CfgSimplifier::new(body).simplify(); CfgSimplifier::new(body).simplify();
remove_dead_blocks(tcx, body); remove_dead_blocks(body);
// FIXME: Should probably be moved into some kind of pass manager // FIXME: Should probably be moved into some kind of pass manager
body.basic_blocks_mut().raw.shrink_to_fit(); body.basic_blocks_mut().raw.shrink_to_fit();
@ -60,9 +59,9 @@ impl<'tcx> MirPass<'tcx> for SimplifyCfg {
Cow::Borrowed(&self.label) Cow::Borrowed(&self.label)
} }
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { fn run_pass(&self, _tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
debug!("SimplifyCfg({:?}) - simplifying {:?}", self.label, body.source); debug!("SimplifyCfg({:?}) - simplifying {:?}", self.label, body.source);
simplify_cfg(tcx, body); simplify_cfg(body);
} }
} }
@ -287,7 +286,7 @@ impl<'a, 'tcx> CfgSimplifier<'a, 'tcx> {
} }
} }
pub fn remove_dead_blocks(tcx: TyCtxt<'tcx>, body: &mut Body<'_>) { pub fn remove_dead_blocks(body: &mut Body<'_>) {
let reachable = traversal::reachable_as_bitset(body); let reachable = traversal::reachable_as_bitset(body);
let num_blocks = body.basic_blocks().len(); let num_blocks = body.basic_blocks().len();
if num_blocks == reachable.count() { if num_blocks == reachable.count() {
@ -307,11 +306,6 @@ pub fn remove_dead_blocks(tcx: TyCtxt<'tcx>, body: &mut Body<'_>) {
} }
used_blocks += 1; used_blocks += 1;
} }
if tcx.sess.instrument_coverage() {
save_unreachable_coverage(basic_blocks, used_blocks);
}
basic_blocks.raw.truncate(used_blocks); basic_blocks.raw.truncate(used_blocks);
for block in basic_blocks { for block in basic_blocks {
@ -321,32 +315,6 @@ pub fn remove_dead_blocks(tcx: TyCtxt<'tcx>, body: &mut Body<'_>) {
} }
} }
fn save_unreachable_coverage(
basic_blocks: &mut IndexVec<BasicBlock, BasicBlockData<'_>>,
first_dead_block: usize,
) {
// retain coverage info for dead blocks, so coverage reports will still
// report `0` executions for the uncovered code regions.
let mut dropped_coverage = Vec::new();
for dead_block in first_dead_block..basic_blocks.len() {
for statement in basic_blocks[BasicBlock::new(dead_block)].statements.iter() {
if let StatementKind::Coverage(coverage) = &statement.kind {
if let Some(code_region) = &coverage.code_region {
dropped_coverage.push((statement.source_info, code_region.clone()));
}
}
}
}
for (source_info, code_region) in dropped_coverage {
basic_blocks[START_BLOCK].statements.push(Statement {
source_info,
kind: StatementKind::Coverage(box Coverage {
kind: CoverageKind::Unreachable,
code_region: Some(code_region),
}),
})
}
}
pub struct SimplifyLocals; pub struct SimplifyLocals;
impl<'tcx> MirPass<'tcx> for SimplifyLocals { impl<'tcx> MirPass<'tcx> for SimplifyLocals {

View file

@ -558,7 +558,7 @@ impl<'tcx> MirPass<'tcx> for SimplifyBranchSame {
if did_remove_blocks { if did_remove_blocks {
// We have dead blocks now, so remove those. // We have dead blocks now, so remove those.
simplify::remove_dead_blocks(tcx, body); simplify::remove_dead_blocks(body);
} }
} }
} }

View file

@ -60,7 +60,7 @@ impl MirPass<'_> for UnreachablePropagation {
} }
if replaced { if replaced {
simplify::remove_dead_blocks(tcx, body); simplify::remove_dead_blocks(body);
} }
} }
} }

View file

@ -12,7 +12,6 @@
12| 1| if b { 12| 1| if b {
13| 1| println!("non_async_func println in block"); 13| 1| println!("non_async_func println in block");
14| 1| } 14| 1| }
^0
15| 1|} 15| 1|}
16| | 16| |
17| | 17| |

View file

@ -5,7 +5,6 @@
5| 1| if true { 5| 1| if true {
6| 1| countdown = 10; 6| 1| countdown = 10;
7| 1| } 7| 1| }
^0
8| | 8| |
9| | const B: u32 = 100; 9| | const B: u32 = 100;
10| 1| let x = if countdown > 7 { 10| 1| let x = if countdown > 7 {
@ -25,7 +24,6 @@
24| 1| if true { 24| 1| if true {
25| 1| countdown = 10; 25| 1| countdown = 10;
26| 1| } 26| 1| }
^0
27| | 27| |
28| 1| if countdown > 7 { 28| 1| if countdown > 7 {
29| 1| countdown -= 4; 29| 1| countdown -= 4;
@ -44,7 +42,6 @@
41| 1| if true { 41| 1| if true {
42| 1| countdown = 10; 42| 1| countdown = 10;
43| 1| } 43| 1| }
^0
44| | 44| |
45| 1| if countdown > 7 { 45| 1| if countdown > 7 {
46| 1| countdown -= 4; 46| 1| countdown -= 4;
@ -57,14 +54,13 @@
53| | } else { 53| | } else {
54| 0| return; 54| 0| return;
55| | } 55| | }
56| 0| } 56| | } // Note: closing brace shows uncovered (vs. `0` for implicit else) because condition literal
57| | 57| | // `true` was const-evaluated. The compiler knows the `if` block will be executed.
58| | 58| |
59| 1| let mut countdown = 0; 59| 1| let mut countdown = 0;
60| 1| if true { 60| 1| if true {
61| 1| countdown = 1; 61| 1| countdown = 1;
62| 1| } 62| 1| }
^0
63| | 63| |
64| 1| let z = if countdown > 7 { 64| 1| let z = if countdown > 7 {
^0 ^0

View file

@ -9,7 +9,7 @@
8| 1|//! assert_eq!(1, 1); 8| 1|//! assert_eq!(1, 1);
9| |//! } else { 9| |//! } else {
10| |//! // this is not! 10| |//! // this is not!
11| 0|//! assert_eq!(1, 2); 11| |//! assert_eq!(1, 2);
12| |//! } 12| |//! }
13| 1|//! ``` 13| 1|//! ```
14| |//! 14| |//!
@ -84,7 +84,7 @@
74| 1| if true { 74| 1| if true {
75| 1| assert_eq!(1, 1); 75| 1| assert_eq!(1, 1);
76| | } else { 76| | } else {
77| 0| assert_eq!(1, 2); 77| | assert_eq!(1, 2);
78| | } 78| | }
79| 1|} 79| 1|}
80| | 80| |

View file

@ -19,11 +19,11 @@
19| 1| if true { 19| 1| if true {
20| 1| println!("Exiting with error..."); 20| 1| println!("Exiting with error...");
21| 1| return Err(1); 21| 1| return Err(1);
22| 0| } 22| | }
23| 0| 23| |
24| 0| let _ = Firework { strength: 1000 }; 24| | let _ = Firework { strength: 1000 };
25| 0| 25| |
26| 0| Ok(()) 26| | Ok(())
27| 1|} 27| 1|}
28| | 28| |
29| |// Expected program output: 29| |// Expected program output:

View file

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

View file

@ -9,23 +9,23 @@
9| 1| fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { 9| 1| fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
10| 1| if true { 10| 1| if true {
11| 1| if false { 11| 1| if false {
12| 0| while true { 12| | while true {
13| 0| } 13| | }
14| 1| } 14| 1| }
15| 1| write!(f, "cool")?; 15| 1| write!(f, "error")?;
^0 ^0
16| 0| } else { 16| | } else {
17| 0| } 17| | }
18| | 18| |
19| 10| for i in 0..10 { 19| 10| for i in 0..10 {
20| 10| if true { 20| 10| if true {
21| 10| if false { 21| 10| if false {
22| 0| while true {} 22| | while true {}
23| 10| } 23| 10| }
24| 10| write!(f, "cool")?; 24| 10| write!(f, "error")?;
^0 ^0
25| 0| } else { 25| | } else {
26| 0| } 26| | }
27| | } 27| | }
28| 1| Ok(()) 28| 1| Ok(())
29| 1| } 29| 1| }
@ -36,21 +36,21 @@
34| |impl std::fmt::Display for DisplayTest { 34| |impl std::fmt::Display for DisplayTest {
35| 1| fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { 35| 1| fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
36| 1| if false { 36| 1| if false {
37| 0| } else { 37| | } else {
38| 1| if false { 38| 1| if false {
39| 0| while true {} 39| | while true {}
40| 1| } 40| 1| }
41| 1| write!(f, "cool")?; 41| 1| write!(f, "error")?;
^0 ^0
42| | } 42| | }
43| 10| for i in 0..10 { 43| 10| for i in 0..10 {
44| 10| if false { 44| 10| if false {
45| 0| } else { 45| | } else {
46| 10| if false { 46| 10| if false {
47| 0| while true {} 47| | while true {}
48| 10| } 48| 10| }
49| 10| write!(f, "cool")?; 49| 10| write!(f, "error")?;
^0 ^0
50| | } 50| | }
51| | } 51| | }
52| 1| Ok(()) 52| 1| Ok(())

View file

@ -1,6 +1,6 @@
1| 1|fn main() { 1| 1|fn main() {
2| 1| if false { 2| 1| if false {
3| 0| loop {} 3| | loop {}
4| 1| } 4| 1| }
5| 1|} 5| 1|}

View file

@ -53,8 +53,8 @@ fn main() {
} else { } else {
return; return;
} }
} } // Note: closing brace shows uncovered (vs. `0` for implicit else) because condition literal
// `true` was const-evaluated. The compiler knows the `if` block will be executed.
let mut countdown = 0; let mut countdown = 0;
if true { if true {

View file

@ -30,11 +30,11 @@ fn main() -> Result<(),u8> {
if true { if true {
println!("Exiting with error..."); println!("Exiting with error...");
return Err(1); return Err(1);
} } // The remaining lines below have no coverage because `if true` (with the constant literal
// `true`) is guaranteed to execute the `then` block, which is also guaranteed to `return`.
// Thankfully, in the normal case, conditions are not guaranteed ahead of time, and as shown
// in other tests, the lines below would have coverage (which would show they had `0`
// executions, assuming the condition still evaluated to `true`).
let _ = Firework { strength: 1000 }; let _ = Firework { strength: 1000 };

View file

@ -12,7 +12,7 @@ impl std::fmt::Debug for DebugTest {
while true { while true {
} }
} }
write!(f, "cool")?; write!(f, "error")?;
} else { } else {
} }
@ -21,7 +21,7 @@ impl std::fmt::Debug for DebugTest {
if false { if false {
while true {} while true {}
} }
write!(f, "cool")?; write!(f, "error")?;
} else { } else {
} }
} }
@ -38,7 +38,7 @@ impl std::fmt::Display for DisplayTest {
if false { if false {
while true {} while true {}
} }
write!(f, "cool")?; write!(f, "error")?;
} }
for i in 0..10 { for i in 0..10 {
if false { if false {
@ -46,7 +46,7 @@ impl std::fmt::Display for DisplayTest {
if false { if false {
while true {} while true {}
} }
write!(f, "cool")?; write!(f, "error")?;
} }
} }
Ok(()) Ok(())