coverage: Don't split bang-macro spans, just truncate them

Author: Zalathar

Diff for: compiler/rustc_mir_transform/src/coverage/mappings.rs

@@ -96,7 +96,7 @@ pub(super) fn extract_all_mapping_info_from_mir<'tcx>(
         }
     } else {
         // Extract coverage spans from MIR statements/terminators as normal.
-        extract_refined_covspans(mir_body, hir_info, graph, &mut code_mappings);
+        extract_refined_covspans(tcx, mir_body, hir_info, graph, &mut code_mappings);
     }
 
     branch_pairs.extend(extract_branch_pairs(mir_body, hir_info, graph));

Diff for: compiler/rustc_mir_transform/src/coverage/spans.rs

@@ -3,6 +3,7 @@ use std::iter;
 
 use rustc_data_structures::fx::FxHashSet;
 use rustc_middle::mir;
+use rustc_middle::ty::TyCtxt;
 use rustc_span::{DesugaringKind, ExpnKind, MacroKind, Span};
 use tracing::{debug, debug_span, instrument};
 
@@ -12,8 +13,9 @@ use crate::coverage::{ExtractedHirInfo, mappings, unexpand};
 
 mod from_mir;
 
-pub(super) fn extract_refined_covspans(
-    mir_body: &mir::Body<'_>,
+pub(super) fn extract_refined_covspans<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    mir_body: &mir::Body<'tcx>,
     hir_info: &ExtractedHirInfo,
     graph: &CoverageGraph,
     code_mappings: &mut impl Extend<mappings::CodeMapping>,
@@ -51,7 +53,7 @@ pub(super) fn extract_refined_covspans(
     // First, perform the passes that need macro information.
     covspans.sort_by(|a, b| graph.cmp_in_dominator_order(a.bcb, b.bcb));
     remove_unwanted_expansion_spans(&mut covspans);
-    split_visible_macro_spans(&mut covspans);
+    shrink_visible_macro_spans(tcx, &mut covspans);
 
     // We no longer need the extra information in `SpanFromMir`, so convert to `Covspan`.
     let mut covspans = covspans.into_iter().map(SpanFromMir::into_covspan).collect::<Vec<_>>();
@@ -128,35 +130,16 @@ fn remove_unwanted_expansion_spans(covspans: &mut Vec<SpanFromMir>) {
 }
 
 /// When a span corresponds to a macro invocation that is visible from the
-/// function body, split it into two parts. The first part covers just the
-/// macro name plus `!`, and the second part covers the rest of the macro
-/// invocation. This seems to give better results for code that uses macros.
-fn split_visible_macro_spans(covspans: &mut Vec<SpanFromMir>) {
-    let mut extra_spans = vec![];
-
-    covspans.retain(|covspan| {
-        let Some(ExpnKind::Macro(MacroKind::Bang, visible_macro)) = covspan.expn_kind else {
-            return true;
-        };
-
-        let split_len = visible_macro.as_str().len() as u32 + 1;
-        let (before, after) = covspan.span.split_at(split_len);
-        if !covspan.span.contains(before) || !covspan.span.contains(after) {
-            // Something is unexpectedly wrong with the split point.
-            // The debug assertion in `split_at` will have already caught this,
-            // but in release builds it's safer to do nothing and maybe get a
-            // bug report for unexpected coverage, rather than risk an ICE.
-            return true;
+/// function body, truncate it to just the macro name plus `!`.
+/// This seems to give better results for code that uses macros.
+fn shrink_visible_macro_spans(tcx: TyCtxt<'_>, covspans: &mut Vec<SpanFromMir>) {
+    let source_map = tcx.sess.source_map();
+
+    for covspan in covspans {
+        if matches!(covspan.expn_kind, Some(ExpnKind::Macro(MacroKind::Bang, _))) {
+            covspan.span = source_map.span_through_char(covspan.span, '!');
         }
-
-        extra_spans.push(SpanFromMir::new(before, covspan.expn_kind.clone(), covspan.bcb));
-        extra_spans.push(SpanFromMir::new(after, covspan.expn_kind.clone(), covspan.bcb));
-        false // Discard the original covspan that we just split.
-    });
-
-    // The newly-split spans are added at the end, so any previous sorting
-    // is not preserved.
-    covspans.extend(extra_spans);
+    }
 }
 
 /// Uses the holes to divide the given covspans into buckets, such that:

Diff for: tests/coverage/abort.cov-map

@@ -34,14 +34,14 @@ Number of file 0 mappings: 13
 Highest counter ID seen: c4
 
 Function name: abort::might_abort
-Raw bytes (21): 0x[01, 01, 01, 01, 05, 03, 01, 03, 01, 01, 14, 05, 02, 09, 01, 24, 02, 02, 0c, 03, 02]
+Raw bytes (21): 0x[01, 01, 01, 01, 05, 03, 01, 03, 01, 01, 14, 05, 02, 09, 01, 0f, 02, 02, 0c, 03, 02]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 1
 - expression 0 operands: lhs = Counter(0), rhs = Counter(1)
 Number of file 0 mappings: 3
 - Code(Counter(0)) at (prev + 3, 1) to (start + 1, 20)
-- Code(Counter(1)) at (prev + 2, 9) to (start + 1, 36)
+- Code(Counter(1)) at (prev + 2, 9) to (start + 1, 15)
 - Code(Expression(0, Sub)) at (prev + 2, 12) to (start + 3, 2)
     = (c0 - c1)
 Highest counter ID seen: c1

Diff for: tests/coverage/assert_not.cov-map

@@ -1,13 +1,13 @@
 Function name: assert_not::main
-Raw bytes (29): 0x[01, 01, 00, 05, 01, 06, 01, 01, 12, 01, 02, 05, 00, 14, 01, 01, 05, 00, 14, 01, 01, 05, 00, 16, 01, 01, 01, 00, 02]
+Raw bytes (29): 0x[01, 01, 00, 05, 01, 06, 01, 01, 11, 01, 02, 05, 00, 13, 01, 01, 05, 00, 13, 01, 01, 05, 00, 15, 01, 01, 01, 00, 02]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 0
 Number of file 0 mappings: 5
-- Code(Counter(0)) at (prev + 6, 1) to (start + 1, 18)
-- Code(Counter(0)) at (prev + 2, 5) to (start + 0, 20)
-- Code(Counter(0)) at (prev + 1, 5) to (start + 0, 20)
-- Code(Counter(0)) at (prev + 1, 5) to (start + 0, 22)
+- Code(Counter(0)) at (prev + 6, 1) to (start + 1, 17)
+- Code(Counter(0)) at (prev + 2, 5) to (start + 0, 19)
+- Code(Counter(0)) at (prev + 1, 5) to (start + 0, 19)
+- Code(Counter(0)) at (prev + 1, 5) to (start + 0, 21)
 - Code(Counter(0)) at (prev + 1, 1) to (start + 0, 2)
 Highest counter ID seen: c0
 

Diff for: tests/coverage/bad_counter_ids.cov-map

@@ -1,10 +1,10 @@
 Function name: bad_counter_ids::eq_bad
-Raw bytes (14): 0x[01, 01, 00, 02, 01, 24, 01, 02, 1f, 00, 03, 01, 00, 02]
+Raw bytes (14): 0x[01, 01, 00, 02, 01, 24, 01, 02, 0f, 00, 03, 01, 00, 02]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 0
 Number of file 0 mappings: 2
-- Code(Counter(0)) at (prev + 36, 1) to (start + 2, 31)
+- Code(Counter(0)) at (prev + 36, 1) to (start + 2, 15)
 - Code(Zero) at (prev + 3, 1) to (start + 0, 2)
 Highest counter ID seen: c0
 
@@ -20,12 +20,12 @@ Number of file 0 mappings: 3
 Highest counter ID seen: c0
 
 Function name: bad_counter_ids::eq_good
-Raw bytes (14): 0x[01, 01, 00, 02, 01, 10, 01, 02, 1f, 01, 03, 01, 00, 02]
+Raw bytes (14): 0x[01, 01, 00, 02, 01, 10, 01, 02, 0f, 01, 03, 01, 00, 02]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 0
 Number of file 0 mappings: 2
-- Code(Counter(0)) at (prev + 16, 1) to (start + 2, 31)
+- Code(Counter(0)) at (prev + 16, 1) to (start + 2, 15)
 - Code(Counter(0)) at (prev + 3, 1) to (start + 0, 2)
 Highest counter ID seen: c0
 
@@ -41,12 +41,12 @@ Number of file 0 mappings: 3
 Highest counter ID seen: c0
 
 Function name: bad_counter_ids::ne_bad
-Raw bytes (14): 0x[01, 01, 00, 02, 01, 2e, 01, 02, 1f, 00, 03, 01, 00, 02]
+Raw bytes (14): 0x[01, 01, 00, 02, 01, 2e, 01, 02, 0f, 00, 03, 01, 00, 02]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 0
 Number of file 0 mappings: 2
-- Code(Counter(0)) at (prev + 46, 1) to (start + 2, 31)
+- Code(Counter(0)) at (prev + 46, 1) to (start + 2, 15)
 - Code(Zero) at (prev + 3, 1) to (start + 0, 2)
 Highest counter ID seen: c0
 
@@ -62,12 +62,12 @@ Number of file 0 mappings: 3
 Highest counter ID seen: c0
 
 Function name: bad_counter_ids::ne_good
-Raw bytes (14): 0x[01, 01, 00, 02, 01, 1a, 01, 02, 1f, 01, 03, 01, 00, 02]
+Raw bytes (14): 0x[01, 01, 00, 02, 01, 1a, 01, 02, 0f, 01, 03, 01, 00, 02]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 0
 Number of file 0 mappings: 2
-- Code(Counter(0)) at (prev + 26, 1) to (start + 2, 31)
+- Code(Counter(0)) at (prev + 26, 1) to (start + 2, 15)
 - Code(Counter(0)) at (prev + 3, 1) to (start + 0, 2)
 Highest counter ID seen: c0
 

Diff for: tests/coverage/branch/guard.cov-map

@@ -1,5 +1,5 @@
 Function name: guard::branch_match_guard
-Raw bytes (89): 0x[01, 01, 08, 05, 0d, 09, 05, 05, 0f, 0d, 11, 17, 1b, 01, 05, 1f, 11, 09, 0d, 0d, 01, 0c, 01, 01, 10, 02, 03, 0b, 00, 0c, 06, 01, 14, 02, 0a, 0d, 03, 0e, 00, 0f, 05, 00, 14, 00, 19, 20, 0d, 02, 00, 14, 00, 1e, 0d, 00, 1d, 02, 0a, 11, 03, 0e, 00, 0f, 02, 00, 14, 00, 19, 20, 11, 0a, 00, 14, 00, 1e, 11, 00, 1d, 02, 0a, 12, 03, 0e, 02, 0a, 01, 04, 01, 00, 02]
+Raw bytes (89): 0x[01, 01, 08, 05, 0d, 09, 05, 05, 0f, 0d, 11, 17, 1b, 01, 05, 1f, 11, 09, 0d, 0d, 01, 0c, 01, 01, 0e, 02, 03, 0b, 00, 0c, 06, 01, 14, 02, 0a, 0d, 03, 0e, 00, 0f, 05, 00, 14, 00, 19, 20, 0d, 02, 00, 14, 00, 1e, 0d, 00, 1d, 02, 0a, 11, 03, 0e, 00, 0f, 02, 00, 14, 00, 19, 20, 11, 0a, 00, 14, 00, 1e, 11, 00, 1d, 02, 0a, 12, 03, 0e, 02, 0a, 01, 04, 01, 00, 02]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 8
@@ -12,7 +12,7 @@ Number of expressions: 8
 - expression 6 operands: lhs = Expression(7, Add), rhs = Counter(4)
 - expression 7 operands: lhs = Counter(2), rhs = Counter(3)
 Number of file 0 mappings: 13
-- Code(Counter(0)) at (prev + 12, 1) to (start + 1, 16)
+- Code(Counter(0)) at (prev + 12, 1) to (start + 1, 14)
 - Code(Expression(0, Sub)) at (prev + 3, 11) to (start + 0, 12)
     = (c1 - c3)
 - Code(Expression(1, Sub)) at (prev + 1, 20) to (start + 2, 10)

Diff for: tests/coverage/branch/if-let.cov-map

@@ -1,11 +1,11 @@
 Function name: if_let::if_let
-Raw bytes (43): 0x[01, 01, 01, 01, 05, 07, 01, 0c, 01, 01, 10, 20, 02, 05, 03, 0c, 00, 13, 02, 00, 11, 00, 12, 01, 00, 16, 00, 1b, 02, 00, 1c, 02, 06, 05, 02, 0c, 02, 06, 01, 03, 05, 01, 02]
+Raw bytes (43): 0x[01, 01, 01, 01, 05, 07, 01, 0c, 01, 01, 0e, 20, 02, 05, 03, 0c, 00, 13, 02, 00, 11, 00, 12, 01, 00, 16, 00, 1b, 02, 00, 1c, 02, 06, 05, 02, 0c, 02, 06, 01, 03, 05, 01, 02]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 1
 - expression 0 operands: lhs = Counter(0), rhs = Counter(1)
 Number of file 0 mappings: 7
-- Code(Counter(0)) at (prev + 12, 1) to (start + 1, 16)
+- Code(Counter(0)) at (prev + 12, 1) to (start + 1, 14)
 - Branch { true: Expression(0, Sub), false: Counter(1) } at (prev + 3, 12) to (start + 0, 19)
     true  = (c0 - c1)
     false = c1

Diff for: tests/coverage/branch/if.cov-map

@@ -1,13 +1,13 @@
 Function name: if::branch_and
-Raw bytes (54): 0x[01, 01, 03, 01, 05, 05, 09, 01, 09, 08, 01, 2b, 01, 01, 10, 01, 03, 08, 00, 09, 20, 05, 02, 00, 08, 00, 09, 05, 00, 0d, 00, 0e, 20, 09, 06, 00, 0d, 00, 0e, 09, 00, 0f, 02, 06, 0a, 02, 0c, 02, 06, 01, 03, 01, 00, 02]
+Raw bytes (54): 0x[01, 01, 03, 01, 05, 05, 09, 01, 09, 08, 01, 2b, 01, 01, 0e, 01, 03, 08, 00, 09, 20, 05, 02, 00, 08, 00, 09, 05, 00, 0d, 00, 0e, 20, 09, 06, 00, 0d, 00, 0e, 09, 00, 0f, 02, 06, 0a, 02, 0c, 02, 06, 01, 03, 01, 00, 02]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 3
 - expression 0 operands: lhs = Counter(0), rhs = Counter(1)
 - expression 1 operands: lhs = Counter(1), rhs = Counter(2)
 - expression 2 operands: lhs = Counter(0), rhs = Counter(2)
 Number of file 0 mappings: 8
-- Code(Counter(0)) at (prev + 43, 1) to (start + 1, 16)
+- Code(Counter(0)) at (prev + 43, 1) to (start + 1, 14)
 - Code(Counter(0)) at (prev + 3, 8) to (start + 0, 9)
 - Branch { true: Counter(1), false: Expression(0, Sub) } at (prev + 0, 8) to (start + 0, 9)
     true  = c1
@@ -23,7 +23,7 @@ Number of file 0 mappings: 8
 Highest counter ID seen: c2
 
 Function name: if::branch_not
-Raw bytes (116): 0x[01, 01, 07, 01, 05, 01, 09, 01, 09, 01, 0d, 01, 0d, 01, 11, 01, 11, 12, 01, 0c, 01, 01, 10, 01, 03, 08, 00, 09, 20, 05, 02, 00, 08, 00, 09, 05, 01, 09, 00, 10, 02, 01, 05, 00, 06, 01, 01, 08, 00, 0a, 20, 0a, 09, 00, 08, 00, 0a, 0a, 00, 0b, 02, 06, 09, 02, 05, 00, 06, 01, 01, 08, 00, 0b, 20, 0d, 12, 00, 08, 00, 0b, 0d, 00, 0c, 02, 06, 12, 02, 05, 00, 06, 01, 01, 08, 00, 0c, 20, 1a, 11, 00, 08, 00, 0c, 1a, 00, 0d, 02, 06, 11, 02, 05, 00, 06, 01, 01, 01, 00, 02]
+Raw bytes (116): 0x[01, 01, 07, 01, 05, 01, 09, 01, 09, 01, 0d, 01, 0d, 01, 11, 01, 11, 12, 01, 0c, 01, 01, 0e, 01, 03, 08, 00, 09, 20, 05, 02, 00, 08, 00, 09, 05, 01, 09, 00, 10, 02, 01, 05, 00, 06, 01, 01, 08, 00, 0a, 20, 0a, 09, 00, 08, 00, 0a, 0a, 00, 0b, 02, 06, 09, 02, 05, 00, 06, 01, 01, 08, 00, 0b, 20, 0d, 12, 00, 08, 00, 0b, 0d, 00, 0c, 02, 06, 12, 02, 05, 00, 06, 01, 01, 08, 00, 0c, 20, 1a, 11, 00, 08, 00, 0c, 1a, 00, 0d, 02, 06, 11, 02, 05, 00, 06, 01, 01, 01, 00, 02]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 7
@@ -35,7 +35,7 @@ Number of expressions: 7
 - expression 5 operands: lhs = Counter(0), rhs = Counter(4)
 - expression 6 operands: lhs = Counter(0), rhs = Counter(4)
 Number of file 0 mappings: 18
-- Code(Counter(0)) at (prev + 12, 1) to (start + 1, 16)
+- Code(Counter(0)) at (prev + 12, 1) to (start + 1, 14)
 - Code(Counter(0)) at (prev + 3, 8) to (start + 0, 9)
 - Branch { true: Counter(1), false: Expression(0, Sub) } at (prev + 0, 8) to (start + 0, 9)
     true  = c1
@@ -68,7 +68,7 @@ Number of file 0 mappings: 18
 Highest counter ID seen: c4
 
 Function name: if::branch_not_as
-Raw bytes (90): 0x[01, 01, 05, 01, 05, 01, 09, 01, 09, 01, 0d, 01, 0d, 0e, 01, 1d, 01, 01, 10, 01, 03, 08, 00, 14, 20, 02, 05, 00, 08, 00, 14, 02, 00, 15, 02, 06, 05, 02, 05, 00, 06, 01, 01, 08, 00, 15, 20, 09, 0a, 00, 08, 00, 15, 09, 00, 16, 02, 06, 0a, 02, 05, 00, 06, 01, 01, 08, 00, 16, 20, 12, 0d, 00, 08, 00, 16, 12, 00, 17, 02, 06, 0d, 02, 05, 00, 06, 01, 01, 01, 00, 02]
+Raw bytes (90): 0x[01, 01, 05, 01, 05, 01, 09, 01, 09, 01, 0d, 01, 0d, 0e, 01, 1d, 01, 01, 0e, 01, 03, 08, 00, 14, 20, 02, 05, 00, 08, 00, 14, 02, 00, 15, 02, 06, 05, 02, 05, 00, 06, 01, 01, 08, 00, 15, 20, 09, 0a, 00, 08, 00, 15, 09, 00, 16, 02, 06, 0a, 02, 05, 00, 06, 01, 01, 08, 00, 16, 20, 12, 0d, 00, 08, 00, 16, 12, 00, 17, 02, 06, 0d, 02, 05, 00, 06, 01, 01, 01, 00, 02]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 5
@@ -78,7 +78,7 @@ Number of expressions: 5
 - expression 3 operands: lhs = Counter(0), rhs = Counter(3)
 - expression 4 operands: lhs = Counter(0), rhs = Counter(3)
 Number of file 0 mappings: 14
-- Code(Counter(0)) at (prev + 29, 1) to (start + 1, 16)
+- Code(Counter(0)) at (prev + 29, 1) to (start + 1, 14)
 - Code(Counter(0)) at (prev + 3, 8) to (start + 0, 20)
 - Branch { true: Expression(0, Sub), false: Counter(1) } at (prev + 0, 8) to (start + 0, 20)
     true  = (c0 - c1)
@@ -104,7 +104,7 @@ Number of file 0 mappings: 14
 Highest counter ID seen: c3
 
 Function name: if::branch_or
-Raw bytes (60): 0x[01, 01, 06, 01, 05, 01, 17, 05, 09, 05, 09, 01, 17, 05, 09, 08, 01, 35, 01, 01, 10, 01, 03, 08, 00, 09, 20, 05, 02, 00, 08, 00, 09, 02, 00, 0d, 00, 0e, 20, 09, 12, 00, 0d, 00, 0e, 17, 00, 0f, 02, 06, 12, 02, 0c, 02, 06, 01, 03, 01, 00, 02]
+Raw bytes (60): 0x[01, 01, 06, 01, 05, 01, 17, 05, 09, 05, 09, 01, 17, 05, 09, 08, 01, 35, 01, 01, 0e, 01, 03, 08, 00, 09, 20, 05, 02, 00, 08, 00, 09, 02, 00, 0d, 00, 0e, 20, 09, 12, 00, 0d, 00, 0e, 17, 00, 0f, 02, 06, 12, 02, 0c, 02, 06, 01, 03, 01, 00, 02]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 6
@@ -115,7 +115,7 @@ Number of expressions: 6
 - expression 4 operands: lhs = Counter(0), rhs = Expression(5, Add)
 - expression 5 operands: lhs = Counter(1), rhs = Counter(2)
 Number of file 0 mappings: 8
-- Code(Counter(0)) at (prev + 53, 1) to (start + 1, 16)
+- Code(Counter(0)) at (prev + 53, 1) to (start + 1, 14)
 - Code(Counter(0)) at (prev + 3, 8) to (start + 0, 9)
 - Branch { true: Counter(1), false: Expression(0, Sub) } at (prev + 0, 8) to (start + 0, 9)
     true  = c1