Check for duplicate attributes.

compiler/rustc_feature/src/lib.rs

@@ -11,6 +11,7 @@
 //! even if it is stabilized or removed, *do not remove it*. Instead, move the
 //! symbol to the `accepted` or `removed` modules respectively.
 
+#![feature(derive_default_enum)]
 #![feature(once_cell)]
 
 mod accepted;
@@ -146,6 +147,7 @@ pub fn find_feature_issue(feature: Symbol, issue: GateIssue) -> Option<NonZeroU3
 
 pub use accepted::ACCEPTED_FEATURES;
 pub use active::{Features, ACTIVE_FEATURES, INCOMPATIBLE_FEATURES};
+pub use builtin_attrs::AttributeDuplicates;
 pub use builtin_attrs::{
     deprecated_attributes, find_gated_cfg, is_builtin_attr_name, AttributeGate, AttributeTemplate,
     AttributeType, BuiltinAttribute, GatedCfg, BUILTIN_ATTRIBUTES, BUILTIN_ATTRIBUTE_MAP,

compiler/rustc_passes/src/check_attr.rs

@@ -9,9 +9,9 @@ use rustc_middle::ty::query::Providers;
 use rustc_middle::ty::TyCtxt;
 
 use rustc_ast::{ast, AttrStyle, Attribute, Lit, LitKind, NestedMetaItem};
-use rustc_data_structures::fx::{FxHashMap, FxHashSet};
+use rustc_data_structures::fx::FxHashMap;
 use rustc_errors::{pluralize, struct_span_err, Applicability};
-use rustc_feature::{AttributeType, BuiltinAttribute, BUILTIN_ATTRIBUTE_MAP};
+use rustc_feature::{AttributeDuplicates, AttributeType, BuiltinAttribute, BUILTIN_ATTRIBUTE_MAP};
 use rustc_hir as hir;
 use rustc_hir::def_id::LocalDefId;
 use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
@@ -23,6 +23,7 @@ use rustc_session::lint::builtin::{
 use rustc_session::parse::feature_err;
 use rustc_span::symbol::{sym, Symbol};
 use rustc_span::{MultiSpan, Span, DUMMY_SP};
+use std::collections::hash_map::Entry;
 
 pub(crate) fn target_from_impl_item<'tcx>(
     tcx: TyCtxt<'tcx>,
@@ -69,7 +70,7 @@ impl CheckAttrVisitor<'tcx> {
         let mut doc_aliases = FxHashMap::default();
         let mut is_valid = true;
         let mut specified_inline = None;
-        let mut seen = FxHashSet::default();
+        let mut seen = FxHashMap::default();
         let attrs = self.tcx.hir().attrs(hir_id);
         for attr in attrs {
             let attr_is_valid = match attr.name_or_empty() {
@@ -148,6 +149,8 @@ impl CheckAttrVisitor<'tcx> {
                 _ => {}
             }
 
+            let builtin = attr.ident().and_then(|ident| BUILTIN_ATTRIBUTE_MAP.get(&ident.name));
+
             if hir_id != CRATE_HIR_ID {
                 if let Some(BuiltinAttribute { type_: AttributeType::CrateLevel, .. }) =
                     attr.ident().and_then(|ident| BUILTIN_ATTRIBUTE_MAP.get(&ident.name))
@@ -165,21 +168,37 @@ impl CheckAttrVisitor<'tcx> {
                 }
             }
 
-            // Duplicate attributes
-            match attr.name_or_empty() {
-                name @ sym::macro_use => {
-                    let args = attr.meta_item_list().unwrap_or_else(Vec::new);
-                    let args: Vec<_> = args.iter().map(|arg| arg.name_or_empty()).collect();
-                    if !seen.insert((name, args)) {
-                        self.tcx.struct_span_lint_hir(
-                            UNUSED_ATTRIBUTES,
-                            hir_id,
+            if let Some(BuiltinAttribute { duplicates, .. }) = builtin {
+                check_duplicates(self.tcx, attr, hir_id, *duplicates, &mut seen);
+            }
+
+            // Warn on useless empty attributes.
+            if matches!(
+                attr.name_or_empty(),
+                sym::macro_use
+                    | sym::allow
+                    | sym::warn
+                    | sym::deny
+                    | sym::forbid
+                    | sym::feature
+                    | sym::repr
+                    | sym::target_feature
+            ) && attr.meta_item_list().map_or(false, |list| list.is_empty())
+            {
+                self.tcx.struct_span_lint_hir(UNUSED_ATTRIBUTES, hir_id, attr.span, |lint| {
+                    lint.build("unused attribute")
+                        .span_suggestion(
                             attr.span,
-                            |lint| lint.build("unused attribute").emit(),
-                        );
-                    }
-                }
-                _ => {}
+                            "remove this attribute",
+                            String::new(),
+                            Applicability::MachineApplicable,
+                        )
+                        .note(&format!(
+                            "attribute `{}` with an empty list has no effect",
+                            attr.name_or_empty()
+                        ))
+                        .emit();
+                });
             }
         }
 
@@ -1990,3 +2009,77 @@ fn check_mod_attrs(tcx: TyCtxt<'_>, module_def_id: LocalDefId) {
 pub(crate) fn provide(providers: &mut Providers) {
     *providers = Providers { check_mod_attrs, ..*providers };
 }
+
+fn check_duplicates(
+    tcx: TyCtxt<'_>,
+    attr: &Attribute,
+    hir_id: HirId,
+    duplicates: AttributeDuplicates,
+    seen: &mut FxHashMap<Symbol, Span>,
+) {
+    use AttributeDuplicates::*;
+    if matches!(duplicates, WarnFollowingWordOnly) && !attr.is_word() {
+        return;
+    }
+    match duplicates {
+        DuplicatesOk => {}
+        WarnFollowing | FutureWarnFollowing | WarnFollowingWordOnly | FutureWarnPreceding => {
+            match seen.entry(attr.name_or_empty()) {
+                Entry::Occupied(mut entry) => {
+                    let (this, other) = if matches!(duplicates, FutureWarnPreceding) {
+                        let to_remove = entry.insert(attr.span);
+                        (to_remove, attr.span)
+                    } else {
+                        (attr.span, *entry.get())
+                    };
+                    tcx.struct_span_lint_hir(UNUSED_ATTRIBUTES, hir_id, this, |lint| {
+                        let mut db = lint.build("unused attribute");
+                        db.span_note(other, "attribute also specified here").span_suggestion(
+                            this,
+                            "remove this attribute",
+                            String::new(),
+                            Applicability::MachineApplicable,
+                        );
+                        if matches!(duplicates, FutureWarnFollowing | FutureWarnPreceding) {
+                            db.warn(
+                                "this was previously accepted by the compiler but is \
+                                 being phased out; it will become a hard error in \
+                                 a future release!",
+                            );
+                        }
+                        db.emit();
+                    });
+                }
+                Entry::Vacant(entry) => {
+                    entry.insert(attr.span);
+                }
+            }
+        }
+        ErrorFollowing | ErrorPreceding => match seen.entry(attr.name_or_empty()) {
+            Entry::Occupied(mut entry) => {
+                let (this, other) = if matches!(duplicates, ErrorPreceding) {
+                    let to_remove = entry.insert(attr.span);
+                    (to_remove, attr.span)
+                } else {
+                    (attr.span, *entry.get())
+                };
+                tcx.sess
+                    .struct_span_err(
+                        this,
+                        &format!("multiple `{}` attributes", attr.name_or_empty()),
+                    )
+                    .span_note(other, "attribute also specified here")
+                    .span_suggestion(
+                        this,
+                        "remove this attribute",
+                        String::new(),
+                        Applicability::MachineApplicable,
+                    )
+                    .emit();
+            }
+            Entry::Vacant(entry) => {
+                entry.insert(attr.span);
+            }
+        },
+    }
+}

compiler/rustc_typeck/src/collect.rs

@@ -2865,14 +2865,6 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, id: DefId) -> CodegenFnAttrs {
         } else if attr.has_name(sym::link_name) {
             codegen_fn_attrs.link_name = attr.value_str();
         } else if attr.has_name(sym::link_ordinal) {
-            if link_ordinal_span.is_some() {
-                tcx.sess
-                    .struct_span_err(
-                        attr.span,
-                        "multiple `link_ordinal` attributes on a single definition",
-                    )
-                    .emit();
-            }
             link_ordinal_span = Some(attr.span);
             if let ordinal @ Some(_) = check_link_ordinal(tcx, attr) {
                 codegen_fn_attrs.link_ordinal = ordinal;

src/test/ui/empty/empty-attributes.rs

@@ -0,0 +1,14 @@
+#![deny(unused_attributes)]
+#![allow()] //~ ERROR unused attribute
+#![warn()] //~ ERROR unused attribute
+#![deny()] //~ ERROR unused attribute
+#![forbid()] //~ ERROR unused attribute
+#![feature()] //~ ERROR unused attribute
+
+#[repr()] //~ ERROR unused attribute
+pub struct S;
+
+#[target_feature()] //~ ERROR unused attribute
+pub unsafe fn foo() {}
+
+fn main() {}

src/test/ui/empty/empty-attributes.stderr

@@ -0,0 +1,63 @@
+error: unused attribute
+  --> $DIR/empty-attributes.rs:8:1
+   |
+LL | #[repr()]
+   | ^^^^^^^^^ help: remove this attribute
+   |
+note: the lint level is defined here
+  --> $DIR/empty-attributes.rs:1:9
+   |
+LL | #![deny(unused_attributes)]
+   |         ^^^^^^^^^^^^^^^^^
+   = note: attribute `repr` with an empty list has no effect
+
+error: unused attribute
+  --> $DIR/empty-attributes.rs:11:1
+   |
+LL | #[target_feature()]
+   | ^^^^^^^^^^^^^^^^^^^ help: remove this attribute
+   |
+   = note: attribute `target_feature` with an empty list has no effect
+
+error: unused attribute
+  --> $DIR/empty-attributes.rs:2:1
+   |
+LL | #![allow()]
+   | ^^^^^^^^^^^ help: remove this attribute
+   |
+   = note: attribute `allow` with an empty list has no effect
+
+error: unused attribute
+  --> $DIR/empty-attributes.rs:3:1
+   |
+LL | #![warn()]
+   | ^^^^^^^^^^ help: remove this attribute
+   |
+   = note: attribute `warn` with an empty list has no effect
+
+error: unused attribute
+  --> $DIR/empty-attributes.rs:4:1
+   |
+LL | #![deny()]
+   | ^^^^^^^^^^ help: remove this attribute
+   |
+   = note: attribute `deny` with an empty list has no effect
+
+error: unused attribute
+  --> $DIR/empty-attributes.rs:5:1
+   |
+LL | #![forbid()]
+   | ^^^^^^^^^^^^ help: remove this attribute
+   |
+   = note: attribute `forbid` with an empty list has no effect
+
+error: unused attribute
+  --> $DIR/empty-attributes.rs:6:1
+   |
+LL | #![feature()]
+   | ^^^^^^^^^^^^^ help: remove this attribute
+   |
+   = note: attribute `feature` with an empty list has no effect
+
+error: aborting due to 7 previous errors
+

src/test/ui/feature-gates/issue-43106-gating-of-builtin-attrs-error.rs

@@ -134,4 +134,27 @@ mod start {
     //~^ ERROR: `start` attribute can only be used on functions
 }
 
+#[repr(C)]
+//~^ ERROR: attribute should be applied to a struct, enum, or union
+mod repr {
+//~^ NOTE not a struct, enum, or union
+    mod inner { #![repr(C)] }
+    //~^ ERROR: attribute should be applied to a struct, enum, or union
+    //~| NOTE not a struct, enum, or union
+
+    #[repr(C)] fn f() { }
+    //~^ ERROR: attribute should be applied to a struct, enum, or union
+    //~| NOTE not a struct, enum, or union
+
+    struct S;
+
+    #[repr(C)] type T = S;
+    //~^ ERROR: attribute should be applied to a struct, enum, or union
+    //~| NOTE not a struct, enum, or union
+
+    #[repr(C)] impl S { }
+    //~^ ERROR: attribute should be applied to a struct, enum, or union
+    //~| NOTE not a struct, enum, or union
+}
+
 fn main() {}

src/test/ui/feature-gates/issue-43106-gating-of-builtin-attrs-error.stderr

@@ -91,6 +91,21 @@ LL | |     }
 LL | | }
    | |_- not a free function, impl method or static
 
+error[E0517]: attribute should be applied to a struct, enum, or union
+  --> $DIR/issue-43106-gating-of-builtin-attrs-error.rs:137:8
+   |
+LL |   #[repr(C)]
+   |          ^
+LL |
+LL | / mod repr {
+LL | |
+LL | |     mod inner { #![repr(C)] }
+LL | |
+...  |
+LL | |
+LL | | }
+   | |_- not a struct, enum, or union
+
 error: attribute should be applied to an `extern crate` item
   --> $DIR/issue-43106-gating-of-builtin-attrs-error.rs:25:1
    |
@@ -235,7 +250,31 @@ error: attribute should be applied to a free function, impl method or static
 LL |         #[export_name = "2200"] fn bar() {}
    |         ^^^^^^^^^^^^^^^^^^^^^^^ ----------- not a free function, impl method or static
 
-error: aborting due to 34 previous errors
+error[E0517]: attribute should be applied to a struct, enum, or union
+  --> $DIR/issue-43106-gating-of-builtin-attrs-error.rs:141:25
+   |
+LL |     mod inner { #![repr(C)] }
+   |     --------------------^---- not a struct, enum, or union
+
+error[E0517]: attribute should be applied to a struct, enum, or union
+  --> $DIR/issue-43106-gating-of-builtin-attrs-error.rs:145:12
+   |
+LL |     #[repr(C)] fn f() { }
+   |            ^   ---------- not a struct, enum, or union
+
+error[E0517]: attribute should be applied to a struct, enum, or union
+  --> $DIR/issue-43106-gating-of-builtin-attrs-error.rs:151:12
+   |
+LL |     #[repr(C)] type T = S;
+   |            ^   ----------- not a struct, enum, or union
+
+error[E0517]: attribute should be applied to a struct, enum, or union
+  --> $DIR/issue-43106-gating-of-builtin-attrs-error.rs:155:12
+   |
+LL |     #[repr(C)] impl S { }
+   |            ^   ---------- not a struct, enum, or union
+
+error: aborting due to 39 previous errors
 
-Some errors have detailed explanations: E0518, E0658.
-For more information about an error, try `rustc --explain E0518`.
+Some errors have detailed explanations: E0517, E0518, E0658.
+For more information about an error, try `rustc --explain E0517`.

src/test/ui/feature-gates/issue-43106-gating-of-builtin-attrs.rs

@@ -245,19 +245,6 @@ mod bench {
     impl S { }
 }
 
-#[repr()]
-mod repr {
-    mod inner { #![repr()] }
-
-    #[repr()] fn f() { }
-
-    struct S;
-
-    #[repr()] type T = S;
-
-    #[repr()] impl S { }
-}
-
 #[path = "3800"]
 mod path {
     mod inner { #![path="3800"] }