Skip to content

Commit 71db4c5

Browse files
committed
coverage: Remove unhelpful code for handling multiple files per function
Functions currently can't have mappings in multiple files, and if that ever changes (e.g. to properly support expansion regions), this code will need to be completely overhauled anyway.
1 parent a920895 commit 71db4c5

11 files changed

+98
-110
lines changed

compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ pub(crate) struct CoverageSpan {
138138

139139
impl CoverageSpan {
140140
pub(crate) fn from_source_region(file_id: u32, code_region: &SourceRegion) -> Self {
141-
let &SourceRegion { file_name: _, start_line, start_col, end_line, end_col } = code_region;
141+
let &SourceRegion { start_line, start_col, end_line, end_col } = code_region;
142142
// Internally, LLVM uses the high bit of `end_col` to distinguish between
143143
// code regions and gap regions, so it can't be used by the column number.
144144
assert!(end_col & (1u32 << 31) == 0, "high bit of `end_col` must be unset: {end_col:#X}");

compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs

+1-7
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ use rustc_middle::mir::coverage::{
66
SourceRegion,
77
};
88
use rustc_middle::ty::Instance;
9-
use rustc_span::Symbol;
109
use tracing::{debug, instrument};
1110

1211
use crate::coverageinfo::ffi::{Counter, CounterExpression, ExprKind};
@@ -180,7 +179,7 @@ impl<'tcx> FunctionCoverageCollector<'tcx> {
180179
}
181180

182181
pub(crate) struct FunctionCoverage<'tcx> {
183-
function_coverage_info: &'tcx FunctionCoverageInfo,
182+
pub(crate) function_coverage_info: &'tcx FunctionCoverageInfo,
184183
is_used: bool,
185184

186185
counters_seen: BitSet<CounterId>,
@@ -199,11 +198,6 @@ impl<'tcx> FunctionCoverage<'tcx> {
199198
if self.is_used { self.function_coverage_info.function_source_hash } else { 0 }
200199
}
201200

202-
/// Returns an iterator over all filenames used by this function's mappings.
203-
pub(crate) fn all_file_names(&self) -> impl Iterator<Item = Symbol> + Captures<'_> {
204-
self.function_coverage_info.mappings.iter().map(|mapping| mapping.source_region.file_name)
205-
}
206-
207201
/// Convert this function's coverage expression data into a form that can be
208202
/// passed through FFI to LLVM.
209203
pub(crate) fn counter_expressions(

compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs

+55-51
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,10 @@ use rustc_index::IndexVec;
1111
use rustc_middle::mir::coverage::MappingKind;
1212
use rustc_middle::ty::{self, TyCtxt};
1313
use rustc_middle::{bug, mir};
14-
use rustc_span::Symbol;
14+
use rustc_session::RemapFileNameExt;
15+
use rustc_session::config::RemapPathScopeComponents;
1516
use rustc_span::def_id::DefIdSet;
17+
use rustc_span::{Span, Symbol};
1618
use rustc_target::spec::HasTargetSpec;
1719
use tracing::debug;
1820

@@ -69,8 +71,10 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) {
6971
.map(|(instance, function_coverage)| (instance, function_coverage.into_finished()))
7072
.collect::<Vec<_>>();
7173

72-
let all_file_names =
73-
function_coverage_entries.iter().flat_map(|(_, fn_cov)| fn_cov.all_file_names());
74+
let all_file_names = function_coverage_entries
75+
.iter()
76+
.map(|(_, fn_cov)| fn_cov.function_coverage_info.body_span)
77+
.map(|span| span_file_name(tcx, span));
7478
let global_file_table = GlobalFileTable::new(all_file_names);
7579

7680
// Encode all filenames referenced by coverage mappings in this CGU.
@@ -95,7 +99,7 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) {
9599
let is_used = function_coverage.is_used();
96100

97101
let coverage_mapping_buffer =
98-
encode_mappings_for_function(&global_file_table, &function_coverage);
102+
encode_mappings_for_function(tcx, &global_file_table, &function_coverage);
99103

100104
if coverage_mapping_buffer.is_empty() {
101105
if function_coverage.is_used() {
@@ -232,12 +236,20 @@ impl VirtualFileMapping {
232236
}
233237
}
234238

239+
fn span_file_name(tcx: TyCtxt<'_>, span: Span) -> Symbol {
240+
let source_file = tcx.sess.source_map().lookup_source_file(span.lo());
241+
let name =
242+
source_file.name.for_scope(tcx.sess, RemapPathScopeComponents::MACRO).to_string_lossy();
243+
Symbol::intern(&name)
244+
}
245+
235246
/// Using the expressions and counter regions collected for a single function,
236247
/// generate the variable-sized payload of its corresponding `__llvm_covfun`
237248
/// entry. The payload is returned as a vector of bytes.
238249
///
239250
/// Newly-encountered filenames will be added to the global file table.
240251
fn encode_mappings_for_function(
252+
tcx: TyCtxt<'_>,
241253
global_file_table: &GlobalFileTable,
242254
function_coverage: &FunctionCoverage<'_>,
243255
) -> Vec<u8> {
@@ -254,53 +266,45 @@ fn encode_mappings_for_function(
254266
let mut mcdc_branch_regions = vec![];
255267
let mut mcdc_decision_regions = vec![];
256268

257-
// Group mappings into runs with the same filename, preserving the order
258-
// yielded by `FunctionCoverage`.
259-
// Prepare file IDs for each filename, and prepare the mapping data so that
260-
// we can pass it through FFI to LLVM.
261-
for (file_name, counter_regions_for_file) in
262-
&counter_regions.group_by(|(_, region)| region.file_name)
263-
{
264-
// Look up the global file ID for this filename.
265-
let global_file_id = global_file_table.global_file_id_for_file_name(file_name);
266-
267-
// Associate that global file ID with a local file ID for this function.
268-
let local_file_id = virtual_file_mapping.local_id_for_global(global_file_id);
269-
debug!(" file id: {local_file_id:?} => {global_file_id:?} = '{file_name:?}'");
270-
271-
// For each counter/region pair in this function+file, convert it to a
272-
// form suitable for FFI.
273-
for (mapping_kind, region) in counter_regions_for_file {
274-
debug!("Adding counter {mapping_kind:?} to map for {region:?}");
275-
let span = ffi::CoverageSpan::from_source_region(local_file_id.as_u32(), region);
276-
match mapping_kind {
277-
MappingKind::Code(term) => {
278-
code_regions
279-
.push(ffi::CodeRegion { span, counter: ffi::Counter::from_term(term) });
280-
}
281-
MappingKind::Branch { true_term, false_term } => {
282-
branch_regions.push(ffi::BranchRegion {
283-
span,
284-
true_counter: ffi::Counter::from_term(true_term),
285-
false_counter: ffi::Counter::from_term(false_term),
286-
});
287-
}
288-
MappingKind::MCDCBranch { true_term, false_term, mcdc_params } => {
289-
mcdc_branch_regions.push(ffi::MCDCBranchRegion {
290-
span,
291-
true_counter: ffi::Counter::from_term(true_term),
292-
false_counter: ffi::Counter::from_term(false_term),
293-
mcdc_branch_params: ffi::mcdc::BranchParameters::from(mcdc_params),
294-
});
295-
}
296-
MappingKind::MCDCDecision(mcdc_decision_params) => {
297-
mcdc_decision_regions.push(ffi::MCDCDecisionRegion {
298-
span,
299-
mcdc_decision_params: ffi::mcdc::DecisionParameters::from(
300-
mcdc_decision_params,
301-
),
302-
});
303-
}
269+
// Currently a function's mappings must all be in the same file as its body span.
270+
let file_name = span_file_name(tcx, function_coverage.function_coverage_info.body_span);
271+
272+
// Look up the global file ID for that filename.
273+
let global_file_id = global_file_table.global_file_id_for_file_name(file_name);
274+
275+
// Associate that global file ID with a local file ID for this function.
276+
let local_file_id = virtual_file_mapping.local_id_for_global(global_file_id);
277+
debug!(" file id: {local_file_id:?} => {global_file_id:?} = '{file_name:?}'");
278+
279+
// For each counter/region pair in this function+file, convert it to a
280+
// form suitable for FFI.
281+
for (mapping_kind, region) in counter_regions {
282+
debug!("Adding counter {mapping_kind:?} to map for {region:?}");
283+
let span = ffi::CoverageSpan::from_source_region(local_file_id.as_u32(), region);
284+
match mapping_kind {
285+
MappingKind::Code(term) => {
286+
code_regions.push(ffi::CodeRegion { span, counter: ffi::Counter::from_term(term) });
287+
}
288+
MappingKind::Branch { true_term, false_term } => {
289+
branch_regions.push(ffi::BranchRegion {
290+
span,
291+
true_counter: ffi::Counter::from_term(true_term),
292+
false_counter: ffi::Counter::from_term(false_term),
293+
});
294+
}
295+
MappingKind::MCDCBranch { true_term, false_term, mcdc_params } => {
296+
mcdc_branch_regions.push(ffi::MCDCBranchRegion {
297+
span,
298+
true_counter: ffi::Counter::from_term(true_term),
299+
false_counter: ffi::Counter::from_term(false_term),
300+
mcdc_branch_params: ffi::mcdc::BranchParameters::from(mcdc_params),
301+
});
302+
}
303+
MappingKind::MCDCDecision(mcdc_decision_params) => {
304+
mcdc_decision_regions.push(ffi::MCDCDecisionRegion {
305+
span,
306+
mcdc_decision_params: ffi::mcdc::DecisionParameters::from(mcdc_decision_params),
307+
});
304308
}
305309
}
306310
}

compiler/rustc_middle/src/mir/coverage.rs

+4-7
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use std::fmt::{self, Debug, Formatter};
44

55
use rustc_index::IndexVec;
66
use rustc_macros::{HashStable, TyDecodable, TyEncodable, TypeFoldable, TypeVisitable};
7-
use rustc_span::{Span, Symbol};
7+
use rustc_span::Span;
88

99
rustc_index::newtype_index! {
1010
/// Used by [`CoverageKind::BlockMarker`] to mark blocks during THIR-to-MIR
@@ -158,7 +158,6 @@ impl Debug for CoverageKind {
158158
#[derive(Clone, TyEncodable, TyDecodable, Hash, HashStable, PartialEq, Eq, PartialOrd, Ord)]
159159
#[derive(TypeFoldable, TypeVisitable)]
160160
pub struct SourceRegion {
161-
pub file_name: Symbol,
162161
pub start_line: u32,
163162
pub start_col: u32,
164163
pub end_line: u32,
@@ -167,11 +166,8 @@ pub struct SourceRegion {
167166

168167
impl Debug for SourceRegion {
169168
fn fmt(&self, fmt: &mut Formatter<'_>) -> fmt::Result {
170-
write!(
171-
fmt,
172-
"{}:{}:{} - {}:{}",
173-
self.file_name, self.start_line, self.start_col, self.end_line, self.end_col
174-
)
169+
let &Self { start_line, start_col, end_line, end_col } = self;
170+
write!(fmt, "{start_line}:{start_col} - {end_line}:{end_col}")
175171
}
176172
}
177173

@@ -246,6 +242,7 @@ pub struct Mapping {
246242
#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
247243
pub struct FunctionCoverageInfo {
248244
pub function_source_hash: u64,
245+
pub body_span: Span,
249246
pub num_counters: usize,
250247
pub mcdc_bitmap_bits: usize,
251248
pub expressions: IndexVec<ExpressionId, Expression>,

compiler/rustc_middle/src/mir/pretty.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -596,8 +596,10 @@ fn write_function_coverage_info(
596596
function_coverage_info: &coverage::FunctionCoverageInfo,
597597
w: &mut dyn io::Write,
598598
) -> io::Result<()> {
599-
let coverage::FunctionCoverageInfo { expressions, mappings, .. } = function_coverage_info;
599+
let coverage::FunctionCoverageInfo { body_span, expressions, mappings, .. } =
600+
function_coverage_info;
600601

602+
writeln!(w, "{INDENT}coverage body span: {body_span:?}")?;
601603
for (id, expression) in expressions.iter_enumerated() {
602604
writeln!(w, "{INDENT}coverage {id:?} => {expression:?};")?;
603605
}

compiler/rustc_mir_transform/src/coverage/mod.rs

+7-21
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ use rustc_middle::mir::{
2222
use rustc_middle::ty::TyCtxt;
2323
use rustc_span::def_id::LocalDefId;
2424
use rustc_span::source_map::SourceMap;
25-
use rustc_span::{BytePos, Pos, RelativeBytePos, Span, Symbol};
25+
use rustc_span::{BytePos, Pos, RelativeBytePos, SourceFile, Span};
2626
use tracing::{debug, debug_span, trace};
2727

2828
use crate::coverage::counters::{CounterIncrementSite, CoverageCounters};
@@ -122,6 +122,7 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir:
122122

123123
mir_body.function_coverage_info = Some(Box::new(FunctionCoverageInfo {
124124
function_source_hash: hir_info.function_source_hash,
125+
body_span: hir_info.body_span,
125126
num_counters: coverage_counters.num_counters(),
126127
mcdc_bitmap_bits: extracted_mappings.mcdc_bitmap_bits,
127128
expressions: coverage_counters.into_expressions(),
@@ -142,19 +143,11 @@ fn create_mappings<'tcx>(
142143
coverage_counters: &CoverageCounters,
143144
) -> Vec<Mapping> {
144145
let source_map = tcx.sess.source_map();
145-
let body_span = hir_info.body_span;
146-
147-
let source_file = source_map.lookup_source_file(body_span.lo());
148-
149-
use rustc_session::RemapFileNameExt;
150-
use rustc_session::config::RemapPathScopeComponents;
151-
let file_name = Symbol::intern(
152-
&source_file.name.for_scope(tcx.sess, RemapPathScopeComponents::MACRO).to_string_lossy(),
153-
);
146+
let file = source_map.lookup_source_file(hir_info.body_span.lo());
154147

155148
let term_for_bcb =
156149
|bcb| coverage_counters.term_for_bcb(bcb).expect("all BCBs with spans were given counters");
157-
let region_for_span = |span: Span| make_source_region(source_map, hir_info, file_name, span);
150+
let region_for_span = |span: Span| make_source_region(source_map, hir_info, &file, span);
158151

159152
// Fully destructure the mappings struct to make sure we don't miss any kinds.
160153
let ExtractedMappings {
@@ -398,7 +391,7 @@ fn inject_statement(mir_body: &mut mir::Body<'_>, counter_kind: CoverageKind, bb
398391
data.statements.insert(0, statement);
399392
}
400393

401-
/// Convert the Span into its file name, start line and column, and end line and column.
394+
/// Converts the span into its start line and column, and end line and column.
402395
///
403396
/// Line numbers and column numbers are 1-based. Unlike most column numbers emitted by
404397
/// the compiler, these column numbers are denoted in **bytes**, because that's what
@@ -411,18 +404,12 @@ fn inject_statement(mir_body: &mut mir::Body<'_>, counter_kind: CoverageKind, bb
411404
fn make_source_region(
412405
source_map: &SourceMap,
413406
hir_info: &ExtractedHirInfo,
414-
file_name: Symbol,
407+
file: &SourceFile,
415408
span: Span,
416409
) -> Option<SourceRegion> {
417410
let lo = span.lo();
418411
let hi = span.hi();
419412

420-
let file = source_map.lookup_source_file(lo);
421-
if !file.contains(hi) {
422-
debug!(?span, ?file, ?lo, ?hi, "span crosses multiple files; skipping");
423-
return None;
424-
}
425-
426413
// Column numbers need to be in bytes, so we can't use the more convenient
427414
// `SourceMap` methods for looking up file coordinates.
428415
let rpos_and_line_and_byte_column = |pos: BytePos| -> Option<(RelativeBytePos, usize, usize)> {
@@ -465,7 +452,6 @@ fn make_source_region(
465452
end_line = source_map.doctest_offset_line(&file.name, end_line);
466453

467454
check_source_region(SourceRegion {
468-
file_name,
469455
start_line: start_line as u32,
470456
start_col: start_col as u32,
471457
end_line: end_line as u32,
@@ -478,7 +464,7 @@ fn make_source_region(
478464
/// discard regions that are improperly ordered, or might be interpreted in a
479465
/// way that makes them improperly ordered.
480466
fn check_source_region(source_region: SourceRegion) -> Option<SourceRegion> {
481-
let SourceRegion { file_name: _, start_line, start_col, end_line, end_col } = source_region;
467+
let SourceRegion { start_line, start_col, end_line, end_col } = source_region;
482468

483469
// Line/column coordinates are supposed to be 1-based. If we ever emit
484470
// coordinates of 0, `llvm-cov` might misinterpret them.

tests/mir-opt/coverage/branch_match_arms.main.InstrumentCoverage.diff

+7-6
Original file line numberDiff line numberDiff line change
@@ -26,18 +26,19 @@
2626
debug a => _9;
2727
}
2828

29+
+ coverage body span: $DIR/branch_match_arms.rs:14:11: 21:2 (#0)
2930
+ coverage ExpressionId(0) => Expression { lhs: Counter(1), op: Add, rhs: Counter(2) };
3031
+ coverage ExpressionId(1) => Expression { lhs: Expression(0), op: Add, rhs: Counter(3) };
3132
+ coverage ExpressionId(2) => Expression { lhs: Counter(0), op: Subtract, rhs: Expression(1) };
3233
+ coverage ExpressionId(3) => Expression { lhs: Counter(3), op: Add, rhs: Counter(2) };
3334
+ coverage ExpressionId(4) => Expression { lhs: Expression(3), op: Add, rhs: Counter(1) };
3435
+ coverage ExpressionId(5) => Expression { lhs: Expression(4), op: Add, rhs: Expression(2) };
35-
+ coverage Code(Counter(0)) => $DIR/branch_match_arms.rs:14:1 - 15:21;
36-
+ coverage Code(Counter(3)) => $DIR/branch_match_arms.rs:16:17 - 16:33;
37-
+ coverage Code(Counter(2)) => $DIR/branch_match_arms.rs:17:17 - 17:33;
38-
+ coverage Code(Counter(1)) => $DIR/branch_match_arms.rs:18:17 - 18:33;
39-
+ coverage Code(Expression(2)) => $DIR/branch_match_arms.rs:19:17 - 19:33;
40-
+ coverage Code(Expression(5)) => $DIR/branch_match_arms.rs:21:1 - 21:2;
36+
+ coverage Code(Counter(0)) => 14:1 - 15:21;
37+
+ coverage Code(Counter(3)) => 16:17 - 16:33;
38+
+ coverage Code(Counter(2)) => 17:17 - 17:33;
39+
+ coverage Code(Counter(1)) => 18:17 - 18:33;
40+
+ coverage Code(Expression(2)) => 19:17 - 19:33;
41+
+ coverage Code(Expression(5)) => 21:1 - 21:2;
4142
+
4243
bb0: {
4344
+ Coverage::CounterIncrement(0);

tests/mir-opt/coverage/instrument_coverage.bar.InstrumentCoverage.diff

+2-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44
fn bar() -> bool {
55
let mut _0: bool;
66

7-
+ coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:19:1 - 21:2;
7+
+ coverage body span: $DIR/instrument_coverage.rs:19:18: 21:2 (#0)
8+
+ coverage Code(Counter(0)) => 19:1 - 21:2;
89
+
910
bb0: {
1011
+ Coverage::CounterIncrement(0);

tests/mir-opt/coverage/instrument_coverage.main.InstrumentCoverage.diff

+6-5
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,13 @@
77
let mut _2: bool;
88
let mut _3: !;
99

10+
+ coverage body span: $DIR/instrument_coverage.rs:10:11: 16:2 (#0)
1011
+ coverage ExpressionId(0) => Expression { lhs: Counter(0), op: Add, rhs: Counter(1) };
11-
+ coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:10:1 - 10:11;
12-
+ coverage Code(Expression(0)) => $DIR/instrument_coverage.rs:12:12 - 12:17;
13-
+ coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:13:13 - 13:18;
14-
+ coverage Code(Counter(1)) => $DIR/instrument_coverage.rs:14:10 - 14:11;
15-
+ coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:16:1 - 16:2;
12+
+ coverage Code(Counter(0)) => 10:1 - 10:11;
13+
+ coverage Code(Expression(0)) => 12:12 - 12:17;
14+
+ coverage Code(Counter(0)) => 13:13 - 13:18;
15+
+ coverage Code(Counter(1)) => 14:10 - 14:11;
16+
+ coverage Code(Counter(0)) => 16:1 - 16:2;
1617
+
1718
bb0: {
1819
+ Coverage::CounterIncrement(0);

tests/mir-opt/coverage/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff

+6-5
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,13 @@
77

88
coverage branch { true: BlockMarkerId(0), false: BlockMarkerId(1) } => $DIR/instrument_coverage_cleanup.rs:14:8: 14:36 (#0)
99

10+
coverage body span: $DIR/instrument_coverage_cleanup.rs:13:11: 15:2 (#0)
1011
coverage ExpressionId(0) => Expression { lhs: Counter(0), op: Subtract, rhs: Counter(1) };
11-
coverage Code(Counter(0)) => $DIR/instrument_coverage_cleanup.rs:13:1 - 14:36;
12-
coverage Code(Expression(0)) => $DIR/instrument_coverage_cleanup.rs:14:37 - 14:39;
13-
coverage Code(Counter(1)) => $DIR/instrument_coverage_cleanup.rs:14:39 - 14:40;
14-
coverage Code(Counter(0)) => $DIR/instrument_coverage_cleanup.rs:15:1 - 15:2;
15-
coverage Branch { true_term: Expression(0), false_term: Counter(1) } => $DIR/instrument_coverage_cleanup.rs:14:8 - 14:36;
12+
coverage Code(Counter(0)) => 13:1 - 14:36;
13+
coverage Code(Expression(0)) => 14:37 - 14:39;
14+
coverage Code(Counter(1)) => 14:39 - 14:40;
15+
coverage Code(Counter(0)) => 15:1 - 15:2;
16+
coverage Branch { true_term: Expression(0), false_term: Counter(1) } => 14:8 - 14:36;
1617

1718
bb0: {
1819
Coverage::CounterIncrement(0);

0 commit comments

Comments
 (0)