coverage: Encapsulate coverage spans

By encapsulating the coverage spans in a struct, we can change the internal
representation without disturbing existing call sites. This will be useful for
grouping coverage spans by BCB.

This patch includes some changes that were originally in rust-lang#115912, which avoid
the need for a particular test to deal with coverage spans at all.

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/debug.rs

@@ -124,7 +124,7 @@ use rustc_span::Span;
 
 use super::counters::{BcbCounter, CoverageCounters};
 use super::graph::{BasicCoverageBlock, BasicCoverageBlockData, CoverageGraph};
-use super::spans::CoverageSpan;
+use super::spans::{CoverageSpan, CoverageSpans};
 
 pub const NESTED_INDENT: &str = "    ";
 
@@ -614,7 +614,7 @@ pub(super) fn dump_coverage_spanview<'tcx>(
     basic_coverage_blocks: &CoverageGraph,
     pass_name: &str,
     body_span: Span,
-    coverage_spans: &[CoverageSpan],
+    coverage_spans: &CoverageSpans,
 ) {
     let mir_source = mir_body.source;
     let def_id = mir_source.def_id();
@@ -634,10 +634,10 @@ fn span_viewables<'tcx>(
     tcx: TyCtxt<'tcx>,
     mir_body: &mir::Body<'tcx>,
     basic_coverage_blocks: &CoverageGraph,
-    coverage_spans: &[CoverageSpan],
+    coverage_spans: &CoverageSpans,
 ) -> Vec<SpanViewable> {
     let mut span_viewables = Vec::new();
-    for coverage_span in coverage_spans {
+    for coverage_span in coverage_spans.iter() {
         let tooltip = coverage_span.format_coverage_statements(tcx, mir_body);
         let CoverageSpan { span, bcb, .. } = coverage_span;
         let bcb_data = &basic_coverage_blocks[*bcb];

compiler/rustc_mir_transform/src/coverage/mod.rs

@@ -10,7 +10,7 @@ mod tests;
 
 use self::counters::{BcbCounter, CoverageCounters};
 use self::graph::{BasicCoverageBlock, BasicCoverageBlockData, CoverageGraph};
-use self::spans::{CoverageSpan, CoverageSpans};
+use self::spans::CoverageSpans;
 
 use crate::MirPass;
 
@@ -196,6 +196,8 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
             );
         }
 
+        let bcb_has_coverage_spans = |bcb| coverage_spans.bcb_has_coverage_spans(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 +208,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
@@ -228,7 +230,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
             // `BasicCoverageBlock`s so that the only remaining counters in the `CoverageGraph`
             // are indirect counters (to be injected next, without associated code regions).
             self.inject_coverage_span_counters(
-                coverage_spans,
+                &coverage_spans,
                 &mut graphviz_data,
                 &mut debug_used_expressions,
             );
@@ -290,7 +292,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
     /// `used_expression_operands` map.
     fn inject_coverage_span_counters(
         &mut self,
-        coverage_spans: Vec<CoverageSpan>,
+        coverage_spans: &CoverageSpans,
         graphviz_data: &mut debug::GraphvizData,
         debug_used_expressions: &mut debug::UsedExpressions,
     ) {
@@ -300,7 +302,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
         let file_name = Symbol::intern(&self.source_file.name.prefer_remapped().to_string_lossy());
 
         let mut bcb_counters = IndexVec::from_elem_n(None, self.basic_coverage_blocks.num_nodes());
-        for covspan in coverage_spans {
+        for covspan in coverage_spans.iter() {
             let bcb = covspan.bcb;
             let span = covspan.span;
             let counter_kind = if let Some(&counter_operand) = bcb_counters[bcb].as_ref() {

compiler/rustc_mir_transform/src/coverage/spans.rs

@@ -2,6 +2,7 @@ use super::graph::{BasicCoverageBlock, BasicCoverageBlockData, CoverageGraph, ST
 
 use itertools::Itertools;
 use rustc_data_structures::graph::WithNumNodes;
+use rustc_index::bit_set::BitSet;
 use rustc_middle::mir::spanview::source_range_no_file;
 use rustc_middle::mir::{
     self, AggregateKind, BasicBlock, FakeReadCause, Rvalue, Statement, StatementKind, Terminator,
@@ -13,6 +14,42 @@ use rustc_span::{BytePos, ExpnKind, MacroKind, Span, Symbol};
 
 use std::cell::OnceCell;
 
+pub(super) struct CoverageSpans {
+    coverage_spans: Vec<CoverageSpan>,
+    bcbs_with_coverage_spans: BitSet<BasicCoverageBlock>,
+}
+
+impl CoverageSpans {
+    pub(super) fn generate_coverage_spans(
+        mir_body: &mir::Body<'_>,
+        fn_sig_span: Span,
+        body_span: Span,
+        basic_coverage_blocks: &CoverageGraph,
+    ) -> Self {
+        let coverage_spans = CoverageSpansGenerator::generate_coverage_spans(
+            mir_body,
+            fn_sig_span,
+            body_span,
+            basic_coverage_blocks,
+        );
+
+        let mut bcbs_with_coverage_spans = BitSet::new_empty(basic_coverage_blocks.num_nodes());
+        for coverage_span in &coverage_spans {
+            bcbs_with_coverage_spans.insert(coverage_span.bcb);
+        }
+
+        Self { coverage_spans, bcbs_with_coverage_spans }
+    }
+
+    pub(super) fn bcb_has_coverage_spans(&self, bcb: BasicCoverageBlock) -> bool {
+        self.bcbs_with_coverage_spans.contains(bcb)
+    }
+
+    pub(super) fn iter(&self) -> impl Iterator<Item = &CoverageSpan> {
+        self.coverage_spans.iter()
+    }
+}
+
 #[derive(Debug, Copy, Clone)]
 pub(super) enum CoverageStatement {
     Statement(BasicBlock, Span, usize),
@@ -211,7 +248,7 @@ impl CoverageSpan {
 ///  * Merge spans that represent continuous (both in source code and control flow), non-branching
 ///    execution
 ///  * Carve out (leave uncovered) any span that will be counted by another MIR (notably, closures)
-pub struct CoverageSpans<'a, 'tcx> {
+struct CoverageSpansGenerator<'a, 'tcx> {
     /// The MIR, used to look up `BasicBlockData`.
     mir_body: &'a mir::Body<'tcx>,
 
@@ -267,7 +304,7 @@ pub struct CoverageSpans<'a, 'tcx> {
     refined_spans: Vec<CoverageSpan>,
 }
 
-impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
+impl<'a, 'tcx> CoverageSpansGenerator<'a, 'tcx> {
     /// Generate a minimal set of `CoverageSpan`s, each representing a contiguous code region to be
     /// counted.
     ///
@@ -295,7 +332,7 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
         body_span: Span,
         basic_coverage_blocks: &'a CoverageGraph,
     ) -> Vec<CoverageSpan> {
-        let mut coverage_spans = CoverageSpans {
+        let mut coverage_spans = Self {
             mir_body,
             fn_sig_span,
             body_span,
@@ -783,7 +820,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`.
@@ -830,7 +867,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
@@ -877,7 +914,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);