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

Add a new CoverageKind::Branch to MIR #115061

Closed
wants to merge 16 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
27 changes: 25 additions & 2 deletions compiler/rustc_mir_transform/src/coverage/counters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,31 @@ impl CoverageCounters {
&mut self,
basic_coverage_blocks: &CoverageGraph,
coverage_spans: &[CoverageSpan],
) -> Result<(), Error> {
MakeBcbCounters::new(self, basic_coverage_blocks).make_bcb_counters(coverage_spans)
) -> Result<Vec<(BasicCoverageBlock, CoverageKind)>, Error> {
let mut make_counters = MakeBcbCounters::new(self, basic_coverage_blocks);
make_counters.make_bcb_counters(coverage_spans)?;

let branches: Vec<_> = basic_coverage_blocks
.iter_enumerated()
.filter_map(|(bcb, _data)| {
let branches = make_counters.bcb_branches(bcb);
if branches.len() != 2 {
return None;
}
let mut branch_counters =
branches.iter().filter_map(|branch| make_counters.branch_counter(branch));

// FIXME(swatinem): figure out what the correct ordering here is?
// In simple testing, the `false` (aka `0`) branch of `SwitchInt` seems to be first.
let false_ = branch_counters.next()?.as_operand();
let true_ = branch_counters.next()?.as_operand();

let branch = CoverageKind::Branch { true_, false_ };
Some((bcb, branch))
})
.collect();

Ok(branches)
}

fn make_counter<F>(&mut self, debug_block_label_fn: F) -> CoverageKind
Expand Down
27 changes: 25 additions & 2 deletions compiler/rustc_mir_transform/src/coverage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,11 +202,11 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
//
// Intermediate expressions (used to compute other `Expression` values), which have no
// direct association with any `BasicCoverageBlock`, are accumulated inside `coverage_counters`.
let result = self
let mut result = self
.coverage_counters
.make_bcb_counters(&mut self.basic_coverage_blocks, &coverage_spans);

if let Ok(()) = result {
if let Ok(branches) = result.as_mut() {
// If debugging, add any intermediate expressions (which are not associated with any
// BCB) to the `debug_used_expressions` map.
if debug_used_expressions.is_enabled() {
Expand All @@ -231,6 +231,8 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
&mut debug_used_expressions,
);

self.inject_branch_counters(std::mem::take(branches));

////////////////////////////////////////////////////
// For any remaining `BasicCoverageBlock` counters (that were not associated with
// any `CoverageSpan`), inject `Coverage` statements (_without_ code region `Span`s)
Expand Down Expand Up @@ -275,6 +277,27 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
}
}

fn inject_branch_counters(&mut self, branches: Vec<(BasicCoverageBlock, CoverageKind)>) {
let tcx = self.tcx;
let source_map = tcx.sess.source_map();
let body_span = self.body_span;
let file_name = Symbol::intern(&self.source_file.name.prefer_remapped().to_string_lossy());

for branch in branches {
let bcb_data = self.bcb_data(branch.0);
let terminator = bcb_data.terminator(self.mir_body);

let span = terminator.source_info.span;

// FIXME(swatinem): figure out what a good span for the conditional is.
// For trivial code examples, the `terminator` seems to be sufficient.
let code_region =
Some(make_code_region(source_map, file_name, &self.source_file, span, body_span));

inject_statement(self.mir_body, branch.1, bcb_data.last_bb(), code_region);
}
}

/// Inject a counter for each `CoverageSpan`. There can be multiple `CoverageSpan`s for a given
/// BCB, but only one actual counter needs to be incremented per BCB. `bb_counters` maps each
/// `bcb` to its `Counter`, when injected. Subsequent `CoverageSpan`s for a BCB that already has
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_transform/src/coverage/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -676,7 +676,7 @@ fn test_make_bcb_counters() {
}
}
let mut coverage_counters = counters::CoverageCounters::new(0, &basic_coverage_blocks);
let () = coverage_counters
let _ = coverage_counters
.make_bcb_counters(&mut basic_coverage_blocks, &coverage_spans)
.expect("should be Ok");
assert_eq!(coverage_counters.intermediate_expressions.len(), 0);
Expand Down
15 changes: 11 additions & 4 deletions src/tools/compiletest/src/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ use crate::ColorConfig;
use regex::{Captures, Regex};
use rustfix::{apply_suggestions, get_suggestions_from_json, Filter};

use std::borrow::Cow;
use std::collections::hash_map::DefaultHasher;
use std::collections::{HashMap, HashSet};
use std::env;
Expand Down Expand Up @@ -564,7 +563,12 @@ impl<'test> TestCx<'test> {

// Run `llvm-cov show` to produce a coverage report in text format.
let proc_res = self.run_llvm_tool("llvm-cov", |cmd| {
cmd.args(["show", "--format=text", "--show-line-counts-or-regions"]);
cmd.args([
"show",
"--format=text",
"--show-line-counts-or-regions",
"--show-branches=count",
]);

cmd.arg("--Xdemangler");
cmd.arg(self.config.rust_demangler_path.as_ref().unwrap());
Expand Down Expand Up @@ -723,7 +727,7 @@ impl<'test> TestCx<'test> {

/// Replace line numbers in coverage reports with the placeholder `LL`,
/// so that the tests are less sensitive to lines being added/removed.
fn anonymize_coverage_line_numbers(coverage: &str) -> Cow<'_, str> {
fn anonymize_coverage_line_numbers(line: &str) -> String {
Swatinem marked this conversation as resolved.
Show resolved Hide resolved
// The coverage reporter prints line numbers at the start of a line.
// They are truncated or left-padded to occupy exactly 5 columns.
// (`LineNumberColumnWidth` in `SourceCoverageViewText.cpp`.)
Expand All @@ -733,7 +737,10 @@ impl<'test> TestCx<'test> {
// have an additional prefix of ` |` for each nesting level.
static LINE_NUMBER_RE: Lazy<Regex> =
Lazy::new(|| Regex::new(r"(?m:^)(?<prefix>(?: \|)*) *[0-9]+\|").unwrap());
LINE_NUMBER_RE.replace_all(coverage, "$prefix LL|")
static BRANCH_LINE_RE: Lazy<Regex> =
Lazy::new(|| Regex::new(r"(?m:^)(?<prefix>(?: \|)*) Branch \([0-9]+:").unwrap());
let line = LINE_NUMBER_RE.replace_all(line, "$prefix LL|");
BRANCH_LINE_RE.replace_all(&line, "$prefix Branch (LL:").into_owned()
}

/// Coverage reports can describe multiple source files, separated by
Expand Down
21 changes: 17 additions & 4 deletions src/tools/coverage-dump/src/covfun.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,9 @@ pub(crate) fn dump_covfun_mappings(
// If the mapping contains expressions, also print the resolved
// form of those expressions
kind.for_each_operand(|label, operand| {
if matches!(operand, Operand::Expression { .. }) {
if matches!(operand, Operand::Expression { .. })
|| matches!(kind, MappingKind::Branch { .. })
{
let pad = if label.is_empty() { "" } else { " " };
let resolved = expression_resolver.format_operand(operand);
println!(" {label}{pad}= {resolved}");
Expand Down Expand Up @@ -207,7 +209,6 @@ impl Operand {
}
}

#[derive(Debug)]
enum MappingKind {
Code(Operand),
Gap(Operand),
Expand All @@ -216,6 +217,18 @@ enum MappingKind {
Branch { true_: Operand, false_: Operand },
}

impl Debug for MappingKind {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Self::Code(operand) => f.debug_tuple("Code").field(operand).finish(),
Self::Gap(operand) => f.debug_tuple("Gap").field(operand).finish(),
Self::Expansion(expansion) => f.debug_tuple("Expansion").field(expansion).finish(),
Self::Skip => write!(f, "Skip"),
Self::Branch { .. } => f.debug_tuple("Branch").finish(),
}
}
}

impl MappingKind {
/// Visits each operand directly contained in this mapping, along with
/// a string label (possibly empty).
Expand All @@ -226,8 +239,8 @@ impl MappingKind {
Self::Expansion(_) => (),
Self::Skip => (),
Self::Branch { true_, false_ } => {
func("true_ ", true_);
func("false_", false_);
func("true ", true_);
func("false", false_);
}
}
}
Expand Down