Skip to content

Commit ce5bdb9

Browse files
committed
coverage: Represent branches as a list of arms
Within the `InstrumentCoverage` pass, we now represent branches as a list of arms, instead of a true/false pair, until we prepare the final table of mappings to be attached to the MIR body. (We then flatten the list into two-way branches by treating each arm as a branch between its success block, and the total of all later arms.) Currently all of the branches produced by MIR building are still two-way, but this is a step towards allowing many-way branches.
1 parent c6eb1ac commit ce5bdb9

File tree

5 files changed

+59
-34
lines changed

5 files changed

+59
-34
lines changed

compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs

+9-1
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,15 @@ impl<'tcx> FunctionCoverageCollector<'tcx> {
6565
// For each expression ID that is directly used by one or more mappings,
6666
// mark it as not-yet-seen. This indicates that we expect to see a
6767
// corresponding `ExpressionUsed` statement during MIR traversal.
68-
for term in function_coverage_info.mappings.iter().flat_map(|m| m.kind.terms()) {
68+
for term in function_coverage_info
69+
.mappings
70+
.iter()
71+
// For many-armed branches, some branch mappings will have expressions
72+
// that don't correspond to any node in the control-flow graph, so don't
73+
// expect to see `ExpressionUsed` statements for them.
74+
.filter(|m| !matches!(m.kind, MappingKind::Branch { .. }))
75+
.flat_map(|m| m.kind.terms())
76+
{
6977
if let CovTerm::Expression(id) = term {
7078
expressions_seen.remove(id);
7179
}

compiler/rustc_mir_transform/src/coverage/counters.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,12 @@ impl CoverageCounters {
8888
BcbCounter::Counter { id }
8989
}
9090

91-
fn make_expression(&mut self, lhs: BcbCounter, op: Op, rhs: BcbCounter) -> BcbCounter {
91+
pub(super) fn make_expression(
92+
&mut self,
93+
lhs: BcbCounter,
94+
op: Op,
95+
rhs: BcbCounter,
96+
) -> BcbCounter {
9297
let expression = Expression { lhs: lhs.as_term(), op, rhs: rhs.as_term() };
9398
let id = self.expressions.push(expression);
9499
BcbCounter::Expression { id }

compiler/rustc_mir_transform/src/coverage/mod.rs

+30-13
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ mod tests;
99

1010
use self::counters::{CounterIncrementSite, CoverageCounters};
1111
use self::graph::{BasicCoverageBlock, CoverageGraph};
12-
use self::spans::{BcbBranchPair, BcbMapping, BcbMappingKind, CoverageSpans};
12+
use self::spans::{BcbBranchArm, BcbMapping, BcbMappingKind, CoverageSpans};
1313

1414
use crate::MirPass;
1515

@@ -83,10 +83,10 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir:
8383
// and all `Expression` dependencies (operands) are also generated, for any other
8484
// `BasicCoverageBlock`s not already associated with a coverage span.
8585
let bcb_has_coverage_spans = |bcb| coverage_spans.bcb_has_coverage_spans(bcb);
86-
let coverage_counters =
86+
let mut coverage_counters =
8787
CoverageCounters::make_bcb_counters(&basic_coverage_blocks, bcb_has_coverage_spans);
8888

89-
let mappings = create_mappings(tcx, &hir_info, &coverage_spans, &coverage_counters);
89+
let mappings = create_mappings(tcx, &hir_info, &coverage_spans, &mut coverage_counters);
9090
if mappings.is_empty() {
9191
// No spans could be converted into valid mappings, so skip this function.
9292
debug!("no spans could be converted into valid mappings; skipping");
@@ -120,7 +120,7 @@ fn create_mappings<'tcx>(
120120
tcx: TyCtxt<'tcx>,
121121
hir_info: &ExtractedHirInfo,
122122
coverage_spans: &CoverageSpans,
123-
coverage_counters: &CoverageCounters,
123+
coverage_counters: &mut CoverageCounters,
124124
) -> Vec<Mapping> {
125125
let source_map = tcx.sess.source_map();
126126
let body_span = hir_info.body_span;
@@ -169,15 +169,32 @@ fn create_mappings<'tcx>(
169169
},
170170
));
171171

172-
mappings.extend(coverage_spans.branch_pairs.iter().filter_map(
173-
|&BcbBranchPair { span, true_bcb, false_bcb }| {
174-
let true_term = term_for_bcb(true_bcb);
175-
let false_term = term_for_bcb(false_bcb);
176-
let kind = MappingKind::Branch { true_term, false_term };
177-
let code_region = make_code_region(source_map, file_name, span, body_span)?;
178-
Some(Mapping { kind, code_region })
179-
},
180-
));
172+
for arm_list in &coverage_spans.branch_arm_lists {
173+
let mut arms_rev = arm_list.iter().rev();
174+
175+
let mut rest_counter = {
176+
// The last arm's span is ignored, because its BCB is only used as the
177+
// false branch of the second-last arm; it's not a branch of its own.
178+
let Some(&BcbBranchArm { span: _, bcb }) = arms_rev.next() else { continue };
179+
coverage_counters.bcb_counter(bcb).expect("all relevant BCBs have counters")
180+
};
181+
182+
for &BcbBranchArm { span, bcb } in arms_rev {
183+
let true_counter =
184+
coverage_counters.bcb_counter(bcb).expect("all relevant BCBs have counters");
185+
let kind = MappingKind::Branch {
186+
true_term: true_counter.as_term(),
187+
false_term: rest_counter.as_term(),
188+
};
189+
190+
if let Some(code_region) = make_code_region(source_map, file_name, span, body_span) {
191+
mappings.push(Mapping { kind, code_region });
192+
}
193+
194+
// FIXME: Avoid creating an unused expression on the last iteration.
195+
rest_counter = coverage_counters.make_expression(true_counter, Op::Add, rest_counter);
196+
}
197+
}
181198

182199
mappings
183200
}

compiler/rustc_mir_transform/src/coverage/spans.rs

+9-14
Original file line numberDiff line numberDiff line change
@@ -37,20 +37,16 @@ pub(super) struct BcbMapping {
3737
pub(super) span: Span,
3838
}
3939

40-
/// This is separate from [`BcbMappingKind`] to help prepare for larger changes
41-
/// that will be needed for improved branch coverage in the future.
42-
/// (See <https://github.com/rust-lang/rust/pull/124217>.)
4340
#[derive(Debug)]
44-
pub(super) struct BcbBranchPair {
41+
pub(super) struct BcbBranchArm {
4542
pub(super) span: Span,
46-
pub(super) true_bcb: BasicCoverageBlock,
47-
pub(super) false_bcb: BasicCoverageBlock,
43+
pub(super) bcb: BasicCoverageBlock,
4844
}
4945

5046
pub(super) struct CoverageSpans {
5147
bcb_has_mappings: BitSet<BasicCoverageBlock>,
5248
pub(super) mappings: Vec<BcbMapping>,
53-
pub(super) branch_pairs: Vec<BcbBranchPair>,
49+
pub(super) branch_arm_lists: Vec<Vec<BcbBranchArm>>,
5450
test_vector_bitmap_bytes: u32,
5551
}
5652

@@ -74,7 +70,7 @@ pub(super) fn generate_coverage_spans(
7470
basic_coverage_blocks: &CoverageGraph,
7571
) -> Option<CoverageSpans> {
7672
let mut mappings = vec![];
77-
let mut branch_pairs = vec![];
73+
let mut branch_arm_lists = vec![];
7874

7975
if hir_info.is_async_fn {
8076
// An async function desugars into a function that returns a future,
@@ -96,7 +92,7 @@ pub(super) fn generate_coverage_spans(
9692
BcbMapping { kind: BcbMappingKind::Code(bcb), span }
9793
}));
9894

99-
branch_pairs.extend(from_mir::extract_branch_pairs(
95+
branch_arm_lists.extend(from_mir::extract_branch_arm_lists(
10096
mir_body,
10197
hir_info,
10298
basic_coverage_blocks,
@@ -109,7 +105,7 @@ pub(super) fn generate_coverage_spans(
109105
));
110106
}
111107

112-
if mappings.is_empty() && branch_pairs.is_empty() {
108+
if mappings.is_empty() && branch_arm_lists.is_empty() {
113109
return None;
114110
}
115111

@@ -135,12 +131,11 @@ pub(super) fn generate_coverage_spans(
135131
}
136132
}
137133
}
138-
for &BcbBranchPair { true_bcb, false_bcb, .. } in &branch_pairs {
139-
insert(true_bcb);
140-
insert(false_bcb);
134+
for &BcbBranchArm { bcb, .. } in branch_arm_lists.iter().flatten() {
135+
insert(bcb);
141136
}
142137

143-
Some(CoverageSpans { bcb_has_mappings, mappings, branch_pairs, test_vector_bitmap_bytes })
138+
Some(CoverageSpans { bcb_has_mappings, mappings, branch_arm_lists, test_vector_bitmap_bytes })
144139
}
145140

146141
#[derive(Debug)]

compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use rustc_span::{ExpnKind, MacroKind, Span, Symbol};
1313
use crate::coverage::graph::{
1414
BasicCoverageBlock, BasicCoverageBlockData, CoverageGraph, START_BCB,
1515
};
16-
use crate::coverage::spans::{BcbBranchPair, BcbMapping, BcbMappingKind};
16+
use crate::coverage::spans::{BcbBranchArm, BcbMapping, BcbMappingKind};
1717
use crate::coverage::ExtractedHirInfo;
1818

1919
/// Traverses the MIR body to produce an initial collection of coverage-relevant
@@ -388,16 +388,16 @@ fn resolve_block_markers(
388388
}
389389

390390
// FIXME: There is currently a lot of redundancy between
391-
// `extract_branch_pairs` and `extract_mcdc_mappings`. This is needed so
391+
// `extract_branch_arm_lists` and `extract_mcdc_mappings`. This is needed so
392392
// that they can each be modified without interfering with the other, but in
393393
// the long term we should try to bring them together again when branch coverage
394394
// and MC/DC coverage support are more mature.
395395

396-
pub(super) fn extract_branch_pairs(
396+
pub(super) fn extract_branch_arm_lists(
397397
mir_body: &mir::Body<'_>,
398398
hir_info: &ExtractedHirInfo,
399399
basic_coverage_blocks: &CoverageGraph,
400-
) -> Vec<BcbBranchPair> {
400+
) -> Vec<Vec<BcbBranchArm>> {
401401
let Some(branch_info) = mir_body.coverage_branch_info.as_deref() else { return vec![] };
402402

403403
let block_markers = resolve_block_markers(branch_info, mir_body);
@@ -420,7 +420,7 @@ pub(super) fn extract_branch_pairs(
420420
let true_bcb = bcb_from_marker(true_marker)?;
421421
let false_bcb = bcb_from_marker(false_marker)?;
422422

423-
Some(BcbBranchPair { span, true_bcb, false_bcb })
423+
Some(vec![BcbBranchArm { span, bcb: true_bcb }, BcbBranchArm { span, bcb: false_bcb }])
424424
})
425425
.collect::<Vec<_>>()
426426
}

0 commit comments

Comments
 (0)