Auto merge of #144298 - Zalathar:empty-span, r=<try>

coverage: Enlarge empty spans during MIR instrumentation, not codegen

Author: rust-bors[bot]

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

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()?
+}

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
 

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);

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);

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);

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: {

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: {