Skip to content

Commit a702f55

Browse files
committed
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.
1 parent 327e6cf commit a702f55

File tree

4 files changed

+23
-43
lines changed

4 files changed

+23
-43
lines changed

compiler/rustc_mir_transform/src/coverage/counters.rs

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,9 @@ use super::Error;
22

33
use super::debug;
44
use super::graph;
5-
use super::spans;
65

76
use debug::{DebugCounters, NESTED_INDENT};
87
use graph::{BasicCoverageBlock, BcbBranch, CoverageGraph, TraverseCoverageGraphWithLoops};
9-
use spans::CoverageSpan;
108

119
use rustc_data_structures::fx::FxHashMap;
1210
use rustc_data_structures::graph::WithNumNodes;
@@ -108,9 +106,9 @@ impl CoverageCounters {
108106
pub fn make_bcb_counters(
109107
&mut self,
110108
basic_coverage_blocks: &CoverageGraph,
111-
coverage_spans: &[CoverageSpan],
109+
bcb_has_coverage_spans: impl Fn(BasicCoverageBlock) -> bool,
112110
) -> Result<(), Error> {
113-
MakeBcbCounters::new(self, basic_coverage_blocks).make_bcb_counters(coverage_spans)
111+
MakeBcbCounters::new(self, basic_coverage_blocks).make_bcb_counters(bcb_has_coverage_spans)
114112
}
115113

116114
fn make_counter<F>(&mut self, debug_block_label_fn: F) -> BcbCounter
@@ -270,14 +268,11 @@ impl<'a> MakeBcbCounters<'a> {
270268
/// Returns any non-code-span expressions created to represent intermediate values (such as to
271269
/// add two counters so the result can be subtracted from another counter), or an Error with
272270
/// message for subsequent debugging.
273-
fn make_bcb_counters(&mut self, coverage_spans: &[CoverageSpan]) -> Result<(), Error> {
271+
fn make_bcb_counters(
272+
&mut self,
273+
bcb_has_coverage_spans: impl Fn(BasicCoverageBlock) -> bool,
274+
) -> Result<(), Error> {
274275
debug!("make_bcb_counters(): adding a counter or expression to each BasicCoverageBlock");
275-
let num_bcbs = self.basic_coverage_blocks.num_nodes();
276-
277-
let mut bcbs_with_coverage = BitSet::new_empty(num_bcbs);
278-
for covspan in coverage_spans {
279-
bcbs_with_coverage.insert(covspan.bcb);
280-
}
281276

282277
// Walk the `CoverageGraph`. For each `BasicCoverageBlock` node with an associated
283278
// `CoverageSpan`, add a counter. If the `BasicCoverageBlock` branches, add a counter or
@@ -291,7 +286,7 @@ impl<'a> MakeBcbCounters<'a> {
291286
// the current BCB is in one or more nested loops or not.
292287
let mut traversal = TraverseCoverageGraphWithLoops::new(&self.basic_coverage_blocks);
293288
while let Some(bcb) = traversal.next(self.basic_coverage_blocks) {
294-
if bcbs_with_coverage.contains(bcb) {
289+
if bcb_has_coverage_spans(bcb) {
295290
debug!("{:?} has at least one `CoverageSpan`. Get or make its counter", bcb);
296291
let branching_counter_operand = self.get_or_make_counter_operand(bcb)?;
297292

compiler/rustc_mir_transform/src/coverage/mod.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use crate::MirPass;
1616

1717
use rustc_data_structures::graph::WithNumNodes;
1818
use rustc_data_structures::sync::Lrc;
19+
use rustc_index::bit_set::BitSet;
1920
use rustc_index::IndexVec;
2021
use rustc_middle::hir;
2122
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
@@ -196,6 +197,14 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
196197
);
197198
}
198199

200+
let bcb_has_coverage_spans = {
201+
let mut bcbs_with_coverage = BitSet::new_empty(self.basic_coverage_blocks.num_nodes());
202+
for covspan in &coverage_spans {
203+
bcbs_with_coverage.insert(covspan.bcb);
204+
}
205+
move |bcb| bcbs_with_coverage.contains(bcb)
206+
};
207+
199208
////////////////////////////////////////////////////
200209
// Create an optimized mix of `Counter`s and `Expression`s for the `CoverageGraph`. Ensure
201210
// every `CoverageSpan` has a `Counter` or `Expression` assigned to its `BasicCoverageBlock`
@@ -206,7 +215,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
206215
// direct association with any `BasicCoverageBlock`, are accumulated inside `coverage_counters`.
207216
let result = self
208217
.coverage_counters
209-
.make_bcb_counters(&mut self.basic_coverage_blocks, &coverage_spans);
218+
.make_bcb_counters(&mut self.basic_coverage_blocks, bcb_has_coverage_spans);
210219

211220
if let Ok(()) = result {
212221
// If debugging, add any intermediate expressions (which are not associated with any

compiler/rustc_mir_transform/src/coverage/spans.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -793,7 +793,7 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
793793

794794
/// If the MIR `Statement` has a span contributive to computing coverage spans,
795795
/// return it; otherwise return `None`.
796-
pub(super) fn filtered_statement_span(statement: &Statement<'_>) -> Option<Span> {
796+
fn filtered_statement_span(statement: &Statement<'_>) -> Option<Span> {
797797
match statement.kind {
798798
// These statements have spans that are often outside the scope of the executed source code
799799
// for their parent `BasicBlock`.
@@ -840,7 +840,7 @@ pub(super) fn filtered_statement_span(statement: &Statement<'_>) -> Option<Span>
840840

841841
/// If the MIR `Terminator` has a span contributive to computing coverage spans,
842842
/// return it; otherwise return `None`.
843-
pub(super) fn filtered_terminator_span(terminator: &Terminator<'_>) -> Option<Span> {
843+
fn filtered_terminator_span(terminator: &Terminator<'_>) -> Option<Span> {
844844
match terminator.kind {
845845
// These terminators have spans that don't positively contribute to computing a reasonable
846846
// 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<Sp
887887
/// [^1]Expansions result from Rust syntax including macros, syntactic sugar,
888888
/// etc.).
889889
#[inline]
890-
pub(super) fn function_source_span(span: Span, body_span: Span) -> Span {
890+
fn function_source_span(span: Span, body_span: Span) -> Span {
891891
let original_span = original_sp(span, body_span).with_ctxt(body_span.ctxt());
892892
if body_span.contains(original_span) { original_span } else { body_span }
893893
}

compiler/rustc_mir_transform/src/coverage/tests.rs

Lines changed: 3 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
2727
use super::counters;
2828
use super::graph;
29-
use super::spans;
29+
use super::BasicCoverageBlock;
3030

3131
use coverage_test_macros::let_bcb;
3232

@@ -644,39 +644,15 @@ fn test_traverse_coverage_with_loops() {
644644
);
645645
}
646646

647-
fn synthesize_body_span_from_terminators(mir_body: &Body<'_>) -> Span {
648-
let mut some_span: Option<Span> = None;
649-
for (_, data) in mir_body.basic_blocks.iter_enumerated() {
650-
let term_span = data.terminator().source_info.span;
651-
if let Some(span) = some_span.as_mut() {
652-
*span = span.to(term_span);
653-
} else {
654-
some_span = Some(term_span)
655-
}
656-
}
657-
some_span.expect("body must have at least one BasicBlock")
658-
}
659-
660647
#[test]
661648
fn test_make_bcb_counters() {
662649
rustc_span::create_default_session_globals_then(|| {
663650
let mir_body = goto_switchint();
664-
let body_span = synthesize_body_span_from_terminators(&mir_body);
665651
let mut basic_coverage_blocks = graph::CoverageGraph::from_mir(&mir_body);
666-
let mut coverage_spans = Vec::new();
667-
for (bcb, data) in basic_coverage_blocks.iter_enumerated() {
668-
if let Some(span) = spans::filtered_terminator_span(data.terminator(&mir_body)) {
669-
coverage_spans.push(spans::CoverageSpan::for_terminator(
670-
spans::function_source_span(span, body_span),
671-
span,
672-
bcb,
673-
data.last_bb(),
674-
));
675-
}
676-
}
652+
let bcb_has_coverage_spans = |bcb: BasicCoverageBlock| bcb.as_usize() > 0;
677653
let mut coverage_counters = counters::CoverageCounters::new(&basic_coverage_blocks);
678654
coverage_counters
679-
.make_bcb_counters(&mut basic_coverage_blocks, &coverage_spans)
655+
.make_bcb_counters(&mut basic_coverage_blocks, bcb_has_coverage_spans)
680656
.expect("should be Ok");
681657
assert_eq!(coverage_counters.intermediate_expressions.len(), 0);
682658

0 commit comments

Comments
 (0)