Skip to content

Commit

Permalink
coverage: Encapsulate coverage spans
Browse files Browse the repository at this point in the history
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 rust-lang#115912, which avoid
the need for a particular test to deal with coverage spans at all.
  • Loading branch information
Zalathar committed Sep 19, 2023
1 parent 3d32c98 commit 439a48a
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 54 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
8 changes: 4 additions & 4 deletions compiler/rustc_mir_transform/src/coverage/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ use rustc_span::Span;

use super::counters::{BcbCounter, CoverageCounters};
use super::graph::{BasicCoverageBlock, BasicCoverageBlockData, CoverageGraph};
use super::spans::CoverageSpan;
use super::spans::{CoverageSpan, CoverageSpans};

pub const NESTED_INDENT: &str = " ";

Expand Down Expand Up @@ -614,7 +614,7 @@ pub(super) fn dump_coverage_spanview<'tcx>(
basic_coverage_blocks: &CoverageGraph,
pass_name: &str,
body_span: Span,
coverage_spans: &[CoverageSpan],
coverage_spans: &CoverageSpans,
) {
let mir_source = mir_body.source;
let def_id = mir_source.def_id();
Expand All @@ -634,10 +634,10 @@ fn span_viewables<'tcx>(
tcx: TyCtxt<'tcx>,
mir_body: &mir::Body<'tcx>,
basic_coverage_blocks: &CoverageGraph,
coverage_spans: &[CoverageSpan],
coverage_spans: &CoverageSpans,
) -> Vec<SpanViewable> {
let mut span_viewables = Vec::new();
for coverage_span in coverage_spans {
for coverage_span in coverage_spans.iter() {
let tooltip = coverage_span.format_coverage_statements(tcx, mir_body);
let CoverageSpan { span, bcb, .. } = coverage_span;
let bcb_data = &basic_coverage_blocks[*bcb];
Expand Down
12 changes: 7 additions & 5 deletions compiler/rustc_mir_transform/src/coverage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,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;

Expand Down Expand Up @@ -196,6 +196,8 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
);
}

let bcb_has_coverage_spans = |bcb| coverage_spans.bcb_has_coverage_spans(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 +208,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 All @@ -228,7 +230,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
// `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,
&coverage_spans,
&mut graphviz_data,
&mut debug_used_expressions,
);
Expand Down Expand Up @@ -290,7 +292,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
/// `used_expression_operands` map.
fn inject_coverage_span_counters(
&mut self,
coverage_spans: Vec<CoverageSpan>,
coverage_spans: &CoverageSpans,
graphviz_data: &mut debug::GraphvizData,
debug_used_expressions: &mut debug::UsedExpressions,
) {
Expand All @@ -300,7 +302,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
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 {
for covspan in coverage_spans.iter() {
let bcb = covspan.bcb;
let span = covspan.span;
let counter_kind = if let Some(&counter_operand) = bcb_counters[bcb].as_ref() {
Expand Down
49 changes: 43 additions & 6 deletions compiler/rustc_mir_transform/src/coverage/spans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use super::graph::{BasicCoverageBlock, BasicCoverageBlockData, CoverageGraph, ST

use itertools::Itertools;
use rustc_data_structures::graph::WithNumNodes;
use rustc_index::bit_set::BitSet;
use rustc_middle::mir::spanview::source_range_no_file;
use rustc_middle::mir::{
self, AggregateKind, BasicBlock, FakeReadCause, Rvalue, Statement, StatementKind, Terminator,
Expand All @@ -13,6 +14,42 @@ use rustc_span::{BytePos, ExpnKind, MacroKind, Span, Symbol};

use std::cell::OnceCell;

pub(super) struct CoverageSpans {
coverage_spans: Vec<CoverageSpan>,
bcbs_with_coverage_spans: BitSet<BasicCoverageBlock>,
}

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 iter(&self) -> impl Iterator<Item = &CoverageSpan> {
self.coverage_spans.iter()
}
}

#[derive(Debug, Copy, Clone)]
pub(super) enum CoverageStatement {
Statement(BasicBlock, Span, usize),
Expand Down Expand Up @@ -211,7 +248,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>,

Expand Down Expand Up @@ -267,7 +304,7 @@ pub struct CoverageSpans<'a, 'tcx> {
refined_spans: Vec<CoverageSpan>,
}

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.
///
Expand Down Expand Up @@ -295,7 +332,7 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
body_span: Span,
basic_coverage_blocks: &'a CoverageGraph,
) -> Vec<CoverageSpan> {
let mut coverage_spans = CoverageSpans {
let mut coverage_spans = Self {
mir_body,
fn_sig_span,
body_span,
Expand Down Expand Up @@ -783,7 +820,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 @@ -830,7 +867,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 @@ -877,7 +914,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

0 comments on commit 439a48a

Please sign in to comment.