diff options
| author | Zalathar <Zalathar@users.noreply.github.com> | 2025-05-04 14:09:52 +1000 |
|---|---|---|
| committer | Zalathar <Zalathar@users.noreply.github.com> | 2025-07-30 13:17:05 +1000 |
| commit | 2e6f4a59226fd82c526df0f61fefc6fea44e7750 (patch) | |
| tree | 533ed08b5427ab26f804269347c14e555fb6be4d | |
| parent | ba7e63b63871a429533c189adbfb1d9a6337e000 (diff) | |
| download | rust-2e6f4a59226fd82c526df0f61fefc6fea44e7750.tar.gz rust-2e6f4a59226fd82c526df0f61fefc6fea44e7750.zip | |
coverage: Re-land "Enlarge empty spans during MIR instrumentation"
This allows us to assume that coverage spans will only be discarded during codegen in very unusual situations.
11 files changed, 57 insertions, 73 deletions
diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/spans.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/spans.rs index 39a59560c9d..574463be7ff 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/spans.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/spans.rs @@ -39,7 +39,10 @@ impl Coords { /// or other expansions), and if it does happen then skipping a span or function is /// better than an ICE or `llvm-cov` failure that the user might have no way to avoid. pub(crate) fn make_coords(source_map: &SourceMap, file: &SourceFile, span: Span) -> Option<Coords> { - let span = ensure_non_empty_span(source_map, span)?; + if span.is_empty() { + debug_assert!(false, "can't make coords from empty span: {span:?}"); + return None; + } let lo = span.lo(); let hi = span.hi(); @@ -70,29 +73,6 @@ pub(crate) fn make_coords(source_map: &SourceMap, file: &SourceFile, span: Span) }) } -fn ensure_non_empty_span(source_map: &SourceMap, span: Span) -> Option<Span> { - if !span.is_empty() { - return Some(span); - } - - // The span is empty, so try to enlarge it to cover an adjacent '{' or '}'. - source_map - .span_to_source(span, |src, start, end| try { - // Adjusting span endpoints by `BytePos(1)` is normally a bug, - // but in this case we have specifically checked that the character - // we're skipping over is one of two specific ASCII characters, so - // adjusting by exactly 1 byte is correct. - if src.as_bytes().get(end).copied() == Some(b'{') { - Some(span.with_hi(span.hi() + BytePos(1))) - } else if start > 0 && src.as_bytes()[start - 1] == b'}' { - Some(span.with_lo(span.lo() - BytePos(1))) - } else { - None - } - }) - .ok()? -} - /// If `llvm-cov` sees a source region that is improperly ordered (end < start), /// it will immediately exit with a fatal error. To prevent that from happening, /// discard regions that are improperly ordered, or might be interpreted in a diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index ec76076020e..ddeae093df5 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -1,7 +1,8 @@ use rustc_data_structures::fx::FxHashSet; use rustc_middle::mir; use rustc_middle::ty::TyCtxt; -use rustc_span::{DesugaringKind, ExpnKind, MacroKind, Span}; +use rustc_span::source_map::SourceMap; +use rustc_span::{BytePos, DesugaringKind, ExpnKind, MacroKind, Span}; use tracing::instrument; use crate::coverage::graph::{BasicCoverageBlock, CoverageGraph}; @@ -83,8 +84,18 @@ pub(super) fn extract_refined_covspans<'tcx>( // Discard any span that overlaps with a hole. discard_spans_overlapping_holes(&mut covspans, &holes); - // Perform more refinement steps after holes have been dealt with. + // Discard spans that overlap in unwanted ways. let mut covspans = remove_unwanted_overlapping_spans(covspans); + + // For all empty spans, either enlarge them to be non-empty, or discard them. + let source_map = tcx.sess.source_map(); + covspans.retain_mut(|covspan| { + let Some(span) = ensure_non_empty_span(source_map, covspan.span) else { return false }; + covspan.span = span; + true + }); + + // Merge covspans that can be merged. covspans.dedup_by(|b, a| a.merge_if_eligible(b)); code_mappings.extend(covspans.into_iter().map(|Covspan { span, bcb }| { @@ -230,3 +241,26 @@ fn compare_spans(a: Span, b: Span) -> std::cmp::Ordering { // - Both have the same start and span A extends further right .then_with(|| Ord::cmp(&a.hi(), &b.hi()).reverse()) } + +fn ensure_non_empty_span(source_map: &SourceMap, span: Span) -> Option<Span> { + if !span.is_empty() { + return Some(span); + } + + // The span is empty, so try to enlarge it to cover an adjacent '{' or '}'. + source_map + .span_to_source(span, |src, start, end| try { + // Adjusting span endpoints by `BytePos(1)` is normally a bug, + // but in this case we have specifically checked that the character + // we're skipping over is one of two specific ASCII characters, so + // adjusting by exactly 1 byte is correct. + if src.as_bytes().get(end).copied() == Some(b'{') { + Some(span.with_hi(span.hi() + BytePos(1))) + } else if start > 0 && src.as_bytes()[start - 1] == b'}' { + Some(span.with_lo(span.lo() - BytePos(1))) + } else { + None + } + }) + .ok()? +} diff --git a/tests/coverage/async_closure.cov-map b/tests/coverage/async_closure.cov-map index 9f8dc8d6cbb..53128dd7a48 100644 --- a/tests/coverage/async_closure.cov-map +++ b/tests/coverage/async_closure.cov-map @@ -37,32 +37,29 @@ Number of file 0 mappings: 8 Highest counter ID seen: c0 Function name: async_closure::main::{closure#0} -Raw bytes (14): 0x[01, 01, 00, 02, 01, 0b, 22, 00, 23, 01, 00, 23, 00, 24] +Raw bytes (9): 0x[01, 01, 00, 01, 01, 0b, 22, 00, 24] Number of files: 1 - file 0 => $DIR/async_closure.rs Number of expressions: 0 -Number of file 0 mappings: 2 -- Code(Counter(0)) at (prev + 11, 34) to (start + 0, 35) -- Code(Counter(0)) at (prev + 0, 35) to (start + 0, 36) +Number of file 0 mappings: 1 +- Code(Counter(0)) at (prev + 11, 34) to (start + 0, 36) Highest counter ID seen: c0 Function name: async_closure::main::{closure#0} -Raw bytes (14): 0x[01, 01, 00, 02, 01, 0b, 22, 00, 23, 01, 00, 23, 00, 24] +Raw bytes (9): 0x[01, 01, 00, 01, 01, 0b, 22, 00, 24] Number of files: 1 - file 0 => $DIR/async_closure.rs Number of expressions: 0 -Number of file 0 mappings: 2 -- Code(Counter(0)) at (prev + 11, 34) to (start + 0, 35) -- Code(Counter(0)) at (prev + 0, 35) to (start + 0, 36) +Number of file 0 mappings: 1 +- Code(Counter(0)) at (prev + 11, 34) to (start + 0, 36) Highest counter ID seen: c0 Function name: async_closure::main::{closure#0}::{closure#0}::<i16> -Raw bytes (14): 0x[01, 01, 00, 02, 01, 0b, 22, 00, 23, 01, 00, 23, 00, 24] +Raw bytes (9): 0x[01, 01, 00, 01, 01, 0b, 22, 00, 24] Number of files: 1 - file 0 => $DIR/async_closure.rs Number of expressions: 0 -Number of file 0 mappings: 2 -- Code(Counter(0)) at (prev + 11, 34) to (start + 0, 35) -- Code(Counter(0)) at (prev + 0, 35) to (start + 0, 36) +Number of file 0 mappings: 1 +- Code(Counter(0)) at (prev + 11, 34) to (start + 0, 36) Highest counter ID seen: c0 diff --git a/tests/coverage/try-in-macro.attr.cov-map b/tests/coverage/try-in-macro.attr.cov-map index 7111e89637c..1b6c97cf145 100644 --- a/tests/coverage/try-in-macro.attr.cov-map +++ b/tests/coverage/try-in-macro.attr.cov-map @@ -1,12 +1,3 @@ -Function name: <try_in_macro::MyEnum as try_in_macro::Arbitrary>::try_size_hint -Raw bytes (9): 0x[01, 01, 00, 01, 00, 1e, 2a, 00, 2b] -Number of files: 1 -- file 0 => $DIR/try-in-macro.rs -Number of expressions: 0 -Number of file 0 mappings: 1 -- Code(Zero) at (prev + 30, 42) to (start + 0, 43) -Highest counter ID seen: (none) - Function name: try_in_macro::main Raw bytes (19): 0x[01, 01, 00, 03, 01, 29, 01, 00, 0a, 01, 01, 05, 00, 1a, 01, 01, 01, 00, 02] Number of files: 1 diff --git a/tests/coverage/try-in-macro.bang.cov-map b/tests/coverage/try-in-macro.bang.cov-map index 80bd91a993c..1b6c97cf145 100644 --- a/tests/coverage/try-in-macro.bang.cov-map +++ b/tests/coverage/try-in-macro.bang.cov-map @@ -1,12 +1,3 @@ -Function name: <try_in_macro::MyEnum as try_in_macro::Arbitrary>::try_size_hint -Raw bytes (9): 0x[01, 01, 00, 01, 00, 23, 1c, 00, 1d] -Number of files: 1 -- file 0 => $DIR/try-in-macro.rs -Number of expressions: 0 -Number of file 0 mappings: 1 -- Code(Zero) at (prev + 35, 28) to (start + 0, 29) -Highest counter ID seen: (none) - Function name: try_in_macro::main Raw bytes (19): 0x[01, 01, 00, 03, 01, 29, 01, 00, 0a, 01, 01, 05, 00, 1a, 01, 01, 01, 00, 02] Number of files: 1 diff --git a/tests/coverage/try-in-macro.derive.cov-map b/tests/coverage/try-in-macro.derive.cov-map index 6646b6693ba..1b6c97cf145 100644 --- a/tests/coverage/try-in-macro.derive.cov-map +++ b/tests/coverage/try-in-macro.derive.cov-map @@ -1,12 +1,3 @@ -Function name: <try_in_macro::MyEnum as try_in_macro::Arbitrary>::try_size_hint -Raw bytes (9): 0x[01, 01, 00, 01, 00, 26, 38, 00, 39] -Number of files: 1 -- file 0 => $DIR/try-in-macro.rs -Number of expressions: 0 -Number of file 0 mappings: 1 -- Code(Zero) at (prev + 38, 56) to (start + 0, 57) -Highest counter ID seen: (none) - Function name: try_in_macro::main Raw bytes (19): 0x[01, 01, 00, 03, 01, 29, 01, 00, 0a, 01, 01, 05, 00, 1a, 01, 01, 01, 00, 02] Number of files: 1 diff --git a/tests/mir-opt/coverage/branch_match_arms.main.InstrumentCoverage.diff b/tests/mir-opt/coverage/branch_match_arms.main.InstrumentCoverage.diff index d465b8bded2..fa88211383a 100644 --- a/tests/mir-opt/coverage/branch_match_arms.main.InstrumentCoverage.diff +++ b/tests/mir-opt/coverage/branch_match_arms.main.InstrumentCoverage.diff @@ -40,7 +40,7 @@ + coverage Code { bcb: bcb5 } => $DIR/branch_match_arms.rs:19:17: 19:18 (#0); + coverage Code { bcb: bcb5 } => $DIR/branch_match_arms.rs:19:23: 19:30 (#0); + coverage Code { bcb: bcb5 } => $DIR/branch_match_arms.rs:19:31: 19:32 (#0); -+ coverage Code { bcb: bcb2 } => $DIR/branch_match_arms.rs:21:2: 21:2 (#0); ++ coverage Code { bcb: bcb2 } => $DIR/branch_match_arms.rs:21:1: 21:2 (#0); + bb0: { + Coverage::VirtualCounter(bcb0); diff --git a/tests/mir-opt/coverage/instrument_coverage.bar.InstrumentCoverage.diff b/tests/mir-opt/coverage/instrument_coverage.bar.InstrumentCoverage.diff index cf6d85abd80..9b6d2b22087 100644 --- a/tests/mir-opt/coverage/instrument_coverage.bar.InstrumentCoverage.diff +++ b/tests/mir-opt/coverage/instrument_coverage.bar.InstrumentCoverage.diff @@ -6,7 +6,7 @@ + coverage Code { bcb: bcb0 } => $DIR/instrument_coverage.rs:27:1: 27:17 (#0); + coverage Code { bcb: bcb0 } => $DIR/instrument_coverage.rs:28:5: 28:9 (#0); -+ coverage Code { bcb: bcb0 } => $DIR/instrument_coverage.rs:29:2: 29:2 (#0); ++ coverage Code { bcb: bcb0 } => $DIR/instrument_coverage.rs:29:1: 29:2 (#0); + bb0: { + Coverage::VirtualCounter(bcb0); diff --git a/tests/mir-opt/coverage/instrument_coverage.main.InstrumentCoverage.diff b/tests/mir-opt/coverage/instrument_coverage.main.InstrumentCoverage.diff index 980c5e202ff..b2bb2375aee 100644 --- a/tests/mir-opt/coverage/instrument_coverage.main.InstrumentCoverage.diff +++ b/tests/mir-opt/coverage/instrument_coverage.main.InstrumentCoverage.diff @@ -10,8 +10,8 @@ + coverage Code { bcb: bcb0 } => $DIR/instrument_coverage.rs:13:1: 13:10 (#0); + coverage Code { bcb: bcb1 } => $DIR/instrument_coverage.rs:15:12: 15:15 (#0); + coverage Code { bcb: bcb2 } => $DIR/instrument_coverage.rs:16:13: 16:18 (#0); -+ coverage Code { bcb: bcb3 } => $DIR/instrument_coverage.rs:17:10: 17:10 (#0); -+ coverage Code { bcb: bcb2 } => $DIR/instrument_coverage.rs:19:2: 19:2 (#0); ++ coverage Code { bcb: bcb3 } => $DIR/instrument_coverage.rs:17:9: 17:10 (#0); ++ coverage Code { bcb: bcb2 } => $DIR/instrument_coverage.rs:19:1: 19:2 (#0); + bb0: { + Coverage::VirtualCounter(bcb0); diff --git a/tests/mir-opt/coverage/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff b/tests/mir-opt/coverage/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff index b707cd41788..2eb78c08ee8 100644 --- a/tests/mir-opt/coverage/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff +++ b/tests/mir-opt/coverage/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff @@ -10,8 +10,8 @@ coverage Code { bcb: bcb0 } => $DIR/instrument_coverage_cleanup.rs:13:1: 13:10 (#0); coverage Code { bcb: bcb0 } => $DIR/instrument_coverage_cleanup.rs:14:8: 14:36 (#0); coverage Code { bcb: bcb3 } => $DIR/instrument_coverage_cleanup.rs:14:37: 14:39 (#0); - coverage Code { bcb: bcb1 } => $DIR/instrument_coverage_cleanup.rs:14:39: 14:39 (#0); - coverage Code { bcb: bcb2 } => $DIR/instrument_coverage_cleanup.rs:15:2: 15:2 (#0); + coverage Code { bcb: bcb1 } => $DIR/instrument_coverage_cleanup.rs:14:38: 14:39 (#0); + coverage Code { bcb: bcb2 } => $DIR/instrument_coverage_cleanup.rs:15:1: 15:2 (#0); coverage Branch { true_bcb: bcb3, false_bcb: bcb1 } => $DIR/instrument_coverage_cleanup.rs:14:8: 14:36 (#0); bb0: { diff --git a/tests/mir-opt/coverage/instrument_coverage_cleanup.main.InstrumentCoverage.diff b/tests/mir-opt/coverage/instrument_coverage_cleanup.main.InstrumentCoverage.diff index 239b845c231..0c1bc24b6dc 100644 --- a/tests/mir-opt/coverage/instrument_coverage_cleanup.main.InstrumentCoverage.diff +++ b/tests/mir-opt/coverage/instrument_coverage_cleanup.main.InstrumentCoverage.diff @@ -10,8 +10,8 @@ + coverage Code { bcb: bcb0 } => $DIR/instrument_coverage_cleanup.rs:13:1: 13:10 (#0); + coverage Code { bcb: bcb0 } => $DIR/instrument_coverage_cleanup.rs:14:8: 14:36 (#0); + coverage Code { bcb: bcb3 } => $DIR/instrument_coverage_cleanup.rs:14:37: 14:39 (#0); -+ coverage Code { bcb: bcb1 } => $DIR/instrument_coverage_cleanup.rs:14:39: 14:39 (#0); -+ coverage Code { bcb: bcb2 } => $DIR/instrument_coverage_cleanup.rs:15:2: 15:2 (#0); ++ coverage Code { bcb: bcb1 } => $DIR/instrument_coverage_cleanup.rs:14:38: 14:39 (#0); ++ coverage Code { bcb: bcb2 } => $DIR/instrument_coverage_cleanup.rs:15:1: 15:2 (#0); + coverage Branch { true_bcb: bcb3, false_bcb: bcb1 } => $DIR/instrument_coverage_cleanup.rs:14:8: 14:36 (#0); + bb0: { |
