Skip to content

Commit 8aa3d41

Browse files
jhprattjhpratt
authored
Rollup merge of #144480 - Zalathar:revert-empty-span, r=Zalathar
Revert "coverage: Enlarge empty spans during MIR instrumentation, not codegen" Surprisingly, #144298 alone (extracted from #140847) was enough to re-trigger the failures observed in #141577 (comment). --- This reverts commit f877aa7. --- r? ghost
2 parents d250b5c + 2b17897 commit 8aa3d41

File tree

8 files changed

+46
-57
lines changed

8 files changed

+46
-57
lines changed

‎compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/spans.rs‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,7 @@ impl Coords {
3939
/// or other expansions), and if it does happen then skipping a span or function is
4040
/// better than an ICE or `llvm-cov` failure that the user might have no way to avoid.
4141
pub(crate) fn make_coords(source_map: &SourceMap, file: &SourceFile, span: Span) -> Option<Coords> {
42-
if span.is_empty() {
43-
debug_assert!(false, "can't make coords from empty span: {span:?}");
44-
return None;
45-
}
42+
let span = ensure_non_empty_span(source_map, span)?;
4643

4744
let lo = span.lo();
4845
let hi = span.hi();
@@ -73,6 +70,29 @@ pub(crate) fn make_coords(source_map: &SourceMap, file: &SourceFile, span: Span)
7370
})
7471
}
7572

73+
fn ensure_non_empty_span(source_map: &SourceMap, span: Span) -> Option<Span> {
74+
if !span.is_empty() {
75+
return Some(span);
76+
}
77+
78+
// The span is empty, so try to enlarge it to cover an adjacent '{' or '}'.
79+
source_map
80+
.span_to_source(span, |src, start, end| try {
81+
// Adjusting span endpoints by `BytePos(1)` is normally a bug,
82+
// but in this case we have specifically checked that the character
83+
// we're skipping over is one of two specific ASCII characters, so
84+
// adjusting by exactly 1 byte is correct.
85+
if src.as_bytes().get(end).copied() == Some(b'{') {
86+
Some(span.with_hi(span.hi() + BytePos(1)))
87+
} else if start > 0 && src.as_bytes()[start - 1] == b'}' {
88+
Some(span.with_lo(span.lo() - BytePos(1)))
89+
} else {
90+
None
91+
}
92+
})
93+
.ok()?
94+
}
95+
7696
/// If `llvm-cov` sees a source region that is improperly ordered (end < start),
7797
/// it will immediately exit with a fatal error. To prevent that from happening,
7898
/// discard regions that are improperly ordered, or might be interpreted in a

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

Lines changed: 2 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
use rustc_data_structures::fx::FxHashSet;
22
use rustc_middle::mir;
33
use rustc_middle::ty::TyCtxt;
4-
use rustc_span::source_map::SourceMap;
5-
use rustc_span::{BytePos, DesugaringKind, ExpnKind, MacroKind, Span};
4+
use rustc_span::{DesugaringKind, ExpnKind, MacroKind, Span};
65
use tracing::instrument;
76

87
use crate::coverage::graph::{BasicCoverageBlock, CoverageGraph};
@@ -84,18 +83,8 @@ pub(super) fn extract_refined_covspans<'tcx>(
8483
// Discard any span that overlaps with a hole.
8584
discard_spans_overlapping_holes(&mut covspans, &holes);
8685

87-
// Discard spans that overlap in unwanted ways.
86+
// Perform more refinement steps after holes have been dealt with.
8887
let mut covspans = remove_unwanted_overlapping_spans(covspans);
89-
90-
// For all empty spans, either enlarge them to be non-empty, or discard them.
91-
let source_map = tcx.sess.source_map();
92-
covspans.retain_mut(|covspan| {
93-
let Some(span) = ensure_non_empty_span(source_map, covspan.span) else { return false };
94-
covspan.span = span;
95-
true
96-
});
97-
98-
// Merge covspans that can be merged.
9988
covspans.dedup_by(|b, a| a.merge_if_eligible(b));
10089

10190
code_mappings.extend(covspans.into_iter().map(|Covspan { span, bcb }| {
@@ -241,26 +230,3 @@ fn compare_spans(a: Span, b: Span) -> std::cmp::Ordering {
241230
// - Both have the same start and span A extends further right
242231
.then_with(|| Ord::cmp(&a.hi(), &b.hi()).reverse())
243232
}
244-
245-
fn ensure_non_empty_span(source_map: &SourceMap, span: Span) -> Option<Span> {
246-
if !span.is_empty() {
247-
return Some(span);
248-
}
249-
250-
// The span is empty, so try to enlarge it to cover an adjacent '{' or '}'.
251-
source_map
252-
.span_to_source(span, |src, start, end| try {
253-
// Adjusting span endpoints by `BytePos(1)` is normally a bug,
254-
// but in this case we have specifically checked that the character
255-
// we're skipping over is one of two specific ASCII characters, so
256-
// adjusting by exactly 1 byte is correct.
257-
if src.as_bytes().get(end).copied() == Some(b'{') {
258-
Some(span.with_hi(span.hi() + BytePos(1)))
259-
} else if start > 0 && src.as_bytes()[start - 1] == b'}' {
260-
Some(span.with_lo(span.lo() - BytePos(1)))
261-
} else {
262-
None
263-
}
264-
})
265-
.ok()?
266-
}

‎tests/coverage/async_closure.cov-map‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -37,29 +37,32 @@ Number of file 0 mappings: 8
3737
Highest counter ID seen: c0
3838

3939
Function name: async_closure::main::{closure#0}
40-
Raw bytes (9): 0x[01, 01, 00, 01, 01, 0b, 22, 00, 24]
40+
Raw bytes (14): 0x[01, 01, 00, 02, 01, 0b, 22, 00, 23, 01, 00, 23, 00, 24]
4141
Number of files: 1
4242
- file 0 => $DIR/async_closure.rs
4343
Number of expressions: 0
44-
Number of file 0 mappings: 1
45-
- Code(Counter(0)) at (prev + 11, 34) to (start + 0, 36)
44+
Number of file 0 mappings: 2
45+
- Code(Counter(0)) at (prev + 11, 34) to (start + 0, 35)
46+
- Code(Counter(0)) at (prev + 0, 35) to (start + 0, 36)
4647
Highest counter ID seen: c0
4748

4849
Function name: async_closure::main::{closure#0}
49-
Raw bytes (9): 0x[01, 01, 00, 01, 01, 0b, 22, 00, 24]
50+
Raw bytes (14): 0x[01, 01, 00, 02, 01, 0b, 22, 00, 23, 01, 00, 23, 00, 24]
5051
Number of files: 1
5152
- file 0 => $DIR/async_closure.rs
5253
Number of expressions: 0
53-
Number of file 0 mappings: 1
54-
- Code(Counter(0)) at (prev + 11, 34) to (start + 0, 36)
54+
Number of file 0 mappings: 2
55+
- Code(Counter(0)) at (prev + 11, 34) to (start + 0, 35)
56+
- Code(Counter(0)) at (prev + 0, 35) to (start + 0, 36)
5557
Highest counter ID seen: c0
5658

5759
Function name: async_closure::main::{closure#0}::{closure#0}::<i16>
58-
Raw bytes (9): 0x[01, 01, 00, 01, 01, 0b, 22, 00, 24]
60+
Raw bytes (14): 0x[01, 01, 00, 02, 01, 0b, 22, 00, 23, 01, 00, 23, 00, 24]
5961
Number of files: 1
6062
- file 0 => $DIR/async_closure.rs
6163
Number of expressions: 0
62-
Number of file 0 mappings: 1
63-
- Code(Counter(0)) at (prev + 11, 34) to (start + 0, 36)
64+
Number of file 0 mappings: 2
65+
- Code(Counter(0)) at (prev + 11, 34) to (start + 0, 35)
66+
- Code(Counter(0)) at (prev + 0, 35) to (start + 0, 36)
6467
Highest counter ID seen: c0
6568

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
+ coverage Code { bcb: bcb5 } => $DIR/branch_match_arms.rs:19:17: 19:18 (#0);
4141
+ coverage Code { bcb: bcb5 } => $DIR/branch_match_arms.rs:19:23: 19:30 (#0);
4242
+ coverage Code { bcb: bcb5 } => $DIR/branch_match_arms.rs:19:31: 19:32 (#0);
43-
+ coverage Code { bcb: bcb2 } => $DIR/branch_match_arms.rs:21:1: 21:2 (#0);
43+
+ coverage Code { bcb: bcb2 } => $DIR/branch_match_arms.rs:21:2: 21:2 (#0);
4444
+
4545
bb0: {
4646
+ Coverage::VirtualCounter(bcb0);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
+ coverage Code { bcb: bcb0 } => $DIR/instrument_coverage.rs:27:1: 27:17 (#0);
88
+ coverage Code { bcb: bcb0 } => $DIR/instrument_coverage.rs:28:5: 28:9 (#0);
9-
+ coverage Code { bcb: bcb0 } => $DIR/instrument_coverage.rs:29:1: 29:2 (#0);
9+
+ coverage Code { bcb: bcb0 } => $DIR/instrument_coverage.rs:29:2: 29:2 (#0);
1010
+
1111
bb0: {
1212
+ Coverage::VirtualCounter(bcb0);

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@
1010
+ coverage Code { bcb: bcb0 } => $DIR/instrument_coverage.rs:13:1: 13:10 (#0);
1111
+ coverage Code { bcb: bcb1 } => $DIR/instrument_coverage.rs:15:12: 15:15 (#0);
1212
+ coverage Code { bcb: bcb2 } => $DIR/instrument_coverage.rs:16:13: 16:18 (#0);
13-
+ coverage Code { bcb: bcb3 } => $DIR/instrument_coverage.rs:17:9: 17:10 (#0);
14-
+ coverage Code { bcb: bcb2 } => $DIR/instrument_coverage.rs:19:1: 19:2 (#0);
13+
+ coverage Code { bcb: bcb3 } => $DIR/instrument_coverage.rs:17:10: 17:10 (#0);
14+
+ coverage Code { bcb: bcb2 } => $DIR/instrument_coverage.rs:19:2: 19:2 (#0);
1515
+
1616
bb0: {
1717
+ Coverage::VirtualCounter(bcb0);

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@
1010
coverage Code { bcb: bcb0 } => $DIR/instrument_coverage_cleanup.rs:13:1: 13:10 (#0);
1111
coverage Code { bcb: bcb0 } => $DIR/instrument_coverage_cleanup.rs:14:8: 14:36 (#0);
1212
coverage Code { bcb: bcb3 } => $DIR/instrument_coverage_cleanup.rs:14:37: 14:39 (#0);
13-
coverage Code { bcb: bcb1 } => $DIR/instrument_coverage_cleanup.rs:14:38: 14:39 (#0);
14-
coverage Code { bcb: bcb2 } => $DIR/instrument_coverage_cleanup.rs:15:1: 15:2 (#0);
13+
coverage Code { bcb: bcb1 } => $DIR/instrument_coverage_cleanup.rs:14:39: 14:39 (#0);
14+
coverage Code { bcb: bcb2 } => $DIR/instrument_coverage_cleanup.rs:15:2: 15:2 (#0);
1515
coverage Branch { true_bcb: bcb3, false_bcb: bcb1 } => $DIR/instrument_coverage_cleanup.rs:14:8: 14:36 (#0);
1616

1717
bb0: {

‎tests/mir-opt/coverage/instrument_coverage_cleanup.main.InstrumentCoverage.diff‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@
1010
+ coverage Code { bcb: bcb0 } => $DIR/instrument_coverage_cleanup.rs:13:1: 13:10 (#0);
1111
+ coverage Code { bcb: bcb0 } => $DIR/instrument_coverage_cleanup.rs:14:8: 14:36 (#0);
1212
+ coverage Code { bcb: bcb3 } => $DIR/instrument_coverage_cleanup.rs:14:37: 14:39 (#0);
13-
+ coverage Code { bcb: bcb1 } => $DIR/instrument_coverage_cleanup.rs:14:38: 14:39 (#0);
14-
+ coverage Code { bcb: bcb2 } => $DIR/instrument_coverage_cleanup.rs:15:1: 15:2 (#0);
13+
+ coverage Code { bcb: bcb1 } => $DIR/instrument_coverage_cleanup.rs:14:39: 14:39 (#0);
14+
+ coverage Code { bcb: bcb2 } => $DIR/instrument_coverage_cleanup.rs:15:2: 15:2 (#0);
1515
+ coverage Branch { true_bcb: bcb3, false_bcb: bcb1 } => $DIR/instrument_coverage_cleanup.rs:14:8: 14:36 (#0);
1616
+
1717
bb0: {

0 commit comments

Comments
 (0)