coverage: Tighten validation of #[coverage(off)] and #[coverage(on)]

Author: Zalathar

Diff for: 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

Diff for: 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

Diff for: 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 {

Diff for: compiler/rustc_feature/src/builtin_attrs.rs

@@ -105,6 +105,9 @@ pub struct AttributeTemplate {
     pub word: bool,
     /// If `Some`, the attribute is allowed to take a list of items like `#[allow(..)]`.
     pub list: Option<&'static str>,
+    /// If non-empty, the attribute is allowed to take a list containing exactly
+    /// one of the listed words, like `#[coverage(off)]`.
+    pub one_of: &'static [Symbol],
     /// If `Some`, the attribute is allowed to be a name/value pair where the
     /// value is a string, like `#[must_use = "reason"]`.
     pub name_value_str: Option<&'static str>,
@@ -165,19 +168,20 @@ pub enum AttributeDuplicates {
 /// E.g., `template!(Word, List: "description")` means that the attribute
 /// supports forms `#[attr]` and `#[attr(description)]`.
 macro_rules! template {
-    (Word) => { template!(@ true, None, None) };
-    (List: $descr: expr) => { template!(@ false, Some($descr), None) };
-    (NameValueStr: $descr: expr) => { template!(@ false, None, Some($descr)) };
-    (Word, List: $descr: expr) => { template!(@ true, Some($descr), None) };
-    (Word, NameValueStr: $descr: expr) => { template!(@ true, None, Some($descr)) };
+    (Word) => { template!(@ true, None, &[], None) };
+    (List: $descr: expr) => { template!(@ false, Some($descr), &[], None) };
+    (OneOf: $one_of: expr) => { template!(@ false, None, $one_of, None) };
+    (NameValueStr: $descr: expr) => { template!(@ false, None, &[], Some($descr)) };
+    (Word, List: $descr: expr) => { template!(@ true, Some($descr), &[], None) };
+    (Word, NameValueStr: $descr: expr) => { template!(@ true, None, &[], Some($descr)) };
     (List: $descr1: expr, NameValueStr: $descr2: expr) => {
-        template!(@ false, Some($descr1), Some($descr2))
+        template!(@ false, Some($descr1), &[], Some($descr2))
     };
     (Word, List: $descr1: expr, NameValueStr: $descr2: expr) => {
-        template!(@ true, Some($descr1), Some($descr2))
+        template!(@ true, Some($descr1), &[], Some($descr2))
     };
-    (@ $word: expr, $list: expr, $name_value_str: expr) => { AttributeTemplate {
-        word: $word, list: $list, name_value_str: $name_value_str
+    (@ $word: expr, $list: expr, $one_of: expr, $name_value_str: expr) => { AttributeTemplate {
+        word: $word, list: $list, one_of: $one_of, name_value_str: $name_value_str
     } };
 }
 
@@ -478,7 +482,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
         EncodeCrossCrate::No, experimental!(no_sanitize)
     ),
     gated!(
-        coverage, Normal, template!(Word, List: "on|off"),
+        coverage, Normal, template!(OneOf: &[sym::off, sym::on]),
         ErrorPreceding, EncodeCrossCrate::No,
         coverage_attribute, experimental!(coverage)
     ),

Diff for: compiler/rustc_parse/src/validate_attr.rs

@@ -4,8 +4,10 @@ use crate::{errors, parse_in};
 
 use rustc_ast::token::Delimiter;
 use rustc_ast::tokenstream::DelimSpan;
-use rustc_ast::MetaItemKind;
-use rustc_ast::{self as ast, AttrArgs, AttrArgsEq, Attribute, DelimArgs, MetaItem, Safety};
+use rustc_ast::{
+    self as ast, AttrArgs, AttrArgsEq, Attribute, DelimArgs, MetaItem, MetaItemKind,
+    NestedMetaItem, Safety,
+};
 use rustc_errors::{Applicability, FatalError, PResult};
 use rustc_feature::{
     AttributeSafety, AttributeTemplate, BuiltinAttribute, Features, BUILTIN_ATTRIBUTE_MAP,
@@ -184,9 +186,13 @@ pub(super) fn check_cfg_attr_bad_delim(psess: &ParseSess, span: DelimSpan, delim
 
 /// Checks that the given meta-item is compatible with this `AttributeTemplate`.
 fn is_attr_template_compatible(template: &AttributeTemplate, meta: &ast::MetaItemKind) -> bool {
+    let is_one_allowed_subword = |items: &[NestedMetaItem]| match items {
+        [item] => item.is_word() && template.one_of.iter().any(|&word| item.has_name(word)),
+        _ => false,
+    };
     match meta {
         MetaItemKind::Word => template.word,
-        MetaItemKind::List(..) => template.list.is_some(),
+        MetaItemKind::List(items) => template.list.is_some() || is_one_allowed_subword(items),
         MetaItemKind::NameValue(lit) if lit.kind.is_str() => template.name_value_str.is_some(),
         MetaItemKind::NameValue(..) => false,
     }
@@ -230,6 +236,7 @@ fn emit_malformed_attribute(
     if let Some(descr) = template.list {
         suggestions.push(format!("#{inner}[{name}({descr})]"));
     }
+    suggestions.extend(template.one_of.iter().map(|&word| format!("#{inner}[{name}({word})]")));
     if let Some(descr) = template.name_value_str {
         suggestions.push(format!("#{inner}[{name} = \"{descr}\"]"));
     }