Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

coverage: Don't use actual coverage spans in test_make_bcb_counters #115912

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
12 changes: 11 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,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`
Expand All @@ -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
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 }
}
33 changes: 6 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,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);

Expand Down