Skip to content

Commit

Permalink
coverage: Don't use actual coverage spans in test_make_bcb_counters
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Zalathar committed Sep 17, 2023
1 parent 327e6cf commit a702f55
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 43 deletions.
19 changes: 7 additions & 12 deletions compiler/rustc_mir_transform/src/coverage/counters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)?;

Expand Down
11 changes: 10 additions & 1 deletion compiler/rustc_mir_transform/src/coverage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -196,6 +197,14 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
);
}

let bcb_has_coverage_spans = {
let mut bcbs_with_coverage = BitSet::new_empty(self.basic_coverage_blocks.num_nodes());
for covspan in &coverage_spans {
bcbs_with_coverage.insert(covspan.bcb);
}
move |bcb| bcbs_with_coverage.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`
Expand All @@ -206,7 +215,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
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_mir_transform/src/coverage/spans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 }
}
30 changes: 3 additions & 27 deletions compiler/rustc_mir_transform/src/coverage/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@

use super::counters;
use super::graph;
use super::spans;
use super::BasicCoverageBlock;

use coverage_test_macros::let_bcb;

Expand Down Expand Up @@ -644,39 +644,15 @@ 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(),
));
}
}
let bcb_has_coverage_spans = |bcb: BasicCoverageBlock| bcb.as_usize() > 0;
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);

Expand Down

0 comments on commit a702f55

Please sign in to comment.