Auto merge of rust-lang#84797 - richkadel:cover-unreachable-statement…

…s, r=tmandry

Report coverage `0` of dead blocks

Fixes: rust-lang#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](rust-lang@0b0d293)).

r? `@tmandry`
cc: `@wesleywiser`

compiler/rustc_mir/src/transform/const_goto.rs

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

compiler/rustc_mir/src/transform/deduplicate_blocks.rs

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

compiler/rustc_mir/src/transform/early_otherwise_branch.rs

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

compiler/rustc_mir/src/transform/generator.rs

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

compiler/rustc_mir/src/transform/inline.rs

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

compiler/rustc_mir/src/transform/match_branches.rs

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

compiler/rustc_mir/src/transform/multiple_return_terminators.rs

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

compiler/rustc_mir/src/transform/remove_unneeded_drops.rs

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

compiler/rustc_mir/src/transform/simplify.rs

@@ -29,6 +29,7 @@
 
 use crate::transform::MirPass;
 use rustc_index::vec::{Idx, IndexVec};
+use rustc_middle::mir::coverage::*;
 use rustc_middle::mir::visit::{MutVisitor, MutatingUseContext, PlaceContext, Visitor};
 use rustc_middle::mir::*;
 use rustc_middle::ty::TyCtxt;
@@ -46,9 +47,9 @@ impl SimplifyCfg {
     }
 }
 
-pub fn simplify_cfg(body: &mut Body<'_>) {
+pub fn simplify_cfg(tcx: TyCtxt<'tcx>, body: &mut Body<'_>) {
     CfgSimplifier::new(body).simplify();
-    remove_dead_blocks(body);
+    remove_dead_blocks(tcx, body);
 
     // FIXME: Should probably be moved into some kind of pass manager
     body.basic_blocks_mut().raw.shrink_to_fit();
@@ -59,9 +60,9 @@ impl<'tcx> MirPass<'tcx> for SimplifyCfg {
         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);
-        simplify_cfg(body);
+        simplify_cfg(tcx, body);
     }
 }
 
@@ -286,7 +287,7 @@ impl<'a, 'tcx> CfgSimplifier<'a, 'tcx> {
     }
 }
 
-pub fn remove_dead_blocks(body: &mut Body<'_>) {
+pub fn remove_dead_blocks(tcx: TyCtxt<'tcx>, body: &mut Body<'_>) {
     let reachable = traversal::reachable_as_bitset(body);
     let num_blocks = body.basic_blocks().len();
     if num_blocks == reachable.count() {
@@ -306,6 +307,11 @@ pub fn remove_dead_blocks(body: &mut Body<'_>) {
         }
         used_blocks += 1;
     }
+
+    if tcx.sess.instrument_coverage() {
+        save_unreachable_coverage(basic_blocks, used_blocks);
+    }
+
     basic_blocks.raw.truncate(used_blocks);
 
     for block in basic_blocks {
@@ -315,6 +321,32 @@ pub fn remove_dead_blocks(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;
 
 impl<'tcx> MirPass<'tcx> for SimplifyLocals {

compiler/rustc_mir/src/transform/simplify_try.rs

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

compiler/rustc_mir/src/transform/unreachable_prop.rs

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

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

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

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

@@ -5,6 +5,7 @@
     5|      1|    if true {
     6|      1|        countdown = 10;
     7|      1|    }
+                   ^0
     8|       |
     9|       |    const B: u32 = 100;
    10|      1|    let x = if countdown > 7 {
@@ -24,6 +25,7 @@
    24|      1|    if true {
    25|      1|        countdown = 10;
    26|      1|    }
+                   ^0
    27|       |
    28|      1|    if countdown > 7 {
    29|      1|        countdown -= 4;
@@ -42,6 +44,7 @@
    41|      1|        if true {
    42|      1|            countdown = 10;
    43|      1|        }
+                       ^0
    44|       |
    45|      1|        if countdown > 7 {
    46|      1|            countdown -= 4;
@@ -54,13 +57,14 @@
    53|       |        } else {
    54|      0|            return;
    55|       |        }
-   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|      0|    }
+   57|       |
    58|       |
    59|      1|    let mut countdown = 0;
    60|      1|    if true {
    61|      1|        countdown = 1;
    62|      1|    }
+                   ^0
    63|       |
    64|      1|    let z = if countdown > 7 {
                       ^0

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

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

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

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

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

@@ -52,15 +52,15 @@
    30|      1|    if true {
    31|      1|        println!("Exiting with error...");
    32|      1|        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`).
-   38|       |
-   39|       |    let _ = Firework { strength: 1000 };
-   40|       |
-   41|       |    Ok(())
+   33|      0|    }
+   34|      0|
+   35|      0|
+   36|      0|
+   37|      0|
+   38|      0|
+   39|      0|    let _ = Firework { strength: 1000 };
+   40|      0|
+   41|      0|    Ok(())
    42|      1|}
    43|       |
    44|       |// Expected program output:

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

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

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

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

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

@@ -53,8 +53,8 @@ fn main() {
         } else {
             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;
     if true {

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

@@ -30,11 +30,11 @@ fn main() -> Result<(),u8> {
     if true {
         println!("Exiting with error...");
         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 };