coverage: Don't use actual coverage spans in test_make_bcb_counters

This test calls internal functions in `spans` in order to create a list of
coverage spans to pass in when making BCB counters. That makes it difficult to
modify those internals without breaking the test.

However, making BCB counters doesn't require the actual coverage spans; it just
needs some way to identify which BCBs are associated with one or more spans.
This can be achieved by passing in a `Fn(BasicCoverageBlock) -> bool` instead.

Author: Zalathar

compiler/rustc_mir_transform/src/coverage/counters.rs

@@ -2,11 +2,9 @@ use super::Error;
 
 use super::debug;
 use super::graph;
-use super::spans;
 
 use debug::{DebugCounters, NESTED_INDENT};
 use graph::{BasicCoverageBlock, BcbBranch, CoverageGraph, TraverseCoverageGraphWithLoops};
-use spans::CoverageSpan;
 
 use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::graph::WithNumNodes;
@@ -108,9 +106,9 @@ impl CoverageCounters {
     pub fn make_bcb_counters(
         &mut self,
         basic_coverage_blocks: &CoverageGraph,
-        coverage_spans: &[CoverageSpan],
+        bcb_has_coverage_spans: impl Fn(BasicCoverageBlock) -> bool,
     ) -> Result<(), Error> {
-        MakeBcbCounters::new(self, basic_coverage_blocks).make_bcb_counters(coverage_spans)
+        MakeBcbCounters::new(self, basic_coverage_blocks).make_bcb_counters(bcb_has_coverage_spans)
     }
 
     fn make_counter<F>(&mut self, debug_block_label_fn: F) -> BcbCounter
@@ -270,14 +268,11 @@ impl<'a> MakeBcbCounters<'a> {
     /// Returns any non-code-span expressions created to represent intermediate values (such as to
     /// add two counters so the result can be subtracted from another counter), or an Error with
     /// message for subsequent debugging.
-    fn make_bcb_counters(&mut self, coverage_spans: &[CoverageSpan]) -> Result<(), Error> {
+    fn make_bcb_counters(
+        &mut self,
+        bcb_has_coverage_spans: impl Fn(BasicCoverageBlock) -> bool,
+    ) -> Result<(), Error> {
         debug!("make_bcb_counters(): adding a counter or expression to each BasicCoverageBlock");
-        let num_bcbs = self.basic_coverage_blocks.num_nodes();
-
-        let mut bcbs_with_coverage = BitSet::new_empty(num_bcbs);
-        for covspan in coverage_spans {
-            bcbs_with_coverage.insert(covspan.bcb);
-        }
 
         // Walk the `CoverageGraph`. For each `BasicCoverageBlock` node with an associated
         // `CoverageSpan`, add a counter. If the `BasicCoverageBlock` branches, add a counter or
@@ -291,7 +286,7 @@ impl<'a> MakeBcbCounters<'a> {
         // the current BCB is in one or more nested loops or not.
         let mut traversal = TraverseCoverageGraphWithLoops::new(&self.basic_coverage_blocks);
         while let Some(bcb) = traversal.next(self.basic_coverage_blocks) {
-            if bcbs_with_coverage.contains(bcb) {
+            if bcb_has_coverage_spans(bcb) {
                 debug!("{:?} has at least one `CoverageSpan`. Get or make its counter", bcb);
                 let branching_counter_operand = self.get_or_make_counter_operand(bcb)?;
 

compiler/rustc_mir_transform/src/coverage/mod.rs

@@ -16,6 +16,7 @@ use crate::MirPass;
 
 use rustc_data_structures::graph::WithNumNodes;
 use rustc_data_structures::sync::Lrc;
+use rustc_index::bit_set::BitSet;
 use rustc_index::IndexVec;
 use rustc_middle::hir;
 use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
@@ -196,6 +197,15 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
             );
         }
 
+        let bcb_has_coverage_spans = {
+            let mut bcbs_with_coverage_spans =
+                BitSet::new_empty(self.basic_coverage_blocks.num_nodes());
+            for covspan in &coverage_spans {
+                bcbs_with_coverage_spans.insert(covspan.bcb);
+            }
+            move |bcb| bcbs_with_coverage_spans.contains(bcb)
+        };
+
         ////////////////////////////////////////////////////
         // Create an optimized mix of `Counter`s and `Expression`s for the `CoverageGraph`. Ensure
         // every `CoverageSpan` has a `Counter` or `Expression` assigned to its `BasicCoverageBlock`
@@ -206,7 +216,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
         // direct association with any `BasicCoverageBlock`, are accumulated inside `coverage_counters`.
         let result = self
             .coverage_counters
-            .make_bcb_counters(&mut self.basic_coverage_blocks, &coverage_spans);
+            .make_bcb_counters(&mut self.basic_coverage_blocks, bcb_has_coverage_spans);
 
         if let Ok(()) = result {
             // If debugging, add any intermediate expressions (which are not associated with any

compiler/rustc_mir_transform/src/coverage/spans.rs

@@ -793,7 +793,7 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
 
 /// If the MIR `Statement` has a span contributive to computing coverage spans,
 /// return it; otherwise return `None`.
-pub(super) fn filtered_statement_span(statement: &Statement<'_>) -> Option<Span> {
+fn filtered_statement_span(statement: &Statement<'_>) -> Option<Span> {
     match statement.kind {
         // These statements have spans that are often outside the scope of the executed source code
         // for their parent `BasicBlock`.
@@ -840,7 +840,7 @@ pub(super) fn filtered_statement_span(statement: &Statement<'_>) -> Option<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: &Terminator<'_>) -> Option<Span> {
+fn filtered_terminator_span(terminator: &Terminator<'_>) -> 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
@@ -887,7 +887,7 @@ pub(super) fn filtered_terminator_span(terminator: &Terminator<'_>) -> Option<Sp
 /// [^1]Expansions result from Rust syntax including macros, syntactic sugar,
 /// etc.).
 #[inline]
-pub(super) fn function_source_span(span: Span, body_span: Span) -> Span {
+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 }
 }

compiler/rustc_mir_transform/src/coverage/tests.rs

@@ -26,7 +26,7 @@
 
 use super::counters;
 use super::graph;
-use super::spans;
+use super::BasicCoverageBlock;
 
 use coverage_test_macros::let_bcb;
 
@@ -644,39 +644,18 @@ fn test_traverse_coverage_with_loops() {
     );
 }
 
-fn synthesize_body_span_from_terminators(mir_body: &Body<'_>) -> Span {
-    let mut some_span: Option<Span> = None;
-    for (_, data) in mir_body.basic_blocks.iter_enumerated() {
-        let term_span = data.terminator().source_info.span;
-        if let Some(span) = some_span.as_mut() {
-            *span = span.to(term_span);
-        } else {
-            some_span = Some(term_span)
-        }
-    }
-    some_span.expect("body must have at least one BasicBlock")
-}
-
 #[test]
 fn test_make_bcb_counters() {
     rustc_span::create_default_session_globals_then(|| {
         let mir_body = goto_switchint();
-        let body_span = synthesize_body_span_from_terminators(&mir_body);
         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) = spans::filtered_terminator_span(data.terminator(&mir_body)) {
-                coverage_spans.push(spans::CoverageSpan::for_terminator(
-                    spans::function_source_span(span, body_span),
-                    span,
-                    bcb,
-                    data.last_bb(),
-                ));
-            }
-        }
+        // Historically this test would use `spans` internals to set up fake
+        // coverage spans for BCBs 1 and 2. Now we skip that step and just tell
+        // BCB counter construction that those BCBs have spans.
+        let bcb_has_coverage_spans = |bcb: BasicCoverageBlock| (1..=2).contains(&bcb.as_usize());
         let mut coverage_counters = counters::CoverageCounters::new(&basic_coverage_blocks);
         coverage_counters
-            .make_bcb_counters(&mut basic_coverage_blocks, &coverage_spans)
+            .make_bcb_counters(&mut basic_coverage_blocks, bcb_has_coverage_spans)
             .expect("should be Ok");
         assert_eq!(coverage_counters.intermediate_expressions.len(), 0);