Skip to content

Commit 82b2f99

Browse files
rust-timerrust-timer
authored
Unrolled build for rust-lang#127352
Rollup merge of rust-lang#127352 - Zalathar:coverage-info, r=oli-obk 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, during MIR building. There is no change to the output of coverage instrumentation, but one deliberate change is that functions now *always* have an attached `CoverageInfoHi` (if coverage is enabled and they are eligible), even if they didn't collect any interesting branch information. --- `@rustbot` label +A-code-coverage
2 parents 2ad6630 + f96f443 commit 82b2f99

File tree

9 files changed

+117
-87
lines changed

9 files changed

+117
-87
lines changed

‎compiler/rustc_middle/src/mir/coverage.rs

+8-3
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ pub enum CoverageKind {
103103
SpanMarker,
104104

105105
/// Marks its enclosing basic block with an ID that can be referred to by
106-
/// side data in [`BranchInfo`].
106+
/// side data in [`CoverageInfoHi`].
107107
///
108108
/// Should be erased before codegen (at some point after `InstrumentCoverage`).
109109
BlockMarker { id: BlockMarkerId },
@@ -274,10 +274,15 @@ pub struct FunctionCoverageInfo {
274274
pub mcdc_num_condition_bitmaps: usize,
275275
}
276276

277-
/// Branch information recorded during THIR-to-MIR lowering, and stored in MIR.
277+
/// Coverage information for a function, recorded during MIR building and
278+
/// attached to the corresponding `mir::Body`. Used by the `InstrumentCoverage`
279+
/// MIR pass.
280+
///
281+
/// ("Hi" indicates that this is "high-level" information collected at the
282+
/// THIR/MIR boundary, before the MIR-based coverage instrumentation pass.)
278283
#[derive(Clone, Debug)]
279284
#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
280-
pub struct BranchInfo {
285+
pub struct CoverageInfoHi {
281286
/// 1 more than the highest-numbered [`CoverageKind::BlockMarker`] that was
282287
/// injected into the MIR body. This makes it possible to allocate per-ID
283288
/// data structures without having to scan the entire body first.

‎compiler/rustc_middle/src/mir/mod.rs

+7-6
Original file line numberDiff line numberDiff line change
@@ -430,11 +430,12 @@ pub struct Body<'tcx> {
430430

431431
pub tainted_by_errors: Option<ErrorGuaranteed>,
432432

433-
/// Branch coverage information collected during MIR building, to be used by
434-
/// the `InstrumentCoverage` pass.
433+
/// Coverage information collected from THIR/MIR during MIR building,
434+
/// to be used by the `InstrumentCoverage` pass.
435435
///
436-
/// Only present if branch coverage is enabled and this function is eligible.
437-
pub coverage_branch_info: Option<Box<coverage::BranchInfo>>,
436+
/// Only present if coverage is enabled and this function is eligible.
437+
/// Boxed to limit space overhead in non-coverage builds.
438+
pub coverage_info_hi: Option<Box<coverage::CoverageInfoHi>>,
438439

439440
/// Per-function coverage information added by the `InstrumentCoverage`
440441
/// pass, to be used in conjunction with the coverage statements injected
@@ -484,7 +485,7 @@ impl<'tcx> Body<'tcx> {
484485
is_polymorphic: false,
485486
injection_phase: None,
486487
tainted_by_errors,
487-
coverage_branch_info: None,
488+
coverage_info_hi: None,
488489
function_coverage_info: None,
489490
};
490491
body.is_polymorphic = body.has_non_region_param();
@@ -515,7 +516,7 @@ impl<'tcx> Body<'tcx> {
515516
is_polymorphic: false,
516517
injection_phase: None,
517518
tainted_by_errors: None,
518-
coverage_branch_info: None,
519+
coverage_info_hi: None,
519520
function_coverage_info: None,
520521
};
521522
body.is_polymorphic = body.has_non_region_param();

‎compiler/rustc_middle/src/mir/pretty.rs

+17-8
Original file line numberDiff line numberDiff line change
@@ -473,8 +473,8 @@ pub fn write_mir_intro<'tcx>(
473473
// Add an empty line before the first block is printed.
474474
writeln!(w)?;
475475

476-
if let Some(branch_info) = &body.coverage_branch_info {
477-
write_coverage_branch_info(branch_info, w)?;
476+
if let Some(coverage_info_hi) = &body.coverage_info_hi {
477+
write_coverage_info_hi(coverage_info_hi, w)?;
478478
}
479479
if let Some(function_coverage_info) = &body.function_coverage_info {
480480
write_function_coverage_info(function_coverage_info, w)?;
@@ -483,18 +483,26 @@ pub fn write_mir_intro<'tcx>(
483483
Ok(())
484484
}
485485

486-
fn write_coverage_branch_info(
487-
branch_info: &coverage::BranchInfo,
486+
fn write_coverage_info_hi(
487+
coverage_info_hi: &coverage::CoverageInfoHi,
488488
w: &mut dyn io::Write,
489489
) -> io::Result<()> {
490-
let coverage::BranchInfo { branch_spans, mcdc_branch_spans, mcdc_decision_spans, .. } =
491-
branch_info;
490+
let coverage::CoverageInfoHi {
491+
num_block_markers: _,
492+
branch_spans,
493+
mcdc_branch_spans,
494+
mcdc_decision_spans,
495+
} = coverage_info_hi;
496+
497+
// Only add an extra trailing newline if we printed at least one thing.
498+
let mut did_print = false;
492499

493500
for coverage::BranchSpan { span, true_marker, false_marker } in branch_spans {
494501
writeln!(
495502
w,
496503
"{INDENT}coverage branch {{ true: {true_marker:?}, false: {false_marker:?} }} => {span:?}",
497504
)?;
505+
did_print = true;
498506
}
499507

500508
for coverage::MCDCBranchSpan {
@@ -510,6 +518,7 @@ fn write_coverage_branch_info(
510518
"{INDENT}coverage mcdc branch {{ condition_id: {:?}, true: {true_marker:?}, false: {false_marker:?}, depth: {decision_depth:?} }} => {span:?}",
511519
condition_info.map(|info| info.condition_id)
512520
)?;
521+
did_print = true;
513522
}
514523

515524
for coverage::MCDCDecisionSpan { span, num_conditions, end_markers, decision_depth } in
@@ -519,10 +528,10 @@ fn write_coverage_branch_info(
519528
w,
520529
"{INDENT}coverage mcdc decision {{ num_conditions: {num_conditions:?}, end: {end_markers:?}, depth: {decision_depth:?} }} => {span:?}"
521530
)?;
531+
did_print = true;
522532
}
523533

524-
if !branch_spans.is_empty() || !mcdc_branch_spans.is_empty() || !mcdc_decision_spans.is_empty()
525-
{
534+
if did_print {
526535
writeln!(w)?;
527536
}
528537

‎compiler/rustc_mir_build/src/build/coverageinfo.rs

+60-47
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use std::assert_matches::assert_matches;
22
use std::collections::hash_map::Entry;
33

44
use rustc_data_structures::fx::FxHashMap;
5-
use rustc_middle::mir::coverage::{BlockMarkerId, BranchSpan, CoverageKind};
5+
use rustc_middle::mir::coverage::{BlockMarkerId, BranchSpan, CoverageInfoHi, CoverageKind};
66
use rustc_middle::mir::{self, BasicBlock, SourceInfo, UnOp};
77
use rustc_middle::thir::{ExprId, ExprKind, Pat, Thir};
88
use rustc_middle::ty::TyCtxt;
@@ -13,16 +13,25 @@ use crate::build::{Builder, CFG};
1313

1414
mod mcdc;
1515

16-
pub(crate) struct BranchInfoBuilder {
16+
/// Collects coverage-related information during MIR building, to eventually be
17+
/// turned into a function's [`CoverageInfoHi`] when MIR building is complete.
18+
pub(crate) struct CoverageInfoBuilder {
1719
/// Maps condition expressions to their enclosing `!`, for better instrumentation.
1820
nots: FxHashMap<ExprId, NotInfo>,
1921

2022
markers: BlockMarkerGen,
21-
branch_spans: Vec<BranchSpan>,
2223

24+
/// Present if branch coverage is enabled.
25+
branch_info: Option<BranchInfo>,
26+
/// Present if MC/DC coverage is enabled.
2327
mcdc_info: Option<MCDCInfoBuilder>,
2428
}
2529

30+
#[derive(Default)]
31+
struct BranchInfo {
32+
branch_spans: Vec<BranchSpan>,
33+
}
34+
2635
#[derive(Clone, Copy)]
2736
struct NotInfo {
2837
/// When visiting the associated expression as a branch condition, treat this
@@ -62,20 +71,20 @@ impl BlockMarkerGen {
6271
}
6372
}
6473

65-
impl BranchInfoBuilder {
66-
/// Creates a new branch info builder, but only if branch coverage instrumentation
74+
impl CoverageInfoBuilder {
75+
/// Creates a new coverage info builder, but only if coverage instrumentation
6776
/// is enabled and `def_id` represents a function that is eligible for coverage.
6877
pub(crate) fn new_if_enabled(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Option<Self> {
69-
if tcx.sess.instrument_coverage_branch() && tcx.is_eligible_for_coverage(def_id) {
70-
Some(Self {
71-
nots: FxHashMap::default(),
72-
markers: BlockMarkerGen::default(),
73-
branch_spans: vec![],
74-
mcdc_info: tcx.sess.instrument_coverage_mcdc().then(MCDCInfoBuilder::new),
75-
})
76-
} else {
77-
None
78+
if !tcx.sess.instrument_coverage() || !tcx.is_eligible_for_coverage(def_id) {
79+
return None;
7880
}
81+
82+
Some(Self {
83+
nots: FxHashMap::default(),
84+
markers: BlockMarkerGen::default(),
85+
branch_info: tcx.sess.instrument_coverage_branch().then(BranchInfo::default),
86+
mcdc_info: tcx.sess.instrument_coverage_mcdc().then(MCDCInfoBuilder::new),
87+
})
7988
}
8089

8190
/// Unary `!` expressions inside an `if` condition are lowered by lowering
@@ -88,6 +97,12 @@ impl BranchInfoBuilder {
8897
pub(crate) fn visit_unary_not(&mut self, thir: &Thir<'_>, unary_not: ExprId) {
8998
assert_matches!(thir[unary_not].kind, ExprKind::Unary { op: UnOp::Not, .. });
9099

100+
// The information collected by this visitor is only needed when branch
101+
// coverage or higher is enabled.
102+
if self.branch_info.is_none() {
103+
return;
104+
}
105+
91106
self.visit_with_not_info(
92107
thir,
93108
unary_not,
@@ -137,40 +152,40 @@ impl BranchInfoBuilder {
137152
false_block,
138153
inject_block_marker,
139154
);
140-
} else {
141-
let true_marker = self.markers.inject_block_marker(cfg, source_info, true_block);
142-
let false_marker = self.markers.inject_block_marker(cfg, source_info, false_block);
143-
144-
self.branch_spans.push(BranchSpan {
145-
span: source_info.span,
146-
true_marker,
147-
false_marker,
148-
});
155+
return;
149156
}
157+
158+
// Bail out if branch coverage is not enabled.
159+
let Some(branch_info) = self.branch_info.as_mut() else { return };
160+
161+
let true_marker = self.markers.inject_block_marker(cfg, source_info, true_block);
162+
let false_marker = self.markers.inject_block_marker(cfg, source_info, false_block);
163+
164+
branch_info.branch_spans.push(BranchSpan {
165+
span: source_info.span,
166+
true_marker,
167+
false_marker,
168+
});
150169
}
151170

152-
pub(crate) fn into_done(self) -> Option<Box<mir::coverage::BranchInfo>> {
153-
let Self {
154-
nots: _,
155-
markers: BlockMarkerGen { num_block_markers },
156-
branch_spans,
157-
mcdc_info,
158-
} = self;
171+
pub(crate) fn into_done(self) -> Box<CoverageInfoHi> {
172+
let Self { nots: _, markers: BlockMarkerGen { num_block_markers }, branch_info, mcdc_info } =
173+
self;
159174

160-
if num_block_markers == 0 {
161-
assert!(branch_spans.is_empty());
162-
return None;
163-
}
175+
let branch_spans =
176+
branch_info.map(|branch_info| branch_info.branch_spans).unwrap_or_default();
164177

165178
let (mcdc_decision_spans, mcdc_branch_spans) =
166179
mcdc_info.map(MCDCInfoBuilder::into_done).unwrap_or_default();
167180

168-
Some(Box::new(mir::coverage::BranchInfo {
181+
// For simplicity, always return an info struct (without Option), even
182+
// if there's nothing interesting in it.
183+
Box::new(CoverageInfoHi {
169184
num_block_markers,
170185
branch_spans,
171186
mcdc_branch_spans,
172187
mcdc_decision_spans,
173-
}))
188+
})
174189
}
175190
}
176191

@@ -184,7 +199,7 @@ impl<'tcx> Builder<'_, 'tcx> {
184199
block: &mut BasicBlock,
185200
) {
186201
// Bail out if condition coverage is not enabled for this function.
187-
let Some(branch_info) = self.coverage_branch_info.as_mut() else { return };
202+
let Some(coverage_info) = self.coverage_info.as_mut() else { return };
188203
if !self.tcx.sess.instrument_coverage_condition() {
189204
return;
190205
};
@@ -224,7 +239,7 @@ impl<'tcx> Builder<'_, 'tcx> {
224239
);
225240

226241
// Separate path for handling branches when MC/DC is enabled.
227-
branch_info.register_two_way_branch(
242+
coverage_info.register_two_way_branch(
228243
self.tcx,
229244
&mut self.cfg,
230245
source_info,
@@ -247,12 +262,12 @@ impl<'tcx> Builder<'_, 'tcx> {
247262
mut then_block: BasicBlock,
248263
mut else_block: BasicBlock,
249264
) {
250-
// Bail out if branch coverage is not enabled for this function.
251-
let Some(branch_info) = self.coverage_branch_info.as_mut() else { return };
265+
// Bail out if coverage is not enabled for this function.
266+
let Some(coverage_info) = self.coverage_info.as_mut() else { return };
252267

253268
// If this condition expression is nested within one or more `!` expressions,
254269
// replace it with the enclosing `!` collected by `visit_unary_not`.
255-
if let Some(&NotInfo { enclosing_not, is_flipped }) = branch_info.nots.get(&expr_id) {
270+
if let Some(&NotInfo { enclosing_not, is_flipped }) = coverage_info.nots.get(&expr_id) {
256271
expr_id = enclosing_not;
257272
if is_flipped {
258273
std::mem::swap(&mut then_block, &mut else_block);
@@ -261,7 +276,7 @@ impl<'tcx> Builder<'_, 'tcx> {
261276

262277
let source_info = SourceInfo { span: self.thir[expr_id].span, scope: self.source_scope };
263278

264-
branch_info.register_two_way_branch(
279+
coverage_info.register_two_way_branch(
265280
self.tcx,
266281
&mut self.cfg,
267282
source_info,
@@ -280,13 +295,11 @@ impl<'tcx> Builder<'_, 'tcx> {
280295
true_block: BasicBlock,
281296
false_block: BasicBlock,
282297
) {
283-
// Bail out if branch coverage is not enabled for this function.
284-
let Some(branch_info) = self.coverage_branch_info.as_mut() else { return };
285-
286-
// FIXME(#124144) This may need special handling when MC/DC is enabled.
298+
// Bail out if coverage is not enabled for this function.
299+
let Some(coverage_info) = self.coverage_info.as_mut() else { return };
287300

288301
let source_info = SourceInfo { span: pattern.span, scope: self.source_scope };
289-
branch_info.register_two_way_branch(
302+
coverage_info.register_two_way_branch(
290303
self.tcx,
291304
&mut self.cfg,
292305
source_info,

‎compiler/rustc_mir_build/src/build/coverageinfo/mcdc.rs

+6-6
Original file line numberDiff line numberDiff line change
@@ -250,24 +250,24 @@ impl MCDCInfoBuilder {
250250

251251
impl Builder<'_, '_> {
252252
pub(crate) fn visit_coverage_branch_operation(&mut self, logical_op: LogicalOp, span: Span) {
253-
if let Some(branch_info) = self.coverage_branch_info.as_mut()
254-
&& let Some(mcdc_info) = branch_info.mcdc_info.as_mut()
253+
if let Some(coverage_info) = self.coverage_info.as_mut()
254+
&& let Some(mcdc_info) = coverage_info.mcdc_info.as_mut()
255255
{
256256
mcdc_info.state.record_conditions(logical_op, span);
257257
}
258258
}
259259

260260
pub(crate) fn mcdc_increment_depth_if_enabled(&mut self) {
261-
if let Some(branch_info) = self.coverage_branch_info.as_mut()
262-
&& let Some(mcdc_info) = branch_info.mcdc_info.as_mut()
261+
if let Some(coverage_info) = self.coverage_info.as_mut()
262+
&& let Some(mcdc_info) = coverage_info.mcdc_info.as_mut()
263263
{
264264
mcdc_info.state.decision_ctx_stack.push(MCDCDecisionCtx::default());
265265
};
266266
}
267267

268268
pub(crate) fn mcdc_decrement_depth_if_enabled(&mut self) {
269-
if let Some(branch_info) = self.coverage_branch_info.as_mut()
270-
&& let Some(mcdc_info) = branch_info.mcdc_info.as_mut()
269+
if let Some(coverage_info) = self.coverage_info.as_mut()
270+
&& let Some(mcdc_info) = coverage_info.mcdc_info.as_mut()
271271
{
272272
if mcdc_info.state.decision_ctx_stack.pop().is_none() {
273273
bug!("Unexpected empty decision stack");

‎compiler/rustc_mir_build/src/build/custom/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ pub(super) fn build_custom_mir<'tcx>(
6262
tainted_by_errors: None,
6363
injection_phase: None,
6464
pass_count: 0,
65-
coverage_branch_info: None,
65+
coverage_info_hi: None,
6666
function_coverage_info: None,
6767
};
6868

‎compiler/rustc_mir_build/src/build/matches/mod.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -160,8 +160,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
160160
// Improve branch coverage instrumentation by noting conditions
161161
// nested within one or more `!` expressions.
162162
// (Skipped if branch coverage is not enabled.)
163-
if let Some(branch_info) = this.coverage_branch_info.as_mut() {
164-
branch_info.visit_unary_not(this.thir, expr_id);
163+
if let Some(coverage_info) = this.coverage_info.as_mut() {
164+
coverage_info.visit_unary_not(this.thir, expr_id);
165165
}
166166

167167
let local_scope = this.local_scope();

0 commit comments

Comments
 (0)