From 151da957f73ce56cf8edc7d50360534f7df0a09e Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 17 Sep 2023 22:22:21 +1000 Subject: [PATCH] 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. --- .../src/coverage/counters.rs | 19 ++++------- .../rustc_mir_transform/src/coverage/mod.rs | 12 ++++++- .../rustc_mir_transform/src/coverage/spans.rs | 6 ++-- .../rustc_mir_transform/src/coverage/tests.rs | 33 ++++--------------- 4 files changed, 27 insertions(+), 43 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/counters.rs b/compiler/rustc_mir_transform/src/coverage/counters.rs index 3d442e5dca9f9..40b4e9ec26a35 100644 --- a/compiler/rustc_mir_transform/src/coverage/counters.rs +++ b/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(&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)?; diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index d0b28eb2f5d80..54aa691f8e6da 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/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 diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index 717763a94a0e3..b9bd64489f459 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/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 { +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`. @@ -840,7 +840,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 @@ -887,7 +887,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..9a3025ac1c821 100644 --- a/compiler/rustc_mir_transform/src/coverage/tests.rs +++ b/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 = 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);