coverage: Skip spans that can't be un-expanded back to the function body

Zalathar · Zalathar · commit eb2d4cb54143 · 2023-12-03T12:35:33.000+11:00
When we extract coverage spans from MIR, we try to "un-expand" them back to
spans that are inside the function's body span.

In cases where that doesn't succeed, the current code just swaps in the entire
body span instead. But that tends to result in coverage spans that are
completely unrelated to the control flow of the affected code, so it's better
to just discard those spans.
diff --git a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs
@@ -63,14 +63,14 @@ fn bcb_to_initial_coverage_spans<'a, 'tcx>(
 
         let statement_spans = data.statements.iter().filter_map(move |statement| {
             let expn_span = filtered_statement_span(statement)?;
-            let span = function_source_span(expn_span, body_span);
+            let span = unexpand_into_body_span(expn_span, body_span)?;
 
             Some(CoverageSpan::new(span, expn_span, bcb, is_closure(statement)))
         });
 
         let terminator_span = Some(data.terminator()).into_iter().filter_map(move |terminator| {
             let expn_span = filtered_terminator_span(terminator)?;
-            let span = function_source_span(expn_span, body_span);
+            let span = unexpand_into_body_span(expn_span, body_span)?;
 
             Some(CoverageSpan::new(span, expn_span, bcb, false))
         });
@@ -180,14 +180,16 @@ fn filtered_terminator_span(terminator: &Terminator<'_>) -> Option<Span> {
 /// Returns an extrapolated span (pre-expansion[^1]) corresponding to a range
 /// within the function's body source. This span is guaranteed to be contained
 /// within, or equal to, the `body_span`. If the extrapolated span is not
-/// contained within the `body_span`, the `body_span` is returned.
+/// contained within the `body_span`, `None` is returned.
 ///
 /// [^1]Expansions result from Rust syntax including macros, syntactic sugar,
 /// etc.).
 #[inline]
-fn function_source_span(span: Span, body_span: Span) -> Span {
+fn unexpand_into_body_span(span: Span, body_span: Span) -> Option<Span> {
     use rustc_span::source_map::original_sp;
 
+    // FIXME(#118525): Consider switching from `original_sp` to `Span::find_ancestor_inside`,
+    // which is similar but gives slightly different results in some edge cases.
     let original_span = original_sp(span, body_span).with_ctxt(body_span.ctxt());
-    if body_span.contains(original_span) { original_span } else { body_span }
+    body_span.contains(original_span).then_some(original_span)
 }
diff --git a/tests/coverage/async.cov-map b/tests/coverage/async.cov-map
@@ -74,28 +74,28 @@ Number of file 0 mappings: 6
     = ((c0 + c1) - c1)
 
 Function name: async::executor::block_on::VTABLE::{closure#0}
-Raw bytes (9): 0x[01, 01, 00, 01, 01, 72, 11, 00, 33]
+Raw bytes (9): 0x[01, 01, 00, 01, 01, 72, 11, 00, 31]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 0
 Number of file 0 mappings: 1
-- Code(Counter(0)) at (prev + 114, 17) to (start + 0, 51)
+- Code(Counter(0)) at (prev + 114, 17) to (start + 0, 49)
 
 Function name: async::executor::block_on::VTABLE::{closure#1}
-Raw bytes (9): 0x[01, 01, 00, 01, 01, 73, 11, 00, 33]
+Raw bytes (9): 0x[01, 01, 00, 01, 01, 73, 11, 00, 31]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 0
 Number of file 0 mappings: 1
-- Code(Counter(0)) at (prev + 115, 17) to (start + 0, 51)
+- Code(Counter(0)) at (prev + 115, 17) to (start + 0, 49)
 
 Function name: async::executor::block_on::VTABLE::{closure#2}
-Raw bytes (9): 0x[01, 01, 00, 01, 01, 74, 11, 00, 33]
+Raw bytes (9): 0x[01, 01, 00, 01, 01, 74, 11, 00, 31]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 0
 Number of file 0 mappings: 1
-- Code(Counter(0)) at (prev + 116, 17) to (start + 0, 51)
+- Code(Counter(0)) at (prev + 116, 17) to (start + 0, 49)
 
 Function name: async::executor::block_on::VTABLE::{closure#3}
 Raw bytes (9): 0x[01, 01, 00, 01, 01, 75, 11, 00, 13]
diff --git a/tests/coverage/async2.cov-map b/tests/coverage/async2.cov-map
@@ -78,28 +78,28 @@ Number of file 0 mappings: 6
     = ((c0 + c1) - c1)
 
 Function name: async2::executor::block_on::VTABLE::{closure#0}
-Raw bytes (9): 0x[01, 01, 00, 01, 01, 2b, 11, 00, 33]
+Raw bytes (9): 0x[01, 01, 00, 01, 01, 2b, 11, 00, 31]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 0
 Number of file 0 mappings: 1
-- Code(Counter(0)) at (prev + 43, 17) to (start + 0, 51)
+- Code(Counter(0)) at (prev + 43, 17) to (start + 0, 49)
 
 Function name: async2::executor::block_on::VTABLE::{closure#1}
-Raw bytes (9): 0x[01, 01, 00, 01, 01, 2c, 11, 00, 33]
+Raw bytes (9): 0x[01, 01, 00, 01, 01, 2c, 11, 00, 31]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 0
 Number of file 0 mappings: 1
-- Code(Counter(0)) at (prev + 44, 17) to (start + 0, 51)
+- Code(Counter(0)) at (prev + 44, 17) to (start + 0, 49)
 
 Function name: async2::executor::block_on::VTABLE::{closure#2}
-Raw bytes (9): 0x[01, 01, 00, 01, 01, 2d, 11, 00, 33]
+Raw bytes (9): 0x[01, 01, 00, 01, 01, 2d, 11, 00, 31]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 0
 Number of file 0 mappings: 1
-- Code(Counter(0)) at (prev + 45, 17) to (start + 0, 51)
+- Code(Counter(0)) at (prev + 45, 17) to (start + 0, 49)
 
 Function name: async2::executor::block_on::VTABLE::{closure#3}
 Raw bytes (9): 0x[01, 01, 00, 01, 01, 2e, 11, 00, 13]
diff --git a/tests/coverage/inline.cov-map b/tests/coverage/inline.cov-map
@@ -15,12 +15,12 @@ Number of file 0 mappings: 5
     = ((c0 + c1) - c1)
 
 Function name: inline::error
-Raw bytes (9): 0x[01, 01, 00, 01, 01, 31, 01, 02, 02]
+Raw bytes (9): 0x[01, 01, 00, 01, 01, 31, 01, 01, 14]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 0
 Number of file 0 mappings: 1
-- Code(Counter(0)) at (prev + 49, 1) to (start + 2, 2)
+- Code(Counter(0)) at (prev + 49, 1) to (start + 1, 20)
 
 Function name: inline::length::<char>
 Raw bytes (9): 0x[01, 01, 00, 01, 01, 1e, 01, 02, 02]
diff --git a/tests/coverage/inline.coverage b/tests/coverage/inline.coverage
@@ -50,5 +50,5 @@
    LL|       |#[inline(always)]
    LL|      0|fn error() {
    LL|      0|    panic!("error");
-   LL|      0|}
+   LL|       |}
 
diff --git a/tests/coverage/unreachable.cov-map b/tests/coverage/unreachable.cov-map
@@ -1,24 +1,24 @@
 Function name: unreachable::UNREACHABLE_CLOSURE::{closure#0}
-Raw bytes (9): 0x[01, 01, 00, 01, 01, 0f, 27, 00, 49]
+Raw bytes (9): 0x[01, 01, 00, 01, 01, 0f, 27, 00, 47]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 0
 Number of file 0 mappings: 1
-- Code(Counter(0)) at (prev + 15, 39) to (start + 0, 73)
+- Code(Counter(0)) at (prev + 15, 39) to (start + 0, 71)
 
 Function name: unreachable::unreachable_function
-Raw bytes (9): 0x[01, 01, 00, 01, 01, 11, 01, 02, 02]
+Raw bytes (9): 0x[01, 01, 00, 01, 01, 11, 01, 01, 25]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 0
 Number of file 0 mappings: 1
-- Code(Counter(0)) at (prev + 17, 1) to (start + 2, 2)
+- Code(Counter(0)) at (prev + 17, 1) to (start + 1, 37)
 
 Function name: unreachable::unreachable_intrinsic
-Raw bytes (9): 0x[01, 01, 00, 01, 01, 16, 01, 02, 02]
+Raw bytes (9): 0x[01, 01, 00, 01, 01, 16, 01, 01, 2c]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 0
 Number of file 0 mappings: 1
-- Code(Counter(0)) at (prev + 22, 1) to (start + 2, 2)
+- Code(Counter(0)) at (prev + 22, 1) to (start + 1, 44)
 
diff --git a/tests/coverage/unreachable.coverage b/tests/coverage/unreachable.coverage
@@ -16,12 +16,12 @@
    LL|       |
    LL|      0|fn unreachable_function() {
    LL|      0|    unsafe { unreachable_unchecked() }
-   LL|      0|}
+   LL|       |}
    LL|       |
    LL|       |// Use an intrinsic to more reliably trigger unreachable-propagation.
    LL|      0|fn unreachable_intrinsic() {
    LL|      0|    unsafe { std::intrinsics::unreachable() }
-   LL|      0|}
+   LL|       |}
    LL|       |
    LL|       |#[coverage(off)]
    LL|       |fn main() {
