Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit e5d65e4

Browse filesBrowse files
matthiaskrgrmatthiaskrgr
authored
Rollup merge of #127758 - Zalathar:expression-used, r=oli-obk
coverage: Restrict `ExpressionUsed` simplification to `Code` mappings In the future, branch and MC/DC mappings might have expressions that don't correspond to any single point in the control-flow graph. That makes it trickier to keep track of which expressions should expect an `ExpressionUsed` node. We therefore sidestep that complexity by only performing `ExpressionUsed` simplification for expressions associated directly with ordinary `Code` mappings. (This simplification step is inherited from the original coverage implementation, which only supported `Code` mappings anyway, so there's no particular reason to extend it to other kinds of mappings unless we specifically choose to.) Relevant to: - #124154 - #126677 - #124278 ```@rustbot``` label +A-code-coverage
2 parents cd25232 + d4f1f92 commit e5d65e4
Copy full SHA for e5d65e4

File tree

Expand file treeCollapse file tree

4 files changed

+37
-26
lines changed
Filter options
Expand file treeCollapse file tree

4 files changed

+37
-26
lines changed

‎compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs

Copy file name to clipboardExpand all lines: compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs
+9-2Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,15 @@ impl<'tcx> FunctionCoverageCollector<'tcx> {
6666
// For each expression ID that is directly used by one or more mappings,
6767
// mark it as not-yet-seen. This indicates that we expect to see a
6868
// corresponding `ExpressionUsed` statement during MIR traversal.
69-
for term in function_coverage_info.mappings.iter().flat_map(|m| m.kind.terms()) {
70-
if let CovTerm::Expression(id) = term {
69+
for mapping in function_coverage_info.mappings.iter() {
70+
// Currently we only worry about ordinary code mappings.
71+
// For branch and MC/DC mappings, expressions might not correspond
72+
// to any particular point in the control-flow graph.
73+
// (Keep this in sync with the injection of `ExpressionUsed`
74+
// statements in the `InstrumentCoverage` MIR pass.)
75+
if let MappingKind::Code(term) = mapping.kind
76+
&& let CovTerm::Expression(id) = term
77+
{
7178
expressions_seen.remove(id);
7279
}
7380
}

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

Copy file name to clipboardExpand all lines: compiler/rustc_middle/src/mir/coverage.rs
-13Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -220,19 +220,6 @@ pub enum MappingKind {
220220
}
221221

222222
impl MappingKind {
223-
/// Iterator over all coverage terms in this mapping kind.
224-
pub fn terms(&self) -> impl Iterator<Item = CovTerm> {
225-
let zero = || None.into_iter().chain(None);
226-
let one = |a| Some(a).into_iter().chain(None);
227-
let two = |a, b| Some(a).into_iter().chain(Some(b));
228-
match *self {
229-
Self::Code(term) => one(term),
230-
Self::Branch { true_term, false_term } => two(true_term, false_term),
231-
Self::MCDCBranch { true_term, false_term, .. } => two(true_term, false_term),
232-
Self::MCDCDecision(_) => zero(),
233-
}
234-
}
235-
236223
/// Returns a copy of this mapping kind, in which all coverage terms have
237224
/// been replaced with ones returned by the given function.
238225
pub fn map_terms(&self, map_fn: impl Fn(CovTerm) -> CovTerm) -> Self {

‎compiler/rustc_mir_transform/src/coverage/mappings.rs

Copy file name to clipboardExpand all lines: compiler/rustc_mir_transform/src/coverage/mappings.rs
+17-5Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,10 @@ pub(super) struct MCDCDecision {
5656

5757
#[derive(Default)]
5858
pub(super) struct ExtractedMappings {
59+
/// Store our own copy of [`CoverageGraph::num_nodes`], so that we don't
60+
/// need access to the whole graph when allocating per-BCB data. This is
61+
/// only public so that other code can still use exhaustive destructuring.
62+
pub(super) num_bcbs: usize,
5963
pub(super) code_mappings: Vec<CodeMapping>,
6064
pub(super) branch_pairs: Vec<BranchPair>,
6165
pub(super) mcdc_bitmap_bytes: u32,
@@ -106,6 +110,7 @@ pub(super) fn extract_all_mapping_info_from_mir<'tcx>(
106110
);
107111

108112
ExtractedMappings {
113+
num_bcbs: basic_coverage_blocks.num_nodes(),
109114
code_mappings,
110115
branch_pairs,
111116
mcdc_bitmap_bytes,
@@ -115,12 +120,10 @@ pub(super) fn extract_all_mapping_info_from_mir<'tcx>(
115120
}
116121

117122
impl ExtractedMappings {
118-
pub(super) fn all_bcbs_with_counter_mappings(
119-
&self,
120-
basic_coverage_blocks: &CoverageGraph, // Only used for allocating a correctly-sized set
121-
) -> BitSet<BasicCoverageBlock> {
123+
pub(super) fn all_bcbs_with_counter_mappings(&self) -> BitSet<BasicCoverageBlock> {
122124
// Fully destructure self to make sure we don't miss any fields that have mappings.
123125
let Self {
126+
num_bcbs,
124127
code_mappings,
125128
branch_pairs,
126129
mcdc_bitmap_bytes: _,
@@ -129,7 +132,7 @@ impl ExtractedMappings {
129132
} = self;
130133

131134
// Identify which BCBs have one or more mappings.
132-
let mut bcbs_with_counter_mappings = BitSet::new_empty(basic_coverage_blocks.num_nodes());
135+
let mut bcbs_with_counter_mappings = BitSet::new_empty(*num_bcbs);
133136
let mut insert = |bcb| {
134137
bcbs_with_counter_mappings.insert(bcb);
135138
};
@@ -156,6 +159,15 @@ impl ExtractedMappings {
156159

157160
bcbs_with_counter_mappings
158161
}
162+
163+
/// Returns the set of BCBs that have one or more `Code` mappings.
164+
pub(super) fn bcbs_with_ordinary_code_mappings(&self) -> BitSet<BasicCoverageBlock> {
165+
let mut bcbs = BitSet::new_empty(self.num_bcbs);
166+
for &CodeMapping { span: _, bcb } in &self.code_mappings {
167+
bcbs.insert(bcb);
168+
}
169+
bcbs
170+
}
159171
}
160172

161173
fn resolve_block_markers(

‎compiler/rustc_mir_transform/src/coverage/mod.rs

Copy file name to clipboardExpand all lines: compiler/rustc_mir_transform/src/coverage/mod.rs
+11-6Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ use rustc_span::source_map::SourceMap;
2525
use rustc_span::{BytePos, Pos, RelativeBytePos, Span, Symbol};
2626

2727
use crate::coverage::counters::{CounterIncrementSite, CoverageCounters};
28-
use crate::coverage::graph::{BasicCoverageBlock, CoverageGraph};
28+
use crate::coverage::graph::CoverageGraph;
2929
use crate::coverage::mappings::ExtractedMappings;
3030
use crate::MirPass;
3131

@@ -88,8 +88,7 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir:
8888
// every coverage span has a `Counter` or `Expression` assigned to its `BasicCoverageBlock`
8989
// and all `Expression` dependencies (operands) are also generated, for any other
9090
// `BasicCoverageBlock`s not already associated with a coverage span.
91-
let bcbs_with_counter_mappings =
92-
extracted_mappings.all_bcbs_with_counter_mappings(&basic_coverage_blocks);
91+
let bcbs_with_counter_mappings = extracted_mappings.all_bcbs_with_counter_mappings();
9392
if bcbs_with_counter_mappings.is_empty() {
9493
// No relevant spans were found in MIR, so skip instrumenting this function.
9594
return;
@@ -109,7 +108,7 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir:
109108
inject_coverage_statements(
110109
mir_body,
111110
&basic_coverage_blocks,
112-
bcb_has_counter_mappings,
111+
&extracted_mappings,
113112
&coverage_counters,
114113
);
115114

@@ -163,6 +162,7 @@ fn create_mappings<'tcx>(
163162

164163
// Fully destructure the mappings struct to make sure we don't miss any kinds.
165164
let ExtractedMappings {
165+
num_bcbs: _,
166166
code_mappings,
167167
branch_pairs,
168168
mcdc_bitmap_bytes: _,
@@ -219,7 +219,7 @@ fn create_mappings<'tcx>(
219219
fn inject_coverage_statements<'tcx>(
220220
mir_body: &mut mir::Body<'tcx>,
221221
basic_coverage_blocks: &CoverageGraph,
222-
bcb_has_coverage_spans: impl Fn(BasicCoverageBlock) -> bool,
222+
extracted_mappings: &ExtractedMappings,
223223
coverage_counters: &CoverageCounters,
224224
) {
225225
// Inject counter-increment statements into MIR.
@@ -252,11 +252,16 @@ fn inject_coverage_statements<'tcx>(
252252
// can check whether the injected statement survived MIR optimization.
253253
// (BCB edges can't have spans, so we only need to process BCB nodes here.)
254254
//
255+
// We only do this for ordinary `Code` mappings, because branch and MC/DC
256+
// mappings might have expressions that don't correspond to any single
257+
// point in the control-flow graph.
258+
//
255259
// See the code in `rustc_codegen_llvm::coverageinfo::map_data` that deals
256260
// with "expressions seen" and "zero terms".
261+
let eligible_bcbs = extracted_mappings.bcbs_with_ordinary_code_mappings();
257262
for (bcb, expression_id) in coverage_counters
258263
.bcb_nodes_with_coverage_expressions()
259-
.filter(|&(bcb, _)| bcb_has_coverage_spans(bcb))
264+
.filter(|&(bcb, _)| eligible_bcbs.contains(bcb))
260265
{
261266
inject_statement(
262267
mir_body,

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.