1
Fork 0

coverage: Rename mir::coverage::BranchInfo to CoverageInfoHi

This opens the door to collecting and storing coverage information that is
unrelated to branch coverage or MC/DC.
This commit is contained in:
Zalathar 2024-07-04 23:52:49 +10:00
parent 489233170a
commit f095de4bf1
9 changed files with 110 additions and 85 deletions

View file

@ -2,7 +2,7 @@ use std::assert_matches::assert_matches;
use std::collections::hash_map::Entry;
use rustc_data_structures::fx::FxHashMap;
use rustc_middle::mir::coverage::{BlockMarkerId, BranchSpan, CoverageKind};
use rustc_middle::mir::coverage::{BlockMarkerId, BranchSpan, CoverageInfoHi, CoverageKind};
use rustc_middle::mir::{self, BasicBlock, SourceInfo, UnOp};
use rustc_middle::thir::{ExprId, ExprKind, Pat, Thir};
use rustc_middle::ty::TyCtxt;
@ -13,16 +13,25 @@ use crate::build::{Builder, CFG};
mod mcdc;
pub(crate) struct BranchInfoBuilder {
/// Collects coverage-related information during MIR building, to eventually be
/// turned into a function's [`CoverageInfoHi`] when MIR building is complete.
pub(crate) struct CoverageInfoBuilder {
/// Maps condition expressions to their enclosing `!`, for better instrumentation.
nots: FxHashMap<ExprId, NotInfo>,
markers: BlockMarkerGen,
branch_spans: Vec<BranchSpan>,
/// Present if branch coverage is enabled.
branch_info: Option<BranchInfo>,
/// Present if MC/DC coverage is enabled.
mcdc_info: Option<MCDCInfoBuilder>,
}
#[derive(Default)]
struct BranchInfo {
branch_spans: Vec<BranchSpan>,
}
#[derive(Clone, Copy)]
struct NotInfo {
/// When visiting the associated expression as a branch condition, treat this
@ -62,20 +71,20 @@ impl BlockMarkerGen {
}
}
impl BranchInfoBuilder {
/// Creates a new branch info builder, but only if branch coverage instrumentation
impl CoverageInfoBuilder {
/// Creates a new coverage info builder, but only if coverage instrumentation
/// is enabled and `def_id` represents a function that is eligible for coverage.
pub(crate) fn new_if_enabled(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Option<Self> {
if tcx.sess.instrument_coverage_branch() && tcx.is_eligible_for_coverage(def_id) {
Some(Self {
nots: FxHashMap::default(),
markers: BlockMarkerGen::default(),
branch_spans: vec![],
mcdc_info: tcx.sess.instrument_coverage_mcdc().then(MCDCInfoBuilder::new),
})
} else {
None
if !tcx.sess.instrument_coverage() || !tcx.is_eligible_for_coverage(def_id) {
return None;
}
Some(Self {
nots: FxHashMap::default(),
markers: BlockMarkerGen::default(),
branch_info: tcx.sess.instrument_coverage_branch().then(BranchInfo::default),
mcdc_info: tcx.sess.instrument_coverage_mcdc().then(MCDCInfoBuilder::new),
})
}
/// Unary `!` expressions inside an `if` condition are lowered by lowering
@ -88,6 +97,12 @@ impl BranchInfoBuilder {
pub(crate) fn visit_unary_not(&mut self, thir: &Thir<'_>, unary_not: ExprId) {
assert_matches!(thir[unary_not].kind, ExprKind::Unary { op: UnOp::Not, .. });
// The information collected by this visitor is only needed when branch
// coverage or higher is enabled.
if self.branch_info.is_none() {
return;
}
self.visit_with_not_info(
thir,
unary_not,
@ -137,40 +152,40 @@ impl BranchInfoBuilder {
false_block,
inject_block_marker,
);
} else {
let true_marker = self.markers.inject_block_marker(cfg, source_info, true_block);
let false_marker = self.markers.inject_block_marker(cfg, source_info, false_block);
self.branch_spans.push(BranchSpan {
span: source_info.span,
true_marker,
false_marker,
});
return;
}
// Bail out if branch coverage is not enabled.
let Some(branch_info) = self.branch_info.as_mut() else { return };
let true_marker = self.markers.inject_block_marker(cfg, source_info, true_block);
let false_marker = self.markers.inject_block_marker(cfg, source_info, false_block);
branch_info.branch_spans.push(BranchSpan {
span: source_info.span,
true_marker,
false_marker,
});
}
pub(crate) fn into_done(self) -> Option<Box<mir::coverage::BranchInfo>> {
let Self {
nots: _,
markers: BlockMarkerGen { num_block_markers },
branch_spans,
mcdc_info,
} = self;
pub(crate) fn into_done(self) -> Box<CoverageInfoHi> {
let Self { nots: _, markers: BlockMarkerGen { num_block_markers }, branch_info, mcdc_info } =
self;
if num_block_markers == 0 {
assert!(branch_spans.is_empty());
return None;
}
let branch_spans =
branch_info.map(|branch_info| branch_info.branch_spans).unwrap_or_default();
let (mcdc_decision_spans, mcdc_branch_spans) =
mcdc_info.map(MCDCInfoBuilder::into_done).unwrap_or_default();
Some(Box::new(mir::coverage::BranchInfo {
// For simplicity, always return an info struct (without Option), even
// if there's nothing interesting in it.
Box::new(CoverageInfoHi {
num_block_markers,
branch_spans,
mcdc_branch_spans,
mcdc_decision_spans,
}))
})
}
}
@ -184,7 +199,7 @@ impl<'tcx> Builder<'_, 'tcx> {
block: &mut BasicBlock,
) {
// Bail out if condition coverage is not enabled for this function.
let Some(branch_info) = self.coverage_branch_info.as_mut() else { return };
let Some(coverage_info) = self.coverage_info.as_mut() else { return };
if !self.tcx.sess.instrument_coverage_condition() {
return;
};
@ -224,7 +239,7 @@ impl<'tcx> Builder<'_, 'tcx> {
);
// Separate path for handling branches when MC/DC is enabled.
branch_info.register_two_way_branch(
coverage_info.register_two_way_branch(
self.tcx,
&mut self.cfg,
source_info,
@ -247,12 +262,12 @@ impl<'tcx> Builder<'_, 'tcx> {
mut then_block: BasicBlock,
mut else_block: BasicBlock,
) {
// Bail out if branch coverage is not enabled for this function.
let Some(branch_info) = self.coverage_branch_info.as_mut() else { return };
// Bail out if coverage is not enabled for this function.
let Some(coverage_info) = self.coverage_info.as_mut() else { return };
// If this condition expression is nested within one or more `!` expressions,
// replace it with the enclosing `!` collected by `visit_unary_not`.
if let Some(&NotInfo { enclosing_not, is_flipped }) = branch_info.nots.get(&expr_id) {
if let Some(&NotInfo { enclosing_not, is_flipped }) = coverage_info.nots.get(&expr_id) {
expr_id = enclosing_not;
if is_flipped {
std::mem::swap(&mut then_block, &mut else_block);
@ -261,7 +276,7 @@ impl<'tcx> Builder<'_, 'tcx> {
let source_info = SourceInfo { span: self.thir[expr_id].span, scope: self.source_scope };
branch_info.register_two_way_branch(
coverage_info.register_two_way_branch(
self.tcx,
&mut self.cfg,
source_info,
@ -280,13 +295,11 @@ impl<'tcx> Builder<'_, 'tcx> {
true_block: BasicBlock,
false_block: BasicBlock,
) {
// Bail out if branch coverage is not enabled for this function.
let Some(branch_info) = self.coverage_branch_info.as_mut() else { return };
// FIXME(#124144) This may need special handling when MC/DC is enabled.
// Bail out if coverage is not enabled for this function.
let Some(coverage_info) = self.coverage_info.as_mut() else { return };
let source_info = SourceInfo { span: pattern.span, scope: self.source_scope };
branch_info.register_two_way_branch(
coverage_info.register_two_way_branch(
self.tcx,
&mut self.cfg,
source_info,

View file

@ -250,24 +250,24 @@ impl MCDCInfoBuilder {
impl Builder<'_, '_> {
pub(crate) fn visit_coverage_branch_operation(&mut self, logical_op: LogicalOp, span: Span) {
if let Some(branch_info) = self.coverage_branch_info.as_mut()
&& let Some(mcdc_info) = branch_info.mcdc_info.as_mut()
if let Some(coverage_info) = self.coverage_info.as_mut()
&& let Some(mcdc_info) = coverage_info.mcdc_info.as_mut()
{
mcdc_info.state.record_conditions(logical_op, span);
}
}
pub(crate) fn mcdc_increment_depth_if_enabled(&mut self) {
if let Some(branch_info) = self.coverage_branch_info.as_mut()
&& let Some(mcdc_info) = branch_info.mcdc_info.as_mut()
if let Some(coverage_info) = self.coverage_info.as_mut()
&& let Some(mcdc_info) = coverage_info.mcdc_info.as_mut()
{
mcdc_info.state.decision_ctx_stack.push(MCDCDecisionCtx::default());
};
}
pub(crate) fn mcdc_decrement_depth_if_enabled(&mut self) {
if let Some(branch_info) = self.coverage_branch_info.as_mut()
&& let Some(mcdc_info) = branch_info.mcdc_info.as_mut()
if let Some(coverage_info) = self.coverage_info.as_mut()
&& let Some(mcdc_info) = coverage_info.mcdc_info.as_mut()
{
if mcdc_info.state.decision_ctx_stack.pop().is_none() {
bug!("Unexpected empty decision stack");

View file

@ -62,7 +62,7 @@ pub(super) fn build_custom_mir<'tcx>(
tainted_by_errors: None,
injection_phase: None,
pass_count: 0,
coverage_branch_info: None,
coverage_info_hi: None,
function_coverage_info: None,
};

View file

@ -160,8 +160,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// Improve branch coverage instrumentation by noting conditions
// nested within one or more `!` expressions.
// (Skipped if branch coverage is not enabled.)
if let Some(branch_info) = this.coverage_branch_info.as_mut() {
branch_info.visit_unary_not(this.thir, expr_id);
if let Some(coverage_info) = this.coverage_info.as_mut() {
coverage_info.visit_unary_not(this.thir, expr_id);
}
let local_scope = this.local_scope();

View file

@ -218,8 +218,8 @@ struct Builder<'a, 'tcx> {
lint_level_roots_cache: GrowableBitSet<hir::ItemLocalId>,
/// Collects additional coverage information during MIR building.
/// Only present if branch coverage is enabled and this function is eligible.
coverage_branch_info: Option<coverageinfo::BranchInfoBuilder>,
/// Only present if coverage is enabled and this function is eligible.
coverage_info: Option<coverageinfo::CoverageInfoBuilder>,
}
type CaptureMap<'tcx> = SortedIndexMultiMap<usize, HirId, Capture<'tcx>>;
@ -773,7 +773,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
unit_temp: None,
var_debug_info: vec![],
lint_level_roots_cache: GrowableBitSet::new_empty(),
coverage_branch_info: coverageinfo::BranchInfoBuilder::new_if_enabled(tcx, def),
coverage_info: coverageinfo::CoverageInfoBuilder::new_if_enabled(tcx, def),
};
assert_eq!(builder.cfg.start_new_block(), START_BLOCK);
@ -802,7 +802,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
self.coroutine,
None,
);
body.coverage_branch_info = self.coverage_branch_info.and_then(|b| b.into_done());
body.coverage_info_hi = self.coverage_info.map(|b| b.into_done());
body
}