From 567df37864000c27423625ad7dcd1cb0f746b65a Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 17 Sep 2023 22:22:21 +1000 Subject: [PATCH] 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 #115912, which avoid the need for a particular test to deal with coverage spans at all. (Comments/logs referring to `CoverageSpan` are updated in a subsequent patch.) --- .../src/coverage/counters.rs | 19 +++---- .../rustc_mir_transform/src/coverage/mod.rs | 13 +++-- .../rustc_mir_transform/src/coverage/spans.rs | 51 ++++++++++++++++--- .../rustc_mir_transform/src/coverage/tests.rs | 34 +++---------- 4 files changed, 63 insertions(+), 54 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/counters.rs b/compiler/rustc_mir_transform/src/coverage/counters.rs index d56d4ad4f1e7b..17e1213fefcc6 100644 --- a/compiler/rustc_mir_transform/src/coverage/counters.rs +++ b/compiler/rustc_mir_transform/src/coverage/counters.rs @@ -1,10 +1,8 @@ use super::Error; use super::graph; -use super::spans; use graph::{BasicCoverageBlock, BcbBranch, CoverageGraph, TraverseCoverageGraphWithLoops}; -use spans::CoverageSpan; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::graph::WithNumNodes; @@ -98,9 +96,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(&mut self) -> BcbCounter { @@ -234,14 +232,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 @@ -255,7 +250,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)?; diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index c75d33eeb315f..387d8c7312973 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -9,7 +9,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; @@ -170,9 +170,10 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { // // Intermediate expressions (used to compute other `Expression` values), which have no // direct association with any `BasicCoverageBlock`, are accumulated inside `coverage_counters`. + let bcb_has_coverage_spans = |bcb| coverage_spans.bcb_has_coverage_spans(bcb); 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 { //////////////////////////////////////////////////// @@ -185,7 +186,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { // These `CoverageSpan`-associated counters are removed from their associated // `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); + self.inject_coverage_span_counters(&coverage_spans); //////////////////////////////////////////////////// // For any remaining `BasicCoverageBlock` counters (that were not associated with @@ -219,16 +220,14 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { /// `bcb` to its `Counter`, when injected. Subsequent `CoverageSpan`s for a BCB that already has /// a `Counter` will inject an `Expression` instead, and compute its value by adding `ZERO` to /// the BCB `Counter` value. - fn inject_coverage_span_counters(&mut self, coverage_spans: Vec) { + fn inject_coverage_span_counters(&mut self, coverage_spans: &CoverageSpans) { let tcx = self.tcx; let source_map = tcx.sess.source_map(); let body_span = self.body_span; 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 { - let bcb = covspan.bcb; - let span = covspan.span; + for (bcb, span) in coverage_spans.bcb_span_pairs() { let counter_kind = if let Some(&counter_operand) = bcb_counters[bcb].as_ref() { self.coverage_counters.make_identity_counter(counter_operand) } else if let Some(counter_kind) = self.coverage_counters.take_bcb_counter(bcb) { diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index 767f8e9f4fa15..4741ce7994bd0 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -1,6 +1,7 @@ use super::graph::{BasicCoverageBlock, BasicCoverageBlockData, CoverageGraph, START_BCB}; use rustc_data_structures::graph::WithNumNodes; +use rustc_index::bit_set::BitSet; use rustc_middle::mir::{ self, AggregateKind, BasicBlock, FakeReadCause, Rvalue, Statement, StatementKind, Terminator, TerminatorKind, @@ -10,6 +11,42 @@ use rustc_span::{BytePos, ExpnKind, MacroKind, Span, Symbol}; use std::cell::OnceCell; +pub(super) struct CoverageSpans { + coverage_spans: Vec, + bcbs_with_coverage_spans: BitSet, +} + +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 bcb_span_pairs(&self) -> impl Iterator + '_ { + self.coverage_spans.iter().map(|&CoverageSpan { bcb, span, .. }| (bcb, span)) + } +} + #[derive(Debug, Copy, Clone)] pub(super) enum CoverageStatement { Statement(BasicBlock, Span, usize), @@ -35,7 +72,7 @@ impl CoverageStatement { /// or is subsumed by the `Span` associated with this `CoverageSpan`, and it's `BasicBlock` /// `dominates()` the `BasicBlock`s in this `CoverageSpan`. #[derive(Debug, Clone)] -pub(super) struct CoverageSpan { +struct CoverageSpan { pub span: Span, pub expn_span: Span, pub current_macro_or_none: OnceCell>, @@ -162,7 +199,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>, @@ -218,7 +255,7 @@ pub struct CoverageSpans<'a, 'tcx> { refined_spans: Vec, } -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. /// @@ -246,7 +283,7 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { body_span: Span, basic_coverage_blocks: &'a CoverageGraph, ) -> Vec { - let mut coverage_spans = CoverageSpans { + let mut coverage_spans = Self { mir_body, fn_sig_span, body_span, @@ -734,7 +771,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 { +fn filtered_statement_span(statement: &Statement<'_>) -> Option { match statement.kind { // These statements have spans that are often outside the scope of the executed source code // for their parent `BasicBlock`. @@ -781,7 +818,7 @@ pub(super) fn filtered_statement_span(statement: &Statement<'_>) -> Option /// 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 { +fn filtered_terminator_span(terminator: &Terminator<'_>) -> Option { 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 @@ -828,7 +865,7 @@ pub(super) fn filtered_terminator_span(terminator: &Terminator<'_>) -> Option 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 } } diff --git a/compiler/rustc_mir_transform/src/coverage/tests.rs b/compiler/rustc_mir_transform/src/coverage/tests.rs index 4a066ed3abdea..7476d3ce9272a 100644 --- a/compiler/rustc_mir_transform/src/coverage/tests.rs +++ b/compiler/rustc_mir_transform/src/coverage/tests.rs @@ -25,8 +25,7 @@ //! to: `rustc_span::create_default_session_globals_then(|| { test_here(); })`. use super::counters; -use super::graph; -use super::spans; +use super::graph::{self, BasicCoverageBlock}; use coverage_test_macros::let_bcb; @@ -644,39 +643,18 @@ fn test_traverse_coverage_with_loops() { ); } -fn synthesize_body_span_from_terminators(mir_body: &Body<'_>) -> Span { - let mut some_span: Option = 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);