Rollup merge of rust-lang#121135 - Zalathar:no-whole-body-span, r=wes…

…leywiser coverage: Discard spans that fill the entire function body While debugging some other coverage changes, I discovered a frustrating inconsistency that occurs in functions containing closures, if they end with an implicit `()` return instead of an explicit trailing-expression. This turns out to have been caused by the corresponding node in MIR having a span that covers the entire function body. When preparing coverage spans, any span that fills the whole body tends to cause more harm than good, so this PR detects and discards those spans. (This isn't the first time whole-body spans have caused problems; we also eliminated some of them in rust-lang#118525.)
Nadrieril · Feb 17, 2024 · 134d9d0 · 134d9d0
2 parents 7fedf99 + cd9021e
commit 134d9d0
Show file tree

Hide file tree

Showing 11 changed files with 117 additions and 28 deletions.
diff --git a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs
@@ -132,18 +132,23 @@ fn bcb_to_initial_coverage_spans<'a, 'tcx>(
     bcb_data.basic_blocks.iter().flat_map(move |&bb| {
         let data = &mir_body[bb];
 
+        let unexpand = move |expn_span| {
+            unexpand_into_body_span_with_visible_macro(expn_span, body_span)
+                // Discard any spans that fill the entire body, because they tend
+                // to represent compiler-inserted code, e.g. implicitly returning `()`.
+                .filter(|(span, _)| !span.source_equal(body_span))
+        };
+
         let statement_spans = data.statements.iter().filter_map(move |statement| {
             let expn_span = filtered_statement_span(statement)?;
-            let (span, visible_macro) =
-                unexpand_into_body_span_with_visible_macro(expn_span, body_span)?;
+            let (span, visible_macro) = unexpand(expn_span)?;
 
             Some(SpanFromMir::new(span, visible_macro, bcb, is_closure_like(statement)))
         });
 
         let terminator_span = Some(data.terminator()).into_iter().filter_map(move |terminator| {
             let expn_span = filtered_terminator_span(terminator)?;
-            let (span, visible_macro) =
-                unexpand_into_body_span_with_visible_macro(expn_span, body_span)?;
+            let (span, visible_macro) = unexpand(expn_span)?;
 
             Some(SpanFromMir::new(span, visible_macro, bcb, false))
         });

diff --git a/tests/coverage/closure_unit_return.cov-map b/tests/coverage/closure_unit_return.cov-map
@@ -0,0 +1,34 @@
+Function name: closure_unit_return::explicit_unit
+Raw bytes (14): 0x[01, 01, 00, 02, 01, 07, 01, 01, 10, 01, 05, 05, 02, 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 + 7, 1) to (start + 1, 16)
+- Code(Counter(0)) at (prev + 5, 5) to (start + 2, 2)
+
+Function name: closure_unit_return::explicit_unit::{closure#0} (unused)
+Raw bytes (9): 0x[01, 01, 00, 01, 00, 08, 16, 02, 06]
+Number of files: 1
+- file 0 => global file 1
+Number of expressions: 0
+Number of file 0 mappings: 1
+- Code(Zero) at (prev + 8, 22) to (start + 2, 6)
+
+Function name: closure_unit_return::implicit_unit
+Raw bytes (14): 0x[01, 01, 00, 02, 01, 10, 01, 01, 10, 01, 05, 05, 02, 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 + 1, 16)
+- Code(Counter(0)) at (prev + 5, 5) to (start + 2, 2)
+
+Function name: closure_unit_return::implicit_unit::{closure#0} (unused)
+Raw bytes (9): 0x[01, 01, 00, 01, 00, 11, 16, 02, 06]
+Number of files: 1
+- file 0 => global file 1
+Number of expressions: 0
+Number of file 0 mappings: 1
+- Code(Zero) at (prev + 17, 22) to (start + 2, 6)
+
diff --git a/tests/coverage/closure_unit_return.coverage b/tests/coverage/closure_unit_return.coverage
@@ -0,0 +1,30 @@
+   LL|       |#![feature(coverage_attribute)]
+   LL|       |// edition: 2021
+   LL|       |
+   LL|       |// Regression test for an inconsistency between functions that return the value
+   LL|       |// of their trailing expression, and functions that implicitly return `()`.
+   LL|       |
+   LL|      1|fn explicit_unit() {
+   LL|      1|    let closure = || {
+   LL|      0|        ();
+   LL|      0|    };
+   LL|       |
+   LL|      1|    drop(closure);
+   LL|      1|    () // explicit return of trailing value
+   LL|      1|}
+   LL|       |
+   LL|      1|fn implicit_unit() {
+   LL|      1|    let closure = || {
+   LL|      0|        ();
+   LL|      0|    };
+   LL|       |
+   LL|      1|    drop(closure);
+   LL|      1|    // implicit return of `()`
+   LL|      1|}
+   LL|       |
+   LL|       |#[coverage(off)]
+   LL|       |fn main() {
+   LL|       |    explicit_unit();
+   LL|       |    implicit_unit();
+   LL|       |}
+
diff --git a/tests/coverage/closure_unit_return.rs b/tests/coverage/closure_unit_return.rs
@@ -0,0 +1,29 @@
+#![feature(coverage_attribute)]
+// edition: 2021
+
+// Regression test for an inconsistency between functions that return the value
+// of their trailing expression, and functions that implicitly return `()`.
+
+fn explicit_unit() {
+    let closure = || {
+        ();
+    };
+
+    drop(closure);
+    () // explicit return of trailing value
+}
+
+fn implicit_unit() {
+    let closure = || {
+        ();
+    };
+
+    drop(closure);
+    // implicit return of `()`
+}
+
+#[coverage(off)]
+fn main() {
+    explicit_unit();
+    implicit_unit();
+}
diff --git a/tests/coverage/coverage_attr_closure.cov-map b/tests/coverage/coverage_attr_closure.cov-map
@@ -15,14 +15,14 @@ Number of file 0 mappings: 1
 - Code(Zero) at (prev + 29, 19) to (start + 2, 6)
 
 Function name: coverage_attr_closure::contains_closures_on
-Raw bytes (19): 0x[01, 01, 00, 03, 01, 0f, 01, 02, 05, 01, 04, 06, 02, 05, 01, 04, 06, 01, 02]
+Raw bytes (19): 0x[01, 01, 00, 03, 01, 0f, 01, 01, 1a, 01, 05, 09, 00, 1b, 01, 04, 01, 00, 02]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 0
 Number of file 0 mappings: 3
-- Code(Counter(0)) at (prev + 15, 1) to (start + 2, 5)
-- Code(Counter(0)) at (prev + 4, 6) to (start + 2, 5)
-- Code(Counter(0)) at (prev + 4, 6) to (start + 1, 2)
+- Code(Counter(0)) at (prev + 15, 1) to (start + 1, 26)
+- Code(Counter(0)) at (prev + 5, 9) to (start + 0, 27)
+- Code(Counter(0)) at (prev + 4, 1) to (start + 0, 2)
 
 Function name: coverage_attr_closure::contains_closures_on::{closure#0} (unused)
 Raw bytes (9): 0x[01, 01, 00, 01, 00, 11, 13, 02, 06]

diff --git a/tests/coverage/coverage_attr_closure.coverage b/tests/coverage/coverage_attr_closure.coverage
@@ -14,13 +14,13 @@
    LL|       |#[coverage(on)]
    LL|      1|fn contains_closures_on() {
    LL|      1|    let _local_closure_on = #[coverage(on)]
-   LL|      1|    |input: &str| {
+   LL|      0|    |input: &str| {
    LL|      0|        println!("{input}");
-   LL|      1|    };
+   LL|      0|    };
    LL|      1|    let _local_closure_off = #[coverage(off)]
-   LL|      1|    |input: &str| {
+   LL|       |    |input: &str| {
    LL|       |        println!("{input}");
-   LL|      1|    };
+   LL|       |    };
    LL|      1|}
    LL|       |
    LL|       |#[coverage(off)]

diff --git a/tests/coverage/inline-dead.cov-map b/tests/coverage/inline-dead.cov-map
@@ -22,13 +22,13 @@ Number of file 0 mappings: 4
     = (Zero + (c0 - Zero))
 
 Function name: inline_dead::main
-Raw bytes (14): 0x[01, 01, 00, 02, 01, 04, 01, 03, 0d, 01, 05, 06, 02, 02]
+Raw bytes (14): 0x[01, 01, 00, 02, 01, 04, 01, 03, 0a, 01, 06, 05, 01, 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 + 4, 1) to (start + 3, 13)
-- Code(Counter(0)) at (prev + 5, 6) to (start + 2, 2)
+- Code(Counter(0)) at (prev + 4, 1) to (start + 3, 10)
+- Code(Counter(0)) at (prev + 6, 5) to (start + 1, 2)
 
 Function name: inline_dead::main::{closure#0}
 Raw bytes (23): 0x[01, 01, 02, 00, 06, 01, 00, 03, 01, 07, 17, 01, 16, 00, 01, 17, 00, 18, 03, 01, 05, 00, 06]

diff --git a/tests/coverage/macro_name_span.cov-map b/tests/coverage/macro_name_span.cov-map
@@ -1,10 +1,10 @@
 Function name: macro_name_span::affected_function
-Raw bytes (9): 0x[01, 01, 00, 01, 01, 16, 1c, 02, 06]
+Raw bytes (9): 0x[01, 01, 00, 01, 01, 16, 1c, 01, 40]
 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, 28) to (start + 2, 6)
+- Code(Counter(0)) at (prev + 22, 28) to (start + 1, 64)
 
 Function name: macro_name_span::main
 Raw bytes (9): 0x[01, 01, 00, 01, 01, 0b, 01, 02, 02]

diff --git a/tests/coverage/macro_name_span.coverage b/tests/coverage/macro_name_span.coverage
@@ -21,6 +21,6 @@
    LL|       |macro_name_span_helper::macro_that_defines_a_function! {
    LL|      1|    fn affected_function() {
    LL|      1|        macro_with_an_unreasonably_and_egregiously_long_name!();
-   LL|      1|    }
+   LL|       |    }
    LL|       |}
 
diff --git a/tests/coverage/unicode.cov-map b/tests/coverage/unicode.cov-map
@@ -27,14 +27,6 @@ Number of file 0 mappings: 9
 - Code(Expression(5, Add)) at (prev + 2, 5) to (start + 1, 2)
     = (c4 + ((((c0 + c1) - c1) - c2) + c3))
 
-Function name: unicode::サビ
-Raw bytes (9): 0x[01, 01, 00, 01, 01, 1e, 14, 00, 18]
-Number of files: 1
-- file 0 => global file 1
-Number of expressions: 0
-Number of file 0 mappings: 1
-- Code(Counter(0)) at (prev + 30, 20) to (start + 0, 24)
-
 Function name: unicode::他 (unused)
 Raw bytes (9): 0x[01, 01, 00, 01, 00, 1e, 19, 00, 25]
 Number of files: 1

diff --git a/tests/coverage/unicode.coverage b/tests/coverage/unicode.coverage
@@ -29,8 +29,7 @@
    LL|       |
    LL|       |macro_rules! macro_that_defines_a_function {
    LL|       |    (fn $名:ident () $体:tt) => {
-   LL|      [0;35m1[0m|        fn $名 () $体 [0;41mfn 他 () {}[0m
-                                      ^0
+   LL|      0|        fn $名 () $体 [0;41mfn 他 () {}[0m
    LL|       |    }
    LL|       |}
    LL|       |