Auto merge of #114656 - bossmc:rework-no-coverage-attr, r=oli-obk

Rework `no_coverage` to `coverage(off)`

As discussed at the tail of #84605 this replaces the `no_coverage` attribute with a `coverage` attribute that takes sub-parameters (currently `off` and `on`) to control the coverage instrumentation.

Allows future-proofing for things like `coverage(off, reason="Tested live", issue="#12345")` or similar.

Author: bors

compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs

@@ -34,7 +34,7 @@ pub fn expand_deriving_eq(
             attributes: thin_vec![
                 cx.attr_word(sym::inline, span),
                 cx.attr_nested_word(sym::doc, sym::hidden, span),
-                cx.attr_word(sym::no_coverage, span)
+                cx.attr_nested_word(sym::coverage, sym::off, span)
             ],
             fieldless_variants_strategy: FieldlessVariantsStrategy::Unify,
             combine_substructure: combine_substructure(Box::new(|a, b, c| {

compiler/rustc_builtin_macros/src/test_harness.rs

@@ -254,7 +254,7 @@ fn generate_test_harness(
     let expn_id = ext_cx.resolver.expansion_for_ast_pass(
         DUMMY_SP,
         AstPass::TestHarness,
-        &[sym::test, sym::rustc_attrs, sym::no_coverage],
+        &[sym::test, sym::rustc_attrs, sym::coverage_attribute],
         None,
     );
     let def_site = DUMMY_SP.with_def_site_ctxt(expn_id.to_expn_id());
@@ -335,8 +335,8 @@ fn mk_main(cx: &mut TestCtxt<'_>) -> P<ast::Item> {
 
     // #[rustc_main]
     let main_attr = ecx.attr_word(sym::rustc_main, sp);
-    // #[no_coverage]
-    let no_coverage_attr = ecx.attr_word(sym::no_coverage, sp);
+    // #[coverage(off)]
+    let coverage_attr = ecx.attr_nested_word(sym::coverage, sym::off, sp);
 
     // pub fn main() { ... }
     let main_ret_ty = ecx.ty(sp, ast::TyKind::Tup(ThinVec::new()));
@@ -366,7 +366,7 @@ fn mk_main(cx: &mut TestCtxt<'_>) -> P<ast::Item> {
 
     let main = P(ast::Item {
         ident: main_id,
-        attrs: thin_vec![main_attr, no_coverage_attr],
+        attrs: thin_vec![main_attr, coverage_attr],
         id: ast::DUMMY_NODE_ID,
         kind: main,
         vis: ast::Visibility { span: sp, kind: ast::VisibilityKind::Public, tokens: None },

compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs

@@ -330,10 +330,10 @@ fn add_unused_functions(cx: &CodegenCx<'_, '_>) {
     {
         let codegen_fn_attrs = tcx.codegen_fn_attrs(non_codegenned_def_id);
 
-        // If a function is marked `#[no_coverage]`, then skip generating a
+        // If a function is marked `#[coverage(off)]`, then skip generating a
         // dead code stub for it.
         if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NO_COVERAGE) {
-            debug!("skipping unused fn marked #[no_coverage]: {:?}", non_codegenned_def_id);
+            debug!("skipping unused fn marked #[coverage(off)]: {:?}", non_codegenned_def_id);
             continue;
         }
 

compiler/rustc_codegen_ssa/messages.ftl

@@ -23,6 +23,8 @@ codegen_ssa_erroneous_constant = erroneous constant encountered
 
 codegen_ssa_error_creating_remark_dir = failed to create remark directory: {$error}
 
+codegen_ssa_expected_coverage_symbol = expected `coverage(off)` or `coverage(on)`
+
 codegen_ssa_expected_used_symbol = expected `used`, `used(compiler)` or `used(linker)`
 
 codegen_ssa_extern_funcs_not_found = some `extern` functions couldn't be found; some native libraries may need to be installed or have their path specified

compiler/rustc_codegen_ssa/src/codegen_attrs.rs

@@ -16,7 +16,10 @@ use rustc_target::spec::{abi, SanitizerSet};
 
 use crate::errors;
 use crate::target_features::from_target_feature;
-use crate::{errors::ExpectedUsedSymbol, target_features::check_target_feature_trait_unsafe};
+use crate::{
+    errors::{ExpectedCoverageSymbol, ExpectedUsedSymbol},
+    target_features::check_target_feature_trait_unsafe,
+};
 
 fn linkage_by_name(tcx: TyCtxt<'_>, def_id: LocalDefId, name: &str) -> Linkage {
     use rustc_middle::mir::mono::Linkage::*;
@@ -128,7 +131,21 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
                         .emit();
                 }
             }
-            sym::no_coverage => codegen_fn_attrs.flags |= CodegenFnAttrFlags::NO_COVERAGE,
+            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.sess.emit_err(ExpectedCoverageSymbol { span: attr.span });
+                    }
+                }
+            }
             sym::rustc_std_internal_symbol => {
                 codegen_fn_attrs.flags |= CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL
             }

compiler/rustc_codegen_ssa/src/errors.rs

@@ -561,6 +561,13 @@ pub struct UnknownArchiveKind<'a> {
     pub kind: &'a str,
 }
 
+#[derive(Diagnostic)]
+#[diag(codegen_ssa_expected_coverage_symbol)]
+pub struct ExpectedCoverageSymbol {
+    #[primary_span]
+    pub span: Span,
+}
+
 #[derive(Diagnostic)]
 #[diag(codegen_ssa_expected_used_symbol)]
 pub struct ExpectedUsedSymbol {

compiler/rustc_error_codes/src/error_codes/E0788.md

@@ -1,4 +1,4 @@
-A `#[no_coverage]` attribute was applied to something which does not show up
+A `#[coverage]` attribute was applied to something which does not show up
 in code coverage, or is too granular to be excluded from the coverage report.
 
 For now, this attribute can only be applied to function, method, and closure
@@ -9,18 +9,18 @@ will just emit an `unused_attributes` lint instead of this error.
 Example of erroneous code:
 
 ```compile_fail,E0788
-#[no_coverage]
+#[coverage(off)]
 struct Foo;
 
-#[no_coverage]
+#[coverage(on)]
 const FOO: Foo = Foo;
 ```
 
-`#[no_coverage]` tells the compiler to not generate coverage instrumentation for
-a piece of code when the `-C instrument-coverage` flag is passed. Things like
-structs and consts are not coverable code, and thus cannot do anything with this
-attribute.
+`#[coverage(off)]` tells the compiler to not generate coverage instrumentation
+for a piece of code when the `-C instrument-coverage` flag is passed. Things
+like structs and consts are not coverable code, and thus cannot do anything
+with this attribute.
 
 If you wish to apply this attribute to all methods in an impl or module,
 manually annotate each method; it is not possible to annotate the entire impl
-with a `#[no_coverage]` attribute.
+with a `#[coverage]` attribute.

compiler/rustc_feature/src/active.rs

@@ -398,6 +398,9 @@ declare_features! (
     (active, const_trait_impl, "1.42.0", Some(67792), None),
     /// Allows the `?` operator in const contexts.
     (active, const_try, "1.56.0", Some(74935), None),
+    /// Allows function attribute `#[coverage(on/off)]`, to control coverage
+    /// instrumentation of that function.
+    (active, coverage_attribute, "CURRENT_RUSTC_VERSION", Some(84605), None),
     /// Allows non-builtin attributes in inner attribute position.
     (active, custom_inner_attributes, "1.30.0", Some(54726), None),
     /// Allows custom test frameworks with `#![test_runner]` and `#[test_case]`.
@@ -509,9 +512,6 @@ declare_features! (
     (active, never_type_fallback, "1.41.0", Some(65992), None),
     /// Allows `#![no_core]`.
     (active, no_core, "1.3.0", Some(29639), None),
-    /// Allows function attribute `#[no_coverage]`, to bypass coverage
-    /// instrumentation of that function.
-    (active, no_coverage, "1.53.0", Some(84605), None),
     /// Allows the use of `no_sanitize` attribute.
     (active, no_sanitize, "1.42.0", Some(39699), None),
     /// Allows using the `non_exhaustive_omitted_patterns` lint.

compiler/rustc_feature/src/builtin_attrs.rs

@@ -395,7 +395,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
         template!(List: "address, kcfi, memory, thread"), DuplicatesOk,
         experimental!(no_sanitize)
     ),
-    gated!(no_coverage, Normal, template!(Word), WarnFollowing, experimental!(no_coverage)),
+    gated!(coverage, Normal, template!(Word, List: "on|off"), WarnFollowing, coverage_attribute, experimental!(coverage)),
 
     ungated!(
         doc, Normal, template!(List: "hidden|inline|...", NameValueStr: "string"), DuplicatesOk

compiler/rustc_feature/src/removed.rs

@@ -136,6 +136,9 @@ declare_features! (
      Some("subsumed by `#![feature(allocator_internals)]`")),
     /// Allows use of unary negate on unsigned integers, e.g., -e for e: u8
     (removed, negate_unsigned, "1.0.0", Some(29645), None, None),
+    /// Allows `#[no_coverage]` on functions.
+    /// The feature was renamed to `coverage` and the attribute to `#[coverage(on|off)]`
+    (removed, no_coverage, "CURRENT_RUSTC_VERSION", Some(84605), None, Some("renamed to `coverage`")),
     /// Allows `#[no_debug]`.
     (removed, no_debug, "1.43.0", Some(29721), None, Some("removed due to lack of demand")),
     /// Allows using `#[on_unimplemented(..)]` on traits.