coverage: Overhaul validation of #[coverage(..)]

Author: Zalathar

compiler/rustc_codegen_ssa/messages.ftl

@@ -27,8 +27,6 @@ codegen_ssa_create_temp_dir = couldn't create a temp dir: {$error}
 
 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

@@ -15,11 +15,7 @@ use rustc_span::{sym, Span};
 use rustc_target::spec::{abi, SanitizerSet};
 
 use crate::errors;
-use crate::target_features::from_target_feature;
-use crate::{
-    errors::{ExpectedCoverageSymbol, ExpectedUsedSymbol},
-    target_features::check_target_feature_trait_unsafe,
-};
+use crate::target_features::{check_target_feature_trait_unsafe, from_target_feature};
 
 fn linkage_by_name(tcx: TyCtxt<'_>, def_id: LocalDefId, name: &str) -> Linkage {
     use rustc_middle::mir::mono::Linkage::*;
@@ -139,7 +135,8 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
                         // coverage on a smaller scope within an excluded larger scope.
                     }
                     Some(_) | None => {
-                        tcx.dcx().emit_err(ExpectedCoverageSymbol { span: attr.span });
+                        tcx.dcx()
+                            .span_delayed_bug(attr.span, "unexpected value of coverage attribute");
                     }
                 }
             }
@@ -174,7 +171,7 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
                         codegen_fn_attrs.flags |= CodegenFnAttrFlags::USED;
                     }
                     Some(_) => {
-                        tcx.dcx().emit_err(ExpectedUsedSymbol { span: attr.span });
+                        tcx.dcx().emit_err(errors::ExpectedUsedSymbol { span: attr.span });
                     }
                     None => {
                         // Unfortunately, unconditionally using `llvm.used` causes

compiler/rustc_codegen_ssa/src/errors.rs

@@ -564,13 +564,6 @@ 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_feature/src/builtin_attrs.rs

@@ -478,7 +478,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
         EncodeCrossCrate::No, experimental!(no_sanitize)
     ),
     gated!(
-        coverage, Normal, template!(Word, List: "on|off"),
+        coverage, Normal, template!(List: "off|on"),
         ErrorPreceding, EncodeCrossCrate::No,
         coverage_attribute, experimental!(coverage)
     ),

compiler/rustc_parse/src/validate_attr.rs

@@ -149,7 +149,9 @@ pub fn check_builtin_meta_item(
 ) {
     // Some special attributes like `cfg` must be checked
     // before the generic check, so we skip them here.
-    let should_skip = |name| name == sym::cfg;
+    // Coverage attributes are checked by the `check_attr` pass, which gives
+    // better error messages.
+    let should_skip = |name| name == sym::cfg || name == sym::coverage;
 
     if !should_skip(name) && !is_attr_template_compatible(&template, &meta.kind) {
         emit_malformed_attribute(psess, style, meta.span, name, template);

compiler/rustc_passes/messages.ftl

@@ -103,18 +103,12 @@ passes_continue_labeled_block =
     .label = labeled blocks cannot be `continue`'d
     .block_label = labeled block the `continue` points to
 
-passes_coverage_fn_defn =
-    `#[coverage]` may only be applied to function definitions
+passes_coverage_expected_off_or_on =
+    expected `coverage(off)` or `coverage(on)`
 
-passes_coverage_ignored_function_prototype =
-    `#[coverage]` is ignored on function prototypes
-
-passes_coverage_not_coverable =
-    `#[coverage]` must be applied to coverable code
-    .label = not coverable code
-
-passes_coverage_propagate =
-    `#[coverage]` does not propagate into items and must be applied to the contained functions directly
+passes_coverage_not_fn_or_closure =
+    attribute should be applied to a function definition or closure
+    .label = not a function or closure
 
 passes_dead_codes =
     { $multiple ->

compiler/rustc_passes/src/check_attr.rs

@@ -126,7 +126,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
                     self.check_diagnostic_on_unimplemented(attr.span, hir_id, target)
                 }
                 [sym::inline] => self.check_inline(hir_id, attr, span, target),
-                [sym::coverage] => self.check_coverage(hir_id, attr, span, target),
+                [sym::coverage] => self.check_coverage(attr, span, target),
                 [sym::non_exhaustive] => self.check_non_exhaustive(hir_id, attr, span, target),
                 [sym::marker] => self.check_marker(hir_id, attr, span, target),
                 [sym::target_feature] => {
@@ -388,53 +388,34 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
         }
     }
 
-    /// Checks if a `#[coverage]` is applied directly to a function
-    fn check_coverage(&self, hir_id: HirId, 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,
-
-            // function prototypes can't be covered
-            Target::Method(MethodKind::Trait { body: false }) | Target::ForeignFn => {
-                self.tcx.emit_node_span_lint(
-                    UNUSED_ATTRIBUTES,
-                    hir_id,
-                    attr.span,
-                    errors::IgnoredCoverageFnProto,
-                );
-                true
-            }
+    /// Checks that `#[coverage(..)]` is applied to a function or closure, and
+    /// that it has the form `#[coverage(off)]` or `#[coverage(on)]`.
+    fn check_coverage(&self, attr: &Attribute, span: Span, target: Target) -> bool {
+        let mut ok = true;
 
-            Target::Mod | Target::ForeignMod | Target::Impl | Target::Trait => {
-                self.tcx.emit_node_span_lint(
-                    UNUSED_ATTRIBUTES,
-                    hir_id,
-                    attr.span,
-                    errors::IgnoredCoveragePropagate,
-                );
-                true
-            }
-
-            Target::Expression | Target::Statement | Target::Arm => {
-                self.tcx.emit_node_span_lint(
-                    UNUSED_ATTRIBUTES,
-                    hir_id,
-                    attr.span,
-                    errors::IgnoredCoverageFnDefn,
-                );
-                true
+        ok &= match attr.meta_item_list().as_deref() {
+            Some([arg]) if arg.has_name(sym::on) || arg.has_name(sym::off) => true,
+            Some(_) | None => {
+                self.dcx().emit_err(errors::CoverageExpectedOffOrOn { attr_span: attr.span });
+                false
             }
+        };
 
+        ok &= match target {
+            // #[coverage(..)] on function is fine
+            Target::Fn
+            | Target::Closure
+            | Target::Method(MethodKind::Trait { body: true } | MethodKind::Inherent) => true,
             _ => {
-                self.dcx().emit_err(errors::IgnoredCoverageNotCoverable {
+                self.dcx().emit_err(errors::CoverageNotFnOrClosure {
                     attr_span: attr.span,
                     defn_span: span,
                 });
                 false
             }
-        }
+        };
+
+        ok
     }
 
     fn check_generic_attr(

compiler/rustc_passes/src/errors.rs

@@ -60,21 +60,16 @@ pub struct InlineNotFnOrClosure {
     pub defn_span: Span,
 }
 
-#[derive(LintDiagnostic)]
-#[diag(passes_coverage_ignored_function_prototype)]
-pub struct IgnoredCoverageFnProto;
-
-#[derive(LintDiagnostic)]
-#[diag(passes_coverage_propagate)]
-pub struct IgnoredCoveragePropagate;
-
-#[derive(LintDiagnostic)]
-#[diag(passes_coverage_fn_defn)]
-pub struct IgnoredCoverageFnDefn;
+#[derive(Diagnostic)]
+#[diag(passes_coverage_expected_off_or_on)]
+pub struct CoverageExpectedOffOrOn {
+    #[primary_span]
+    pub attr_span: Span,
+}
 
 #[derive(Diagnostic)]
-#[diag(passes_coverage_not_coverable, code = E0788)]
-pub struct IgnoredCoverageNotCoverable {
+#[diag(passes_coverage_not_fn_or_closure, code = E0788)]
+pub struct CoverageNotFnOrClosure {
     #[primary_span]
     pub attr_span: Span,
     #[label]