Rollup merge of #126721 - Zalathar:nested-cov-attr, r=oli-obk

coverage: Make `#[coverage(..)]` apply recursively to nested functions

This PR makes the (currently-unstable) `#[coverage(off)]` and `#[coverage(on)]` attributes apply recursively to all nested functions/closures, instead of just the function they are directly attached to.

Those attributes can now also be applied to modules and to impl/impl-trait blocks, where they have no direct effect, but will be inherited by all enclosed functions/closures/methods that don't override the inherited value.

---

Fixes #126625.

Author: jhpratt

compiler/rustc_codegen_ssa/src/codegen_attrs.rs

@@ -124,22 +124,6 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
                         .emit();
                 }
             }
-            sym::coverage => {
-                let inner = attr.meta_item_list();
-                match inner.as_deref() {
-                    Some([item]) if item.has_name(sym::off) => {
-                        codegen_fn_attrs.flags |= CodegenFnAttrFlags::NO_COVERAGE;
-                    }
-                    Some([item]) if item.has_name(sym::on) => {
-                        // Allow #[coverage(on)] for being explicit, maybe also in future to enable
-                        // coverage on a smaller scope within an excluded larger scope.
-                    }
-                    Some(_) | None => {
-                        tcx.dcx()
-                            .span_delayed_bug(attr.span, "unexpected value of coverage attribute");
-                    }
-                }
-            }
             sym::rustc_std_internal_symbol => {
                 codegen_fn_attrs.flags |= CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL
             }
@@ -584,7 +568,6 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
     }
 
     if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NAKED) {
-        codegen_fn_attrs.flags |= CodegenFnAttrFlags::NO_COVERAGE;
         codegen_fn_attrs.inline = InlineAttr::Never;
     }
 

compiler/rustc_middle/src/middle/codegen_fn_attrs.rs

@@ -87,10 +87,7 @@ bitflags::bitflags! {
         /// #[cmse_nonsecure_entry]: with a TrustZone-M extension, declare a
         /// function as an entry function from Non-Secure code.
         const CMSE_NONSECURE_ENTRY      = 1 << 13;
-        /// `#[coverage(off)]`: indicates that the function should be ignored by
-        /// the MIR `InstrumentCoverage` pass and not added to the coverage map
-        /// during codegen.
-        const NO_COVERAGE               = 1 << 14;
+        // (Bit 14 was used for `#[coverage(off)]`, but is now unused.)
         /// `#[used(linker)]`:
         /// indicates that neither LLVM nor the linker will eliminate this function.
         const USED_LINKER               = 1 << 15;

compiler/rustc_middle/src/query/mod.rs

@@ -572,6 +572,15 @@ rustc_queries! {
         separate_provide_extern
     }
 
+    /// Checks for the nearest `#[coverage(off)]` or `#[coverage(on)]` on
+    /// this def and any enclosing defs, up to the crate root.
+    ///
+    /// Returns `false` if `#[coverage(off)]` was found, or `true` if
+    /// either `#[coverage(on)]` or no coverage attribute was found.
+    query coverage_attr_on(key: LocalDefId) -> bool {
+        desc { |tcx| "checking for `#[coverage(..)]` on `{}`", tcx.def_path_str(key) }
+    }
+
     /// Summarizes coverage IDs inserted by the `InstrumentCoverage` MIR pass
     /// (for compiler option `-Cinstrument-coverage`), after MIR optimizations
     /// have had a chance to potentially remove some of them.

compiler/rustc_mir_transform/src/coverage/query.rs

@@ -6,11 +6,13 @@ use rustc_middle::query::TyCtxtAt;
 use rustc_middle::ty::{self, TyCtxt};
 use rustc_middle::util::Providers;
 use rustc_span::def_id::LocalDefId;
+use rustc_span::sym;
 
 /// Registers query/hook implementations related to coverage.
 pub(crate) fn provide(providers: &mut Providers) {
     providers.hooks.is_eligible_for_coverage =
         |TyCtxtAt { tcx, .. }, def_id| is_eligible_for_coverage(tcx, def_id);
+    providers.queries.coverage_attr_on = coverage_attr_on;
     providers.queries.coverage_ids_info = coverage_ids_info;
 }
 
@@ -38,14 +40,43 @@ fn is_eligible_for_coverage(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
         return false;
     }
 
-    if tcx.codegen_fn_attrs(def_id).flags.contains(CodegenFnAttrFlags::NO_COVERAGE) {
+    if tcx.codegen_fn_attrs(def_id).flags.contains(CodegenFnAttrFlags::NAKED) {
+        trace!("InstrumentCoverage skipped for {def_id:?} (`#[naked]`)");
+        return false;
+    }
+
+    if !tcx.coverage_attr_on(def_id) {
         trace!("InstrumentCoverage skipped for {def_id:?} (`#[coverage(off)]`)");
         return false;
     }
 
     true
 }
 
+/// Query implementation for `coverage_attr_on`.
+fn coverage_attr_on(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
+    // Check for annotations directly on this def.
+    if let Some(attr) = tcx.get_attr(def_id, sym::coverage) {
+        match attr.meta_item_list().as_deref() {
+            Some([item]) if item.has_name(sym::off) => return false,
+            Some([item]) if item.has_name(sym::on) => return true,
+            Some(_) | None => {
+                // Other possibilities should have been rejected by `rustc_parse::validate_attr`.
+                tcx.dcx().span_bug(attr.span, "unexpected value of coverage attribute");
+            }
+        }
+    }
+
+    match tcx.opt_local_parent(def_id) {
+        // Check the parent def (and so on recursively) until we find an
+        // enclosing attribute or reach the crate root.
+        Some(parent) => tcx.coverage_attr_on(parent),
+        // We reached the crate root without seeing a coverage attribute, so
+        // allow coverage instrumentation by default.
+        None => true,
+    }
+}
+
 /// Query implementation for `coverage_ids_info`.
 fn coverage_ids_info<'tcx>(
     tcx: TyCtxt<'tcx>,

compiler/rustc_passes/src/check_attr.rs

@@ -369,13 +369,16 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
         }
     }
 
-    /// Checks that `#[coverage(..)]` is applied to a function or closure.
+    /// Checks that `#[coverage(..)]` is applied to a function/closure/method,
+    /// or to an impl block or module.
     fn check_coverage(&self, attr: &Attribute, span: Span, target: Target) -> bool {
         match target {
-            // #[coverage(..)] on function is fine
             Target::Fn
             | Target::Closure
-            | Target::Method(MethodKind::Trait { body: true } | MethodKind::Inherent) => true,
+            | Target::Method(MethodKind::Trait { body: true } | MethodKind::Inherent)
+            | Target::Impl
+            | Target::Mod => true,
+
             _ => {
                 self.dcx().emit_err(errors::CoverageNotFnOrClosure {
                     attr_span: attr.span,

tests/coverage/attr/impl.cov-map

@@ -0,0 +1,24 @@
+Function name: <impl::MyStruct>::off_on (unused)
+Raw bytes (9): 0x[01, 01, 00, 01, 00, 0e, 05, 00, 13]
+Number of files: 1
+- file 0 => global file 1
+Number of expressions: 0
+Number of file 0 mappings: 1
+- Code(Zero) at (prev + 14, 5) to (start + 0, 19)
+
+Function name: <impl::MyStruct>::on_inherit (unused)
+Raw bytes (9): 0x[01, 01, 00, 01, 00, 16, 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 + 22, 5) to (start + 0, 23)
+
+Function name: <impl::MyStruct>::on_on (unused)
+Raw bytes (9): 0x[01, 01, 00, 01, 00, 19, 05, 00, 12]
+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, 18)
+

tests/coverage/attr/impl.coverage

@@ -0,0 +1,42 @@
+   LL|       |#![feature(coverage_attribute)]
+   LL|       |//@ edition: 2021
+   LL|       |
+   LL|       |// Checks that `#[coverage(..)]` can be applied to impl and impl-trait blocks,
+   LL|       |// and is inherited by any enclosed functions.
+   LL|       |
+   LL|       |struct MyStruct;
+   LL|       |
+   LL|       |#[coverage(off)]
+   LL|       |impl MyStruct {
+   LL|       |    fn off_inherit() {}
+   LL|       |
+   LL|       |    #[coverage(on)]
+   LL|      0|    fn off_on() {}
+   LL|       |
+   LL|       |    #[coverage(off)]
+   LL|       |    fn off_off() {}
+   LL|       |}
+   LL|       |
+   LL|       |#[coverage(on)]
+   LL|       |impl MyStruct {
+   LL|      0|    fn on_inherit() {}
+   LL|       |
+   LL|       |    #[coverage(on)]
+   LL|      0|    fn on_on() {}
+   LL|       |
+   LL|       |    #[coverage(off)]
+   LL|       |    fn on_off() {}
+   LL|       |}
+   LL|       |
+   LL|       |trait MyTrait {
+   LL|       |    fn method();
+   LL|       |}
+   LL|       |
+   LL|       |#[coverage(off)]
+   LL|       |impl MyTrait for MyStruct {
+   LL|       |    fn method() {}
+   LL|       |}
+   LL|       |
+   LL|       |#[coverage(off)]
+   LL|       |fn main() {}
+

tests/coverage/attr/impl.rs

@@ -0,0 +1,41 @@
+#![feature(coverage_attribute)]
+//@ edition: 2021
+
+// Checks that `#[coverage(..)]` can be applied to impl and impl-trait blocks,
+// and is inherited by any enclosed functions.
+
+struct MyStruct;
+
+#[coverage(off)]
+impl MyStruct {
+    fn off_inherit() {}
+
+    #[coverage(on)]
+    fn off_on() {}
+
+    #[coverage(off)]
+    fn off_off() {}
+}
+
+#[coverage(on)]
+impl MyStruct {
+    fn on_inherit() {}
+
+    #[coverage(on)]
+    fn on_on() {}
+
+    #[coverage(off)]
+    fn on_off() {}
+}
+
+trait MyTrait {
+    fn method();
+}
+
+#[coverage(off)]
+impl MyTrait for MyStruct {
+    fn method() {}
+}
+
+#[coverage(off)]
+fn main() {}

tests/coverage/attr/module.cov-map

@@ -0,0 +1,24 @@
+Function name: module::off::on (unused)
+Raw bytes (9): 0x[01, 01, 00, 01, 00, 0c, 05, 00, 0f]
+Number of files: 1
+- file 0 => global file 1
+Number of expressions: 0
+Number of file 0 mappings: 1
+- Code(Zero) at (prev + 12, 5) to (start + 0, 15)
+
+Function name: module::on::inherit (unused)
+Raw bytes (9): 0x[01, 01, 00, 01, 00, 14, 05, 00, 14]
+Number of files: 1
+- file 0 => global file 1
+Number of expressions: 0
+Number of file 0 mappings: 1
+- Code(Zero) at (prev + 20, 5) to (start + 0, 20)
+
+Function name: module::on::on (unused)
+Raw bytes (9): 0x[01, 01, 00, 01, 00, 17, 05, 00, 0f]
+Number of files: 1
+- file 0 => global file 1
+Number of expressions: 0
+Number of file 0 mappings: 1
+- Code(Zero) at (prev + 23, 5) to (start + 0, 15)
+

tests/coverage/attr/module.coverage

@@ -0,0 +1,38 @@
+   LL|       |#![feature(coverage_attribute)]
+   LL|       |//@ edition: 2021
+   LL|       |
+   LL|       |// Checks that `#[coverage(..)]` can be applied to modules, and is inherited
+   LL|       |// by any enclosed functions.
+   LL|       |
+   LL|       |#[coverage(off)]
+   LL|       |mod off {
+   LL|       |    fn inherit() {}
+   LL|       |
+   LL|       |    #[coverage(on)]
+   LL|      0|    fn on() {}
+   LL|       |
+   LL|       |    #[coverage(off)]
+   LL|       |    fn off() {}
+   LL|       |}
+   LL|       |
+   LL|       |#[coverage(on)]
+   LL|       |mod on {
+   LL|      0|    fn inherit() {}
+   LL|       |
+   LL|       |    #[coverage(on)]
+   LL|      0|    fn on() {}
+   LL|       |
+   LL|       |    #[coverage(off)]
+   LL|       |    fn off() {}
+   LL|       |}
+   LL|       |
+   LL|       |#[coverage(off)]
+   LL|       |mod nested_a {
+   LL|       |    mod nested_b {
+   LL|       |        fn inner() {}
+   LL|       |    }
+   LL|       |}
+   LL|       |
+   LL|       |#[coverage(off)]
+   LL|       |fn main() {}
+

tests/coverage/attr/module.rs

@@ -0,0 +1,37 @@
+#![feature(coverage_attribute)]
+//@ edition: 2021
+
+// Checks that `#[coverage(..)]` can be applied to modules, and is inherited
+// by any enclosed functions.
+
+#[coverage(off)]
+mod off {
+    fn inherit() {}
+
+    #[coverage(on)]
+    fn on() {}
+
+    #[coverage(off)]
+    fn off() {}
+}
+
+#[coverage(on)]
+mod on {
+    fn inherit() {}
+
+    #[coverage(on)]
+    fn on() {}
+
+    #[coverage(off)]
+    fn off() {}
+}
+
+#[coverage(off)]
+mod nested_a {
+    mod nested_b {
+        fn inner() {}
+    }
+}
+
+#[coverage(off)]
+fn main() {}