Auto merge of #79109 - richkadel:llvm-coverage-counters-2.0.5, r=tmandry

Coverage tests for remaining TerminatorKinds and async, improve Assert

Tested and validate results for panic unwind, panic abort, assert!()
macro, TerminatorKind::Assert (for example, numeric overflow), and
async/await.

Implemented a previous documented idea to change Assert handling to be
the same as FalseUnwind and Goto, so it doesn't get its own
BasicCoverageBlock anymore. This changed a couple of coverage regions,
but I validated those changes are not any worse than the prior results,
and probably help assure some consistency (even if some people might
disagree with how the code region is consistently computed).

Fixed issue with async/await. AggregateKind::Generator needs to be
handled like AggregateKind::Closure; coverage span for the outer async
function should not "cover" the async body, which is actually executed
in a separate "closure" MIR.

1 files changed, 28 insertions, 49 deletions

diff --git a/compiler/rustc_mir/src/transform/coverage/mod.rs b/compiler/rustc_mir/src/transform/coverage/mod.rs
index 192bb6680e4..10f522d6746 100644
--- a/compiler/rustc_mir/src/transform/coverage/mod.rs
+++ b/compiler/rustc_mir/src/transform/coverage/mod.rs
@@ -88,6 +88,7 @@ struct Instrumentor<'a, 'tcx> {
     pass_name: &'a str,
     tcx: TyCtxt<'tcx>,
     mir_body: &'a mut mir::Body<'tcx>,
+    fn_sig_span: Span,
     body_span: Span,
     basic_coverage_blocks: CoverageGraph,
     coverage_counters: CoverageCounters,
@@ -95,14 +96,19 @@ struct Instrumentor<'a, 'tcx> {
 
 impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
     fn new(pass_name: &'a str, tcx: TyCtxt<'tcx>, mir_body: &'a mut mir::Body<'tcx>) -> Self {
-        let hir_body = hir_body(tcx, mir_body.source.def_id());
+        let (some_fn_sig, hir_body) = fn_sig_and_body(tcx, mir_body.source.def_id());
         let body_span = hir_body.value.span;
+        let fn_sig_span = match some_fn_sig {
+            Some(fn_sig) => fn_sig.span.with_hi(body_span.lo()),
+            None => body_span.shrink_to_lo(),
+        };
         let function_source_hash = hash_mir_source(tcx, hir_body);
         let basic_coverage_blocks = CoverageGraph::from_mir(mir_body);
         Self {
             pass_name,
             tcx,
             mir_body,
+            fn_sig_span,
             body_span,
             basic_coverage_blocks,
             coverage_counters: CoverageCounters::new(function_source_hash),
@@ -114,9 +120,15 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
         let source_map = tcx.sess.source_map();
         let mir_source = self.mir_body.source;
         let def_id = mir_source.def_id();
+        let fn_sig_span = self.fn_sig_span;
         let body_span = self.body_span;
 
-        debug!("instrumenting {:?}, span: {}", def_id, source_map.span_to_string(body_span));
+        debug!(
+            "instrumenting {:?}, fn sig span: {}, body span: {}",
+            def_id,
+            source_map.span_to_string(fn_sig_span),
+            source_map.span_to_string(body_span)
+        );
 
         let mut graphviz_data = debug::GraphvizData::new();
         let mut debug_used_expressions = debug::UsedExpressions::new();
@@ -138,6 +150,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
         // Compute `CoverageSpan`s from the `CoverageGraph`.
         let coverage_spans = CoverageSpans::generate_coverage_spans(
             &self.mir_body,
+            fn_sig_span,
             body_span,
             &self.basic_coverage_blocks,
         );
@@ -272,47 +285,13 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
                 bug!("Every BasicCoverageBlock should have a Counter or Expression");
             };
             graphviz_data.add_bcb_coverage_span_with_counter(bcb, &covspan, &counter_kind);
-            // FIXME(#78542): Can spans for `TerminatorKind::Goto` be improved to avoid special
-            // cases?
-            let some_code_region = if self.is_code_region_redundant(bcb, span, body_span) {
-                None
-            } else {
-                Some(make_code_region(file_name, &source_file, span, body_span))
-            };
-            inject_statement(self.mir_body, counter_kind, self.bcb_last_bb(bcb), some_code_region);
-        }
-    }
-
-    /// Returns true if the type of `BasicCoverageBlock` (specifically, it's `BasicBlock`s
-    /// `TerminatorKind`) with the given `Span` (relative to the `body_span`) is known to produce
-    /// a redundant coverage count.
-    ///
-    /// There is at least one case for this, and if it's not handled, the last line in a function
-    /// will be double-counted.
-    ///
-    /// If this method returns `true`, the counter (which other `Expressions` may depend on) is
-    /// still injected, but without an associated code region.
-    // FIXME(#78542): Can spans for `TerminatorKind::Goto` be improved to avoid special cases?
-    fn is_code_region_redundant(
-        &self,
-        bcb: BasicCoverageBlock,
-        span: Span,
-        body_span: Span,
-    ) -> bool {
-        if span.hi() == body_span.hi() {
-            // All functions execute a `Return`-terminated `BasicBlock`, regardless of how the
-            // function returns; but only some functions also _can_ return after a `Goto` block
-            // that ends on the closing brace of the function (with the `Return`). When this
-            // happens, the last character is counted 2 (or possibly more) times, when we know
-            // the function returned only once (of course). By giving all `Goto` terminators at
-            // the end of a function a `non-reportable` code region, they are still counted
-            // if appropriate, but they don't increment the line counter, as long as their is
-            // also a `Return` on that last line.
-            if let TerminatorKind::Goto { .. } = self.bcb_terminator(bcb).kind {
-                return true;
-            }
+            inject_statement(
+                self.mir_body,
+                counter_kind,
+                self.bcb_last_bb(bcb),
+                Some(make_code_region(file_name, &source_file, span, body_span)),
+            );
         }
-        false
     }
 
     /// `inject_coverage_span_counters()` looped through the `CoverageSpan`s and injected the
@@ -412,11 +391,6 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
     }
 
     #[inline]
-    fn bcb_terminator(&self, bcb: BasicCoverageBlock) -> &Terminator<'tcx> {
-        self.bcb_data(bcb).terminator(self.mir_body)
-    }
-
-    #[inline]
     fn bcb_data(&self, bcb: BasicCoverageBlock) -> &BasicCoverageBlockData {
         &self.basic_coverage_blocks[bcb]
     }
@@ -521,10 +495,15 @@ fn make_code_region(
     }
 }
 
-fn hir_body<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> &'tcx rustc_hir::Body<'tcx> {
+fn fn_sig_and_body<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    def_id: DefId,
+) -> (Option<&'tcx rustc_hir::FnSig<'tcx>>, &'tcx rustc_hir::Body<'tcx>) {
+    // FIXME(#79625): Consider improving MIR to provide the information needed, to avoid going back
+    // to HIR for it.
     let hir_node = tcx.hir().get_if_local(def_id).expect("expected DefId is local");
     let fn_body_id = hir::map::associated_body(hir_node).expect("HIR node is a function with body");
-    tcx.hir().body(fn_body_id)
+    (hir::map::fn_sig(hir_node), tcx.hir().body(fn_body_id))
 }
 
 fn hash_mir_source<'tcx>(tcx: TyCtxt<'tcx>, hir_body: &'tcx rustc_hir::Body<'tcx>) -> u64 {