Rollup merge of rust-lang#137251 - Zalathar:holes-visitor, r=jieyouxu

matthiaskrgr · web-flow · commit 3fc6dfd5edcd · 2025-02-19T18:52:10.000+01:00
coverage: Get hole spans from nested items without fully visiting them

This is a small simplification to the code that collects the spans of nested items within a function, so that those spans can be treated as “holes” to be avoided by the current function's coverage mappings.

The old code was using `nested_filter::All` to ensure that the visitor would see nested items. But we don't need the actual items themselves; we just need their spans, which we can obtain via a custom implementation of `visit_nested_item`.

This avoids the more expansive queries required by `nested_filter::All`.
diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs
@@ -346,35 +346,37 @@ fn extract_hole_spans_from_hir<'tcx>(
     body_span: Span, // Usually `hir_body.value.span`, but not always
     hir_body: &hir::Body<'tcx>,
 ) -> Vec<Span> {
-    struct HolesVisitor<'hir, F> {
-        tcx: TyCtxt<'hir>,
-        visit_hole_span: F,
+    struct HolesVisitor<'tcx> {
+        tcx: TyCtxt<'tcx>,
+        body_span: Span,
+        hole_spans: Vec<Span>,
     }
 
-    impl<'hir, F: FnMut(Span)> Visitor<'hir> for HolesVisitor<'hir, F> {
-        /// - We need `NestedFilter::INTRA = true` so that `visit_item` will be called.
-        /// - Bodies of nested items don't actually get visited, because of the
-        ///   `visit_item` override.
-        /// - For nested bodies that are not part of an item, we do want to visit any
-        ///   items contained within them.
-        type NestedFilter = nested_filter::All;
+    impl<'tcx> Visitor<'tcx> for HolesVisitor<'tcx> {
+        /// We have special handling for nested items, but we still want to
+        /// traverse into nested bodies of things that are not considered items,
+        /// such as "anon consts" (e.g. array lengths).
+        type NestedFilter = nested_filter::OnlyBodies;
 
-        fn maybe_tcx(&mut self) -> Self::MaybeTyCtxt {
+        fn maybe_tcx(&mut self) -> TyCtxt<'tcx> {
             self.tcx
         }
 
-        fn visit_item(&mut self, item: &'hir hir::Item<'hir>) {
-            (self.visit_hole_span)(item.span);
+        /// We override `visit_nested_item` instead of `visit_item` because we
+        /// only need the item's span, not the item itself.
+        fn visit_nested_item(&mut self, id: hir::ItemId) -> Self::Result {
+            let span = self.tcx.def_span(id.owner_id.def_id);
+            self.visit_hole_span(span);
             // Having visited this item, we don't care about its children,
             // so don't call `walk_item`.
         }
 
         // We override `visit_expr` instead of the more specific expression
         // visitors, so that we have direct access to the expression span.
-        fn visit_expr(&mut self, expr: &'hir hir::Expr<'hir>) {
+        fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) {
             match expr.kind {
                 hir::ExprKind::Closure(_) | hir::ExprKind::ConstBlock(_) => {
-                    (self.visit_hole_span)(expr.span);
+                    self.visit_hole_span(expr.span);
                     // Having visited this expression, we don't care about its
                     // children, so don't call `walk_expr`.
                 }
@@ -384,18 +386,17 @@ fn extract_hole_spans_from_hir<'tcx>(
             }
         }
     }
-
-    let mut hole_spans = vec![];
-    let mut visitor = HolesVisitor {
-        tcx,
-        visit_hole_span: |hole_span| {
+    impl HolesVisitor<'_> {
+        fn visit_hole_span(&mut self, hole_span: Span) {
             // Discard any holes that aren't directly visible within the body span.
-            if body_span.contains(hole_span) && body_span.eq_ctxt(hole_span) {
-                hole_spans.push(hole_span);
+            if self.body_span.contains(hole_span) && self.body_span.eq_ctxt(hole_span) {
+                self.hole_spans.push(hole_span);
             }
-        },
-    };
+        }
+    }
+
+    let mut visitor = HolesVisitor { tcx, body_span, hole_spans: vec![] };
 
     visitor.visit_body(hir_body);
-    hole_spans
+    visitor.hole_spans
 }
diff --git a/tests/coverage/holes.cov-map b/tests/coverage/holes.cov-map
@@ -1,52 +1,57 @@
 Function name: <holes::main::MyStruct>::_method (unused)
-Raw bytes (9): 0x[01, 01, 00, 01, 00, 25, 09, 00, 1d]
+Raw bytes (9): 0x[01, 01, 00, 01, 00, 2b, 09, 00, 1d]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 0
 Number of file 0 mappings: 1
-- Code(Zero) at (prev + 37, 9) to (start + 0, 29)
+- Code(Zero) at (prev + 43, 9) to (start + 0, 29)
 Highest counter ID seen: (none)
 
 Function name: holes::main
-Raw bytes (44): 0x[01, 01, 00, 08, 01, 08, 01, 06, 11, 01, 0f, 05, 00, 12, 01, 04, 05, 00, 12, 01, 07, 05, 00, 12, 01, 06, 05, 00, 12, 01, 06, 05, 03, 0f, 01, 0a, 05, 03, 0f, 01, 0a, 05, 01, 02]
+Raw bytes (69): 0x[01, 01, 00, 0d, 01, 08, 01, 01, 12, 01, 05, 05, 00, 12, 01, 07, 09, 00, 11, 01, 09, 05, 00, 12, 01, 04, 05, 00, 12, 01, 07, 05, 00, 12, 01, 06, 05, 00, 12, 01, 04, 05, 00, 12, 01, 04, 05, 00, 12, 01, 06, 05, 03, 0f, 01, 0a, 05, 03, 0f, 01, 0a, 05, 0c, 0d, 01, 0f, 0e, 05, 02]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 0
-Number of file 0 mappings: 8
-- Code(Counter(0)) at (prev + 8, 1) to (start + 6, 17)
-- Code(Counter(0)) at (prev + 15, 5) to (start + 0, 18)
+Number of file 0 mappings: 13
+- Code(Counter(0)) at (prev + 8, 1) to (start + 1, 18)
+- Code(Counter(0)) at (prev + 5, 5) to (start + 0, 18)
+- Code(Counter(0)) at (prev + 7, 9) to (start + 0, 17)
+- Code(Counter(0)) at (prev + 9, 5) to (start + 0, 18)
 - Code(Counter(0)) at (prev + 4, 5) to (start + 0, 18)
 - Code(Counter(0)) at (prev + 7, 5) to (start + 0, 18)
 - Code(Counter(0)) at (prev + 6, 5) to (start + 0, 18)
+- Code(Counter(0)) at (prev + 4, 5) to (start + 0, 18)
+- Code(Counter(0)) at (prev + 4, 5) to (start + 0, 18)
 - Code(Counter(0)) at (prev + 6, 5) to (start + 3, 15)
 - Code(Counter(0)) at (prev + 10, 5) to (start + 3, 15)
-- Code(Counter(0)) at (prev + 10, 5) to (start + 1, 2)
+- Code(Counter(0)) at (prev + 10, 5) to (start + 12, 13)
+- Code(Counter(0)) at (prev + 15, 14) to (start + 5, 2)
 Highest counter ID seen: c0
 
 Function name: holes::main::_unused_fn (unused)
-Raw bytes (9): 0x[01, 01, 00, 01, 00, 19, 05, 00, 17]
+Raw bytes (9): 0x[01, 01, 00, 01, 00, 1f, 05, 00, 17]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 0
 Number of file 0 mappings: 1
-- Code(Zero) at (prev + 25, 5) to (start + 0, 23)
+- Code(Zero) at (prev + 31, 5) to (start + 0, 23)
 Highest counter ID seen: (none)
 
 Function name: holes::main::{closure#0} (unused)
-Raw bytes (9): 0x[01, 01, 00, 01, 00, 12, 09, 02, 0a]
+Raw bytes (9): 0x[01, 01, 00, 01, 00, 18, 09, 02, 0a]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 0
 Number of file 0 mappings: 1
-- Code(Zero) at (prev + 18, 9) to (start + 2, 10)
+- Code(Zero) at (prev + 24, 9) to (start + 2, 10)
 Highest counter ID seen: (none)
 
 Function name: holes::main::{closure#1} (unused)
-Raw bytes (9): 0x[01, 01, 00, 01, 00, 3d, 09, 02, 0a]
+Raw bytes (9): 0x[01, 01, 00, 01, 00, 4b, 09, 02, 0a]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 0
 Number of file 0 mappings: 1
-- Code(Zero) at (prev + 61, 9) to (start + 2, 10)
+- Code(Zero) at (prev + 75, 9) to (start + 2, 10)
 Highest counter ID seen: (none)
 
diff --git a/tests/coverage/holes.coverage b/tests/coverage/holes.coverage
@@ -7,10 +7,16 @@
    LL|       |
    LL|      1|fn main() {
    LL|      1|    black_box(());
-   LL|      1|
-   LL|      1|    // Splitting this across multiple lines makes it easier to see where the
-   LL|      1|    // coverage mapping regions begin and end.
-   LL|      1|    #[rustfmt::skip]
+   LL|       |
+   LL|       |    static MY_STATIC: () = ();
+   LL|       |
+   LL|      1|    black_box(());
+   LL|       |
+   LL|       |    const MY_CONST: () = ();
+   LL|       |
+   LL|       |    // Splitting this across multiple lines makes it easier to see where the
+   LL|       |    // coverage mapping regions begin and end.
+   LL|       |    #[rustfmt::skip]
    LL|      1|    let _closure =
    LL|       |        |
    LL|       |            _arg: (),
@@ -39,6 +45,14 @@
    LL|       |
    LL|      1|    black_box(());
    LL|       |
+   LL|       |    trait MyTrait {}
+   LL|       |
+   LL|      1|    black_box(());
+   LL|       |
+   LL|       |    impl MyTrait for MyStruct {}
+   LL|       |
+   LL|      1|    black_box(());
+   LL|       |
    LL|       |    macro_rules! _my_macro {
    LL|       |        () => {};
    LL|       |    }
@@ -64,5 +78,24 @@
    LL|       |        ;
    LL|       |
    LL|      1|    black_box(());
+   LL|      1|
+   LL|      1|    // This tests the edge case of a const block nested inside an "anon const",
+   LL|      1|    // such as the length of an array literal. Handling this case requires
+   LL|      1|    // `nested_filter::OnlyBodies` or equivalent.
+   LL|      1|    #[rustfmt::skip]
+   LL|      1|    let _const_block_inside_anon_const =
+   LL|      1|        [
+   LL|      1|            0
+   LL|      1|            ;
+   LL|      1|            7
+   LL|      1|            +
+   LL|      1|            const
+   LL|       |            {
+   LL|       |                3
+   LL|      1|            }
+   LL|      1|        ]
+   LL|      1|        ;
+   LL|      1|
+   LL|      1|    black_box(());
    LL|      1|}
 
diff --git a/tests/coverage/holes.rs b/tests/coverage/holes.rs
@@ -8,6 +8,12 @@ use core::hint::black_box;
 fn main() {
     black_box(());
 
+    static MY_STATIC: () = ();
+
+    black_box(());
+
+    const MY_CONST: () = ();
+
     // Splitting this across multiple lines makes it easier to see where the
     // coverage mapping regions begin and end.
     #[rustfmt::skip]
@@ -39,6 +45,14 @@ fn main() {
 
     black_box(());
 
+    trait MyTrait {}
+
+    black_box(());
+
+    impl MyTrait for MyStruct {}
+
+    black_box(());
+
     macro_rules! _my_macro {
         () => {};
     }
@@ -64,4 +78,23 @@ fn main() {
         ;
 
     black_box(());
+
+    // This tests the edge case of a const block nested inside an "anon const",
+    // such as the length of an array literal. Handling this case requires
+    // `nested_filter::OnlyBodies` or equivalent.
+    #[rustfmt::skip]
+    let _const_block_inside_anon_const =
+        [
+            0
+            ;
+            7
+            +
+            const
+            {
+                3
+            }
+        ]
+        ;
+
+    black_box(());
 }
