Rollup merge of #85196 - richkadel:reverts-cover-unreachable-statements, r=tmandry

Revert "Auto merge of #84797 - richkadel:cover-unreachable-statements…

This reverts commit e5f83d24ae, reversing
changes made to ac888e8675.

See https://github.com/rust-lang/rust/pull/84797#issuecomment-839068132

r? `@tmandry`

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(tcx, body);
+            simplify_cfg(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(tcx, body);
+            simplify_cfg(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(tcx, body);
+            simplify_cfg(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(tcx, &mut body);
+    simplify::remove_dead_blocks(&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(tcx, body);
+    simplify::remove_dead_blocks(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(tcx, body);
+            remove_dead_blocks(body);
         }
     }
 }

compiler/rustc_mir/src/transform/match_branches.rs

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

compiler/rustc_mir/src/transform/multiple_return_terminators.rs

@@ -38,6 +38,6 @@ impl<'tcx> MirPass<'tcx> for MultipleReturnTerminators {
             }
         }
 
-        simplify::remove_dead_blocks(tcx, body)
+        simplify::remove_dead_blocks(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(tcx, body);
+            simplify_cfg(body);
         }
     }
 }

compiler/rustc_mir/src/transform/simplify.rs

@@ -29,7 +29,6 @@
 
 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;
@@ -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();
-    remove_dead_blocks(tcx, body);
+    remove_dead_blocks(body);
 
     // FIXME: Should probably be moved into some kind of pass manager
     body.basic_blocks_mut().raw.shrink_to_fit();
@@ -60,9 +59,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(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 num_blocks = body.basic_blocks().len();
     if num_blocks == reachable.count() {
@@ -307,11 +306,6 @@ pub fn remove_dead_blocks(tcx: TyCtxt<'tcx>, 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 {
@@ -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;
 
 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(tcx, body);
+            simplify::remove_dead_blocks(body);
         }
     }
 }

compiler/rustc_mir/src/transform/unreachable_prop.rs

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

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

@@ -12,7 +12,6 @@
    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,7 +5,6 @@
     5|      1|    if true {
     6|      1|        countdown = 10;
     7|      1|    }
-                   ^0
     8|       |
     9|       |    const B: u32 = 100;
    10|      1|    let x = if countdown > 7 {
@@ -25,7 +24,6 @@
    24|      1|    if true {
    25|      1|        countdown = 10;
    26|      1|    }
-                   ^0
    27|       |
    28|      1|    if countdown > 7 {
    29|      1|        countdown -= 4;
@@ -44,7 +42,6 @@
    41|      1|        if true {
    42|      1|            countdown = 10;
    43|      1|        }
-                       ^0
    44|       |
    45|      1|        if countdown > 7 {
    46|      1|            countdown -= 4;
@@ -57,14 +54,13 @@
    53|       |        } else {
    54|      0|            return;
    55|       |        }
-   56|      0|    }
-   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.
    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|      0|//!     assert_eq!(1, 2);
+   11|       |//!     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|      0|        assert_eq!(1, 2);
+   77|       |        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|      0|    }
-   23|      0|
-   24|      0|    let _ = Firework { strength: 1000 };
-   25|      0|
-   26|      0|    Ok(())
+   22|       |    }
+   23|       |
+   24|       |    let _ = Firework { strength: 1000 };
+   25|       |
+   26|       |    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|      0|    }
-   34|      0|
-   35|      0|
-   36|      0|
-   37|      0|
-   38|      0|
-   39|      0|    let _ = Firework { strength: 1000 };
-   40|      0|
-   41|      0|    Ok(())
+   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(())
    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|      0|                while true {
-   13|      0|                }
+   12|       |                while true {
+   13|       |                }
    14|      1|            }
-   15|      1|            write!(f, "cool")?;
-                                           ^0
-   16|      0|        } else {
-   17|      0|        }
+   15|      1|            write!(f, "error")?;
+                                            ^0
+   16|       |        } else {
+   17|       |        }
    18|       |
    19|     10|        for i in 0..10 {
    20|     10|            if true {
    21|     10|                if false {
-   22|      0|                    while true {}
+   22|       |                    while true {}
    23|     10|                }
-   24|     10|                write!(f, "cool")?;
-                                               ^0
-   25|      0|            } else {
-   26|      0|            }
+   24|     10|                write!(f, "error")?;
+                                                ^0
+   25|       |            } else {
+   26|       |            }
    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|      0|        } else {
+   37|       |        } else {
    38|      1|            if false {
-   39|      0|                while true {}
+   39|       |                while true {}
    40|      1|            }
-   41|      1|            write!(f, "cool")?;
-                                           ^0
+   41|      1|            write!(f, "error")?;
+                                            ^0
    42|       |        }
    43|     10|        for i in 0..10 {
    44|     10|            if false {
-   45|      0|            } else {
+   45|       |            } else {
    46|     10|                if false {
-   47|      0|                    while true {}
+   47|       |                    while true {}
    48|     10|                }
-   49|     10|                write!(f, "cool")?;
-                                               ^0
+   49|     10|                write!(f, "error")?;
+                                                ^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|      0|        loop {}
+    3|       |        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 };
 

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

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