WIP - fix coverage in attr macro spans

compiler/rustc_codegen_ssa/src/coverageinfo/map.rs

@@ -28,6 +28,7 @@ pub struct Expression {
 /// only whitespace or comments). According to LLVM Code Coverage Mapping documentation, "A count
 /// for a gap area is only used as the line execution count if there are no other regions on a
 /// line."
+#[derive(Debug)]
 pub struct FunctionCoverage<'tcx> {
     instance: Instance<'tcx>,
     source_hash: u64,
@@ -113,6 +114,14 @@ impl<'tcx> FunctionCoverage<'tcx> {
             expression_id, lhs, op, rhs, region
         );
         let expression_index = self.expression_index(u32::from(expression_id));
+        debug_assert!(
+            expression_index.as_usize() < self.expressions.len(),
+            "expression_index {} is out of range for expressions.len() = {}
+            for {:?}",
+            expression_index.as_usize(),
+            self.expressions.len(),
+            self,
+        );
         if let Some(previous_expression) = self.expressions[expression_index].replace(Expression {
             lhs,
             op,
@@ -150,6 +159,13 @@ impl<'tcx> FunctionCoverage<'tcx> {
             self.instance
         );
 
+        if !self.counters.iter().any(|region| region.is_some()) {
+            // FIXME(richkadel): How is this possible and why is it OK? I verified that the first
+            // test program that triggers this still works, and `llvm-cov` does not fail, so I need
+            // to allow it.
+            debug!("no counters for {:?}", self);
+        }
+
         let counter_regions = self.counter_regions();
         let (counter_expressions, expression_regions) = self.expressions_with_regions();
         let unreachable_regions = self.unreachable_regions();

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/coverage/mod.rs

@@ -32,7 +32,7 @@ use rustc_middle::mir::{
 use rustc_middle::ty::TyCtxt;
 use rustc_span::def_id::DefId;
 use rustc_span::source_map::SourceMap;
-use rustc_span::{CharPos, ExpnKind, Pos, SourceFile, Span, Symbol};
+use rustc_span::{CharPos, ExpnData, ExpnKind, Pos, SourceFile, Span, Symbol};
 
 /// A simple error message wrapper for `coverage::Error`s.
 #[derive(Debug)]
@@ -552,24 +552,109 @@ fn get_body_span<'tcx>(
     let mut body_span = hir_body.value.span;
     let def_id = mir_body.source.def_id();
 
-    if tcx.is_closure(def_id) {
+    // If debug is enabled, log expansion span info, if any.
+    debug!("{}", {
+        let mut span = body_span;
+        while span.from_expansion() {
+            let ExpnData { kind, call_site, .. } = span.ctxt().outer_expn_data();
+            debug!(
+                "Body expansion span: {:?}...
+                expanded from {:?}
+                at call_site={:?}",
+                span, kind, call_site
+            );
+            span = call_site;
+        }
+        "-- end of debug message showing body_span expansion --"
+    });
+
+    if body_span.in_derive_expansion() {
+        // Derive macro expansions can be expanded, but they expand to structs
+        // and struct fields, and I think it's less helpful in coverage reports.
+        return body_span;
+    }
+
+    let mut body_is_from_expansion = false;
+    if !tcx.is_closure(def_id) {
+        body_is_from_expansion = body_span.from_expansion();
+    } else {
         // If the MIR function is a closure, and if the closure body span
         // starts from a macro, but it's content is not in that macro, try
         // to find a non-macro callsite, and instrument the spans there
         // instead.
-        loop {
+        while body_span.from_expansion() {
+            body_is_from_expansion = true;
             let expn_data = body_span.ctxt().outer_expn_data();
-            if expn_data.is_root() {
-                break;
+            body_span = expn_data.call_site;
+            match expn_data.kind {
+                ExpnKind::Macro { .. } => {}
+                _ => break,
+            }
+        }
+    };
+
+    if !body_is_from_expansion {
+        // If the body_span (or recomputed body_span, for closures with macros)
+        // is not from an expansion, then there is no need to try to compute
+        // a span from the MIR statements and terminators.
+        return body_span;
+    }
+
+    // If the body is from an expansion, then there's no certainty that the
+    // internal statements were also expanded into the same ctxt or elsewhere.
+    // So, search the MIR `Statement` and `Terminator` spans, and if their
+    // spans exist ouside the `body_span` compute a different `body_span`
+    // from the spans of the MIR's `Statement`s and `Terminator`s, so
+    // coverage can be applied to those spans.
+
+    if let Ok(Some(max_span)) = spans::filtered_from_mir(mir_body).try_fold(
+        None,
+        |some_max_span: Option<Span>, mut span| {
+            if span.is_empty() || span == body_span {
+                // A span that matches the body_span doesn't add anything, and both spans that match
+                // the body_span, and empty spans can exist in the given body_span, even if the
+                // statements we want to cover are from a separate and distinct span, so we need
+                // to ignore them. If no other statements exist outside the body_span, then we will
+                // keep the given body_span.
+                return Ok(some_max_span); // continue iterating
+            }
+
+            while span.ctxt() != body_span.ctxt() {
+                if !span.from_expansion() {
+                    return Ok(some_max_span); // continue iterating
+                }
+                span = span.ctxt().outer_expn_data().call_site;
+            }
+
+            if body_span.contains(span) {
+                debug!(
+                    "using body_span={:?}; it contains a statement or terminator with span={:?}",
+                    body_span, span
+                );
+                return Err(()); // break from iterating
             }
-            if let ExpnKind::Macro { .. } = expn_data.kind {
-                body_span = expn_data.call_site;
+
+            if let Some(max_span) = some_max_span {
+                Ok(Some(max_span.to(span))) // extend max_span
             } else {
-                break;
+                Ok(Some(span)) // initialize max_span to first useful span
             }
+        },
+    ) {
+        debug!(
+            "max_span = {:?}; overlaps body_span? {}, body_span hi() only touches? {}",
+            max_span,
+            body_span.overlaps(max_span),
+            body_span.hi() == max_span.lo()
+        );
+        if body_span.overlaps(max_span) {
+            // extend the body_span to include the computed max_span
+            body_span = body_span.to(max_span);
+        } else {
+            // use the computed max_span instead of the original body_span
+            body_span = max_span;
         }
     }
-
     body_span
 }
 

compiler/rustc_mir/src/transform/coverage/spans.rs

@@ -1,5 +1,6 @@
 use super::debug::term_type;
 use super::graph::{BasicCoverageBlock, BasicCoverageBlockData, CoverageGraph, START_BCB};
+use super::spans;
 
 use crate::util::spanview::source_range_no_file;
 
@@ -889,6 +890,23 @@ pub(super) fn filtered_terminator_span(terminator: &'a Terminator<'tcx>) -> Opti
     }
 }
 
+/// Returns an iterator over all filtered statement and terminator spans.
+pub(super) fn filtered_from_mir<'a>(
+    mir_body: &'a mir::Body<'a>,
+) -> impl Iterator<Item = Span> + 'a {
+    mir_body.basic_blocks().iter().flat_map(|data| {
+        data.statements
+            .iter()
+            .enumerate()
+            .filter_map(move |(_, statement)| spans::filtered_statement_span(statement))
+            .chain(
+                data.terminator
+                    .iter()
+                    .filter_map(move |term| spans::filtered_terminator_span(term)),
+            )
+    })
+}
+
 /// Returns an extrapolated span (pre-expansion[^1]) corresponding to a range
 /// within the function's body source. This span is guaranteed to be contained
 /// within, or equal to, the `body_span`. If the extrapolated span is not

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_with_coverage(tcx, &mut body, simplify::DroppedCoverage::Allow);
 
     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_with_coverage(tcx, body, simplify::DroppedCoverage::Allow);
 
     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);
         }
     }
 }