Rollup merge of #85215 - richkadel:ice-fixes-minus-dead-blocks, r=tmandry

coverage bug fixes and some refactoring

This replaces the relevant commits (2 and 3) from PR #85082, and also corrects an error querying for coverageinfo.

1. `coverageinfo` query needs to use the same MIR as codegen

I ran into an error trying to fix dead block coverage and realized the
`coverageinfo` query is getting a different MIR compared to the
codegenned MIR, which can sometimes be a problem during mapgen.

I changed that query to use the `InstandeDef` (which includes the
generic parameter substitutions, prosibly specific to const params)
instead of the `DefId` (without unknown/default const substitutions).

2. Simplified body_span and filtered span code

  Some code cleanup extracted from future (but unfinished) commit to fix
  coverage in attr macro functions.

3. Spanview needs the relevant body_span used for coverage

The coverage body_span doesn't always match the function body_span.

r? ```@tmandry```

compiler/rustc_codegen_ssa/src/coverageinfo/map.rs

@@ -49,9 +49,9 @@ impl<'tcx> FunctionCoverage<'tcx> {
     }
 
     fn create(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>, is_used: bool) -> Self {
-        let coverageinfo = tcx.coverageinfo(instance.def_id());
+        let coverageinfo = tcx.coverageinfo(instance.def);
         debug!(
-            "FunctionCoverage::new(instance={:?}) has coverageinfo={:?}. is_used={}",
+            "FunctionCoverage::create(instance={:?}) has coverageinfo={:?}. is_used={}",
             instance, coverageinfo, is_used
         );
         Self {

compiler/rustc_codegen_ssa/src/mir/coverageinfo.rs

@@ -31,7 +31,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
                         bx.add_coverage_counter(instance, id, code_region);
                     }
 
-                    let coverageinfo = bx.tcx().coverageinfo(instance.def_id());
+                    let coverageinfo = bx.tcx().coverageinfo(instance.def);
 
                     let fn_name = bx.get_pgo_func_name_var(instance);
                     let hash = bx.const_u64(function_source_hash);

compiler/rustc_middle/src/query/mod.rs

@@ -335,10 +335,9 @@ rustc_queries! {
 
     /// Returns coverage summary info for a function, after executing the `InstrumentCoverage`
     /// MIR pass (assuming the -Zinstrument-coverage option is enabled).
-    query coverageinfo(key: DefId) -> mir::CoverageInfo {
-        desc { |tcx| "retrieving coverage info from MIR for `{}`", tcx.def_path_str(key) }
+    query coverageinfo(key: ty::InstanceDef<'tcx>) -> mir::CoverageInfo {
+        desc { |tcx| "retrieving coverage info from MIR for `{}`", tcx.def_path_str(key.def_id()) }
         storage(ArenaCacheSelector<'tcx>)
-        cache_on_disk_if { key.is_local() }
     }
 
     /// Returns the name of the file that contains the function body, if instrumented for coverage.

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

@@ -120,6 +120,7 @@ use rustc_index::vec::Idx;
 use rustc_middle::mir::coverage::*;
 use rustc_middle::mir::{self, BasicBlock, TerminatorKind};
 use rustc_middle::ty::TyCtxt;
+use rustc_span::Span;
 
 use std::iter;
 use std::lazy::SyncOnceCell;
@@ -636,6 +637,7 @@ pub(super) fn dump_coverage_spanview(
     mir_body: &mir::Body<'tcx>,
     basic_coverage_blocks: &CoverageGraph,
     pass_name: &str,
+    body_span: Span,
     coverage_spans: &Vec<CoverageSpan>,
 ) {
     let mir_source = mir_body.source;
@@ -647,7 +649,7 @@ pub(super) fn dump_coverage_spanview(
     let crate_name = tcx.crate_name(def_id.krate);
     let item_name = tcx.def_path(def_id).to_filename_friendly_no_crate();
     let title = format!("{}.{} - Coverage Spans", crate_name, item_name);
-    spanview::write_document(tcx, def_id, span_viewables, &title, &mut file)
+    spanview::write_document(tcx, body_span, span_viewables, &title, &mut file)
         .expect("Unexpected IO error dumping coverage spans as HTML");
 }
 

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

@@ -95,7 +95,7 @@ impl<'tcx> MirPass<'tcx> for InstrumentCoverage {
 
         trace!("InstrumentCoverage starting for {:?}", mir_source.def_id());
         Instrumentor::new(&self.name(), tcx, mir_body).inject_counters();
-        trace!("InstrumentCoverage starting for {:?}", mir_source.def_id());
+        trace!("InstrumentCoverage done for {:?}", mir_source.def_id());
     }
 }
 
@@ -116,25 +116,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
         let def_id = mir_body.source.def_id();
         let (some_fn_sig, hir_body) = fn_sig_and_body(tcx, def_id);
 
-        let mut body_span = hir_body.value.span;
-
-        if tcx.is_closure(def_id) {
-            // 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 {
-                let expn_data = body_span.ctxt().outer_expn_data();
-                if expn_data.is_root() {
-                    break;
-                }
-                if let ExpnKind::Macro { .. } = expn_data.kind {
-                    body_span = expn_data.call_site;
-                } else {
-                    break;
-                }
-            }
-        }
+        let body_span = get_body_span(tcx, hir_body, mir_body);
 
         let source_file = source_map.lookup_source_file(body_span.lo());
         let fn_sig_span = match some_fn_sig.filter(|fn_sig| {
@@ -144,6 +126,15 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
             Some(fn_sig) => fn_sig.span.with_hi(body_span.lo()),
             None => body_span.shrink_to_lo(),
         };
+
+        debug!(
+            "instrumenting {}: {:?}, fn sig span: {:?}, body span: {:?}",
+            if tcx.is_closure(def_id) { "closure" } else { "function" },
+            def_id,
+            fn_sig_span,
+            body_span
+        );
+
         let function_source_hash = hash_mir_source(tcx, hir_body);
         let basic_coverage_blocks = CoverageGraph::from_mir(mir_body);
         Self {
@@ -160,19 +151,11 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
 
     fn inject_counters(&'a mut self) {
         let tcx = self.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 {:?}, fn sig span: {}, body span: {}",
-            def_id,
-            source_map.span_to_diagnostic_string(fn_sig_span),
-            source_map.span_to_diagnostic_string(body_span)
-        );
-
         let mut graphviz_data = debug::GraphvizData::new();
         let mut debug_used_expressions = debug::UsedExpressions::new();
 
@@ -204,6 +187,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
                 self.mir_body,
                 &self.basic_coverage_blocks,
                 self.pass_name,
+                body_span,
                 &coverage_spans,
             );
         }
@@ -560,6 +544,35 @@ fn fn_sig_and_body<'tcx>(
     (hir::map::fn_sig(hir_node), tcx.hir().body(fn_body_id))
 }
 
+fn get_body_span<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    hir_body: &rustc_hir::Body<'tcx>,
+    mir_body: &mut mir::Body<'tcx>,
+) -> Span {
+    let mut body_span = hir_body.value.span;
+    let def_id = mir_body.source.def_id();
+
+    if tcx.is_closure(def_id) {
+        // 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 {
+            let expn_data = body_span.ctxt().outer_expn_data();
+            if expn_data.is_root() {
+                break;
+            }
+            if let ExpnKind::Macro { .. } = expn_data.kind {
+                body_span = expn_data.call_site;
+            } else {
+                break;
+            }
+        }
+    }
+
+    body_span
+}
+
 fn hash_mir_source<'tcx>(tcx: TyCtxt<'tcx>, hir_body: &'tcx rustc_hir::Body<'tcx>) -> u64 {
     let mut hcx = tcx.create_no_span_stable_hashing_context();
     hash(&mut hcx, &hir_body.value).to_smaller_hash()

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

@@ -120,8 +120,8 @@ impl CoverageVisitor {
     }
 }
 
-fn coverageinfo<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> CoverageInfo {
-    let mir_body = mir_body(tcx, def_id);
+fn coverageinfo<'tcx>(tcx: TyCtxt<'tcx>, instance_def: ty::InstanceDef<'tcx>) -> CoverageInfo {
+    let mir_body = tcx.instance_mir(instance_def);
 
     let mut coverage_visitor = CoverageVisitor {
         // num_counters always has at least the `ZERO` counter.

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

@@ -530,17 +530,25 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
                     .iter()
                     .enumerate()
                     .filter_map(move |(index, statement)| {
-                        filtered_statement_span(statement, self.body_span).map(
-                            |(span, expn_span)| {
-                                CoverageSpan::for_statement(
-                                    statement, span, expn_span, bcb, bb, index,
-                                )
-                            },
-                        )
+                        filtered_statement_span(statement).map(|span| {
+                            CoverageSpan::for_statement(
+                                statement,
+                                function_source_span(span, self.body_span),
+                                span,
+                                bcb,
+                                bb,
+                                index,
+                            )
+                        })
                     })
-                    .chain(filtered_terminator_span(data.terminator(), self.body_span).map(
-                        |(span, expn_span)| CoverageSpan::for_terminator(span, expn_span, bcb, bb),
-                    ))
+                    .chain(filtered_terminator_span(data.terminator()).map(|span| {
+                        CoverageSpan::for_terminator(
+                            function_source_span(span, self.body_span),
+                            span,
+                            bcb,
+                            bb,
+                        )
+                    }))
             })
             .collect()
     }
@@ -795,13 +803,9 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
     }
 }
 
-/// See `function_source_span()` for a description of the two returned spans.
-/// If the MIR `Statement` is not contributive to computing coverage spans,
-/// returns `None`.
-pub(super) fn filtered_statement_span(
-    statement: &'a Statement<'tcx>,
-    body_span: Span,
-) -> Option<(Span, Span)> {
+/// If the MIR `Statement` has a span contributive to computing coverage spans,
+/// return it; otherwise return `None`.
+pub(super) fn filtered_statement_span(statement: &'a Statement<'tcx>) -> Option<Span> {
     match statement.kind {
         // These statements have spans that are often outside the scope of the executed source code
         // for their parent `BasicBlock`.
@@ -838,18 +842,14 @@ pub(super) fn filtered_statement_span(
         | StatementKind::LlvmInlineAsm(_)
         | StatementKind::Retag(_, _)
         | StatementKind::AscribeUserType(_, _) => {
-            Some(function_source_span(statement.source_info.span, body_span))
+            Some(statement.source_info.span)
         }
     }
 }
 
-/// See `function_source_span()` for a description of the two returned spans.
-/// If the MIR `Terminator` is not contributive to computing coverage spans,
-/// returns `None`.
-pub(super) fn filtered_terminator_span(
-    terminator: &'a Terminator<'tcx>,
-    body_span: Span,
-) -> Option<(Span, Span)> {
+/// If the MIR `Terminator` has a span contributive to computing coverage spans,
+/// return it; otherwise return `None`.
+pub(super) fn filtered_terminator_span(terminator: &'a Terminator<'tcx>) -> Option<Span> {
     match terminator.kind {
         // These terminators have spans that don't positively contribute to computing a reasonable
         // span of actually executed source code. (For example, SwitchInt terminators extracted from
@@ -873,7 +873,7 @@ pub(super) fn filtered_terminator_span(
                     span = span.with_lo(constant.span.lo());
                 }
             }
-            Some(function_source_span(span, body_span))
+            Some(span)
         }
 
         // Retain spans from all other terminators
@@ -884,28 +884,20 @@ pub(super) fn filtered_terminator_span(
         | TerminatorKind::GeneratorDrop
         | TerminatorKind::FalseUnwind { .. }
         | TerminatorKind::InlineAsm { .. } => {
-            Some(function_source_span(terminator.source_info.span, body_span))
+            Some(terminator.source_info.span)
         }
     }
 }
 
-/// Returns two spans from the given span (the span associated with a
-/// `Statement` or `Terminator`):
+/// 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
+/// contained within the `body_span`, the `body_span` is returned.
 ///
-///   1. 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
-///      contained within the `body_span`, the `body_span` is returned.
-///   2. The actual `span` value from the `Statement`, before expansion.
-///
-/// Only the first span is used when computing coverage code regions. The second
-/// span is useful if additional expansion data is needed (such as to look up
-/// the macro name for a composed span within that macro).)
-///
-/// [^1]Expansions result from Rust syntax including macros, syntactic
-/// sugar, etc.).
+/// [^1]Expansions result from Rust syntax including macros, syntactic sugar,
+/// etc.).
 #[inline]
-fn function_source_span(span: Span, body_span: Span) -> (Span, Span) {
+pub(super) fn function_source_span(span: Span, body_span: Span) -> Span {
     let original_span = original_sp(span, body_span).with_ctxt(body_span.ctxt());
-    (if body_span.contains(original_span) { original_span } else { body_span }, span)
+    if body_span.contains(original_span) { original_span } else { body_span }
 }

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

@@ -683,12 +683,10 @@ fn test_make_bcb_counters() {
         let mut basic_coverage_blocks = graph::CoverageGraph::from_mir(&mir_body);
         let mut coverage_spans = Vec::new();
         for (bcb, data) in basic_coverage_blocks.iter_enumerated() {
-            if let Some((span, expn_span)) =
-                spans::filtered_terminator_span(data.terminator(&mir_body), body_span)
-            {
+            if let Some(span) = spans::filtered_terminator_span(data.terminator(&mir_body)) {
                 coverage_spans.push(spans::CoverageSpan::for_terminator(
+                    spans::function_source_span(span, body_span),
                     span,
-                    expn_span,
                     bcb,
                     data.last_bb(),
                 ));

compiler/rustc_mir/src/util/spanview.rs

@@ -131,7 +131,7 @@ where
             }
         }
     }
-    write_document(tcx, def_id, span_viewables, title, w)?;
+    write_document(tcx, fn_span(tcx, def_id), span_viewables, title, w)?;
     Ok(())
 }
 
@@ -139,7 +139,7 @@ where
 /// list `SpanViewable`s.
 pub fn write_document<'tcx, W>(
     tcx: TyCtxt<'tcx>,
-    def_id: DefId,
+    spanview_span: Span,
     mut span_viewables: Vec<SpanViewable>,
     title: &str,
     w: &mut W,
@@ -147,16 +147,16 @@ pub fn write_document<'tcx, W>(
 where
     W: Write,
 {
-    let fn_span = fn_span(tcx, def_id);
-    let mut from_pos = fn_span.lo();
-    let end_pos = fn_span.hi();
+    let mut from_pos = spanview_span.lo();
+    let end_pos = spanview_span.hi();
     let source_map = tcx.sess.source_map();
     let start = source_map.lookup_char_pos(from_pos);
     let indent_to_initial_start_col = " ".repeat(start.col.to_usize());
     debug!(
-        "fn_span source is:\n{}{}",
+        "spanview_span={:?}; source is:\n{}{}",
+        spanview_span,
         indent_to_initial_start_col,
-        source_map.span_to_snippet(fn_span).expect("function should have printable source")
+        source_map.span_to_snippet(spanview_span).expect("function should have printable source")
     );
     writeln!(w, "{}", HEADER)?;
     writeln!(w, "<title>{}</title>", title)?;