Infrastructure for lints during attribute parsing, specifically duplicate usages of attributes

Cargo.lock

@@ -3787,6 +3787,7 @@ dependencies = [
  "rustc_arena",
  "rustc_ast",
  "rustc_attr_data_structures",
+ "rustc_attr_parsing",
  "rustc_data_structures",
  "rustc_errors",
  "rustc_feature",

compiler/rustc_ast_lowering/src/expr.rs

@@ -73,16 +73,15 @@ impl<'hir> LoweringContext<'_, 'hir> {
                     // Merge attributes into the inner expression.
                     if !e.attrs.is_empty() {
                         let old_attrs = self.attrs.get(&ex.hir_id.local_id).copied().unwrap_or(&[]);
-                        let attrs = &*self.arena.alloc_from_iter(
-                            self.lower_attrs_vec(&e.attrs, e.span)
-                                .into_iter()
-                                .chain(old_attrs.iter().cloned()),
-                        );
-                        if attrs.is_empty() {
+                        let new_attrs = self
+                            .lower_attrs_vec(&e.attrs, e.span, ex.hir_id)
+                            .into_iter()
+                            .chain(old_attrs.iter().cloned());
+                        let new_attrs = &*self.arena.alloc_from_iter(new_attrs);
+                        if new_attrs.is_empty() {
                             return ex;
                         }
-
-                        self.attrs.insert(ex.hir_id.local_id, attrs);
+                        self.attrs.insert(ex.hir_id.local_id, new_attrs);
                     }
                     return ex;
                 }
@@ -2035,7 +2034,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
                 let ret_expr = self.checked_return(Some(from_residual_expr));
                 self.arena.alloc(self.expr(try_span, ret_expr))
             };
-            self.lower_attrs(ret_expr.hir_id, &attrs, ret_expr.span);
+            self.lower_attrs(ret_expr.hir_id, &attrs, span);
 
             let break_pat = self.pat_cf_break(try_span, residual_local);
             self.arm(break_pat, ret_expr)

compiler/rustc_ast_lowering/src/lib.rs

@@ -51,6 +51,7 @@ use rustc_data_structures::tagged_ptr::TaggedRef;
 use rustc_errors::{DiagArgFromDisplay, DiagCtxtHandle, StashKey};
 use rustc_hir::def::{DefKind, LifetimeRes, Namespace, PartialRes, PerNS, Res};
 use rustc_hir::def_id::{CRATE_DEF_ID, LOCAL_CRATE, LocalDefId};
+use rustc_hir::lints::DelayedLint;
 use rustc_hir::{
     self as hir, AngleBrackets, ConstArg, GenericArg, HirId, ItemLocalMap, LangItem,
     LifetimeSource, LifetimeSyntax, ParamName, TraitCandidate,
@@ -141,6 +142,8 @@ struct LoweringContext<'a, 'hir> {
     allow_for_await: Arc<[Symbol]>,
     allow_async_fn_traits: Arc<[Symbol]>,
 
+    delayed_lints: Vec<DelayedLint>,
+
     attribute_parser: AttributeParser<'hir>,
 }
 
@@ -190,6 +193,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             allow_async_iterator: [sym::gen_future, sym::async_iterator].into(),
 
             attribute_parser: AttributeParser::new(tcx.sess, tcx.features(), registered_tools),
+            delayed_lints: Vec::new(),
         }
     }
 
@@ -198,6 +202,22 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
     }
 }
 
+struct SpanLowerer {
+    is_incremental: bool,
+    def_id: LocalDefId,
+}
+
+impl SpanLowerer {
+    fn lower(&self, span: Span) -> Span {
+        if self.is_incremental {
+            span.with_parent(Some(self.def_id))
+        } else {
+            // Do not make spans relative when not using incremental compilation.
+            span
+        }
+    }
+}
+
 #[extension(trait ResolverAstLoweringExt)]
 impl ResolverAstLowering {
     fn legacy_const_generic_args(&self, expr: &Expr) -> Option<Vec<usize>> {
@@ -573,6 +593,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             std::mem::replace(&mut self.item_local_id_counter, hir::ItemLocalId::new(1));
         let current_impl_trait_defs = std::mem::take(&mut self.impl_trait_defs);
         let current_impl_trait_bounds = std::mem::take(&mut self.impl_trait_bounds);
+        let current_delayed_lints = std::mem::take(&mut self.delayed_lints);
 
         // Do not reset `next_node_id` and `node_id_to_def_id`:
         // we want `f` to be able to refer to the `LocalDefId`s that the caller created.
@@ -606,6 +627,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         self.item_local_id_counter = current_local_counter;
         self.impl_trait_defs = current_impl_trait_defs;
         self.impl_trait_bounds = current_impl_trait_bounds;
+        self.delayed_lints = current_delayed_lints;
 
         debug_assert!(!self.children.iter().any(|(id, _)| id == &owner_id.def_id));
         self.children.push((owner_id.def_id, hir::MaybeOwner::Owner(info)));
@@ -616,6 +638,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         let mut bodies = std::mem::take(&mut self.bodies);
         let define_opaque = std::mem::take(&mut self.define_opaque);
         let trait_map = std::mem::take(&mut self.trait_map);
+        let delayed_lints = std::mem::take(&mut self.delayed_lints).into_boxed_slice();
 
         #[cfg(debug_assertions)]
         for (id, attrs) in attrs.iter() {
@@ -629,14 +652,16 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         let bodies = SortedMap::from_presorted_elements(bodies);
 
         // Don't hash unless necessary, because it's expensive.
-        let (opt_hash_including_bodies, attrs_hash) =
-            self.tcx.hash_owner_nodes(node, &bodies, &attrs, define_opaque);
+        let (opt_hash_including_bodies, attrs_hash, delayed_lints_hash) =
+            self.tcx.hash_owner_nodes(node, &bodies, &attrs, &delayed_lints, define_opaque);
         let num_nodes = self.item_local_id_counter.as_usize();
         let (nodes, parenting) = index::index_hir(self.tcx, node, &bodies, num_nodes);
         let nodes = hir::OwnerNodes { opt_hash_including_bodies, nodes, bodies };
         let attrs = hir::AttributeMap { map: attrs, opt_hash: attrs_hash, define_opaque };
+        let delayed_lints =
+            hir::lints::DelayedLints { lints: delayed_lints, opt_hash: delayed_lints_hash };
 
-        self.arena.alloc(hir::OwnerInfo { nodes, parenting, attrs, trait_map })
+        self.arena.alloc(hir::OwnerInfo { nodes, parenting, attrs, trait_map, delayed_lints })
     }
 
     /// This method allocates a new `HirId` for the given `NodeId`.
@@ -759,15 +784,17 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         })
     }
 
+    fn span_lowerer(&self) -> SpanLowerer {
+        SpanLowerer {
+            is_incremental: self.tcx.sess.opts.incremental.is_some(),
+            def_id: self.current_hir_id_owner.def_id,
+        }
+    }
+
     /// Intercept all spans entering HIR.
     /// Mark a span as relative to the current owning item.
     fn lower_span(&self, span: Span) -> Span {
-        if self.tcx.sess.opts.incremental.is_some() {
-            span.with_parent(Some(self.current_hir_id_owner.def_id))
-        } else {
-            // Do not make spans relative when not using incremental compilation.
-            span
-        }
+        self.span_lowerer().lower(span)
     }
 
     fn lower_ident(&self, ident: Ident) -> Ident {
@@ -889,7 +916,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         if attrs.is_empty() {
             &[]
         } else {
-            let lowered_attrs = self.lower_attrs_vec(attrs, self.lower_span(target_span));
+            let lowered_attrs = self.lower_attrs_vec(attrs, self.lower_span(target_span), id);
 
             debug_assert_eq!(id.owner, self.current_hir_id_owner);
             let ret = self.arena.alloc_from_iter(lowered_attrs);
@@ -909,9 +936,23 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         }
     }
 
-    fn lower_attrs_vec(&self, attrs: &[Attribute], target_span: Span) -> Vec<hir::Attribute> {
-        self.attribute_parser
-            .parse_attribute_list(attrs, target_span, OmitDoc::Lower, |s| self.lower_span(s))
+    fn lower_attrs_vec(
+        &mut self,
+        attrs: &[Attribute],
+        target_span: Span,
+        target_hir_id: HirId,
+    ) -> Vec<hir::Attribute> {
+        let l = self.span_lowerer();
+        self.attribute_parser.parse_attribute_list(
+            attrs,
+            target_span,
+            target_hir_id,
+            OmitDoc::Lower,
+            |s| l.lower(s),
+            |l| {
+                self.delayed_lints.push(DelayedLint::AttributeParsing(l));
+            },
+        )
     }
 
     fn alias_attrs(&mut self, id: HirId, target_id: HirId) {

compiler/rustc_attr_data_structures/src/lib.rs

@@ -8,6 +8,8 @@ mod attributes;
 mod stability;
 mod version;
 
+pub mod lints;
+
 use std::num::NonZero;
 
 pub use attributes::*;

compiler/rustc_attr_data_structures/src/lints.rs

@@ -0,0 +1,14 @@
+use rustc_macros::HashStable_Generic;
+use rustc_span::Span;
+
+#[derive(Clone, Debug, HashStable_Generic)]
+pub struct AttributeLint<Id> {
+    pub id: Id,
+    pub span: Span,
+    pub kind: AttributeLintKind,
+}
+
+#[derive(Clone, Debug, HashStable_Generic)]
+pub enum AttributeLintKind {
+    UnusedDuplicate { this: Span, other: Span, warning: bool },
+}

compiler/rustc_attr_parsing/messages.ftl

@@ -131,7 +131,15 @@ attr_parsing_unsupported_literal_generic =
 attr_parsing_unsupported_literal_suggestion =
     consider removing the prefix
 
+attr_parsing_unused_duplicate =
+    unused attribute
+    .suggestion = remove this attribute
+    .note = attribute also specified here
+    .warn = {-passes_previously_accepted}
 attr_parsing_unused_multiple =
     multiple `{$name}` attributes
     .suggestion = remove this attribute
     .note = attribute also specified here
+
+-attr_parsing_perviously_accepted =
+    this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!

compiler/rustc_attr_parsing/src/attributes/allow_unstable.rs

@@ -4,41 +4,43 @@ use rustc_attr_data_structures::AttributeKind;
 use rustc_span::{Span, Symbol, sym};
 
 use super::{CombineAttributeParser, ConvertFn};
-use crate::context::AcceptContext;
+use crate::context::{AcceptContext, Stage};
 use crate::parser::ArgParser;
 use crate::session_diagnostics;
 
 pub(crate) struct AllowInternalUnstableParser;
-impl CombineAttributeParser for AllowInternalUnstableParser {
-    const PATH: &'static [Symbol] = &[sym::allow_internal_unstable];
+impl<S: Stage> CombineAttributeParser<S> for AllowInternalUnstableParser {
+    const PATH: &[Symbol] = &[sym::allow_internal_unstable];
     type Item = (Symbol, Span);
     const CONVERT: ConvertFn<Self::Item> = AttributeKind::AllowInternalUnstable;
 
-    fn extend<'a>(
-        cx: &'a AcceptContext<'a>,
-        args: &'a ArgParser<'a>,
-    ) -> impl IntoIterator<Item = Self::Item> + 'a {
-        parse_unstable(cx, args, Self::PATH[0]).into_iter().zip(iter::repeat(cx.attr_span))
+    fn extend<'c>(
+        cx: &'c mut AcceptContext<'_, '_, S>,
+        args: &'c ArgParser<'_>,
+    ) -> impl IntoIterator<Item = Self::Item> {
+        parse_unstable(cx, args, <Self as CombineAttributeParser<S>>::PATH[0])
+            .into_iter()
+            .zip(iter::repeat(cx.attr_span))
     }
 }
 
 pub(crate) struct AllowConstFnUnstableParser;
-impl CombineAttributeParser for AllowConstFnUnstableParser {
-    const PATH: &'static [Symbol] = &[sym::rustc_allow_const_fn_unstable];
+impl<S: Stage> CombineAttributeParser<S> for AllowConstFnUnstableParser {
+    const PATH: &[Symbol] = &[sym::rustc_allow_const_fn_unstable];
     type Item = Symbol;
     const CONVERT: ConvertFn<Self::Item> = AttributeKind::AllowConstFnUnstable;
 
-    fn extend<'a>(
-        cx: &'a AcceptContext<'a>,
-        args: &'a ArgParser<'a>,
-    ) -> impl IntoIterator<Item = Self::Item> + 'a {
-        parse_unstable(cx, args, Self::PATH[0])
+    fn extend<'c>(
+        cx: &'c mut AcceptContext<'_, '_, S>,
+        args: &'c ArgParser<'_>,
+    ) -> impl IntoIterator<Item = Self::Item> + 'c {
+        parse_unstable(cx, args, <Self as CombineAttributeParser<S>>::PATH[0])
     }
 }
 
-fn parse_unstable<'a>(
-    cx: &AcceptContext<'_>,
-    args: &'a ArgParser<'a>,
+fn parse_unstable<S: Stage>(
+    cx: &AcceptContext<'_, '_, S>,
+    args: &ArgParser<'_>,
     symbol: Symbol,
 ) -> impl IntoIterator<Item = Symbol> {
     let mut res = Vec::new();

compiler/rustc_attr_parsing/src/attributes/confusables.rs

@@ -3,7 +3,7 @@ use rustc_span::{Span, Symbol, sym};
 use thin_vec::ThinVec;
 
 use super::{AcceptMapping, AttributeParser};
-use crate::context::FinalizeContext;
+use crate::context::{FinalizeContext, Stage};
 use crate::session_diagnostics;
 
 #[derive(Default)]
@@ -12,8 +12,8 @@ pub(crate) struct ConfusablesParser {
     first_span: Option<Span>,
 }
 
-impl AttributeParser for ConfusablesParser {
-    const ATTRIBUTES: AcceptMapping<Self> = &[(&[sym::rustc_confusables], |this, cx, args| {
+impl<S: Stage> AttributeParser<S> for ConfusablesParser {
+    const ATTRIBUTES: AcceptMapping<Self, S> = &[(&[sym::rustc_confusables], |this, cx, args| {
         let Some(list) = args.list() else {
             // FIXME(jdonszelmann): error when not a list? Bring validation code here.
             //       NOTE: currently subsequent attributes are silently ignored using
@@ -45,7 +45,7 @@ impl AttributeParser for ConfusablesParser {
         this.first_span.get_or_insert(cx.attr_span);
     })];
 
-    fn finalize(self, _cx: &FinalizeContext<'_>) -> Option<AttributeKind> {
+    fn finalize(self, _cx: &FinalizeContext<'_, '_, S>) -> Option<AttributeKind> {
         if self.confusables.is_empty() {
             return None;
         }

compiler/rustc_attr_parsing/src/attributes/deprecation.rs

@@ -1,17 +1,17 @@
 use rustc_attr_data_structures::{AttributeKind, DeprecatedSince, Deprecation};
 use rustc_span::{Span, Symbol, sym};
 
-use super::SingleAttributeParser;
 use super::util::parse_version;
-use crate::context::AcceptContext;
+use super::{AttributeOrder, OnDuplicate, SingleAttributeParser};
+use crate::context::{AcceptContext, Stage};
 use crate::parser::ArgParser;
 use crate::session_diagnostics;
 use crate::session_diagnostics::UnsupportedLiteralReason;
 
 pub(crate) struct DeprecationParser;
 
-fn get(
-    cx: &AcceptContext<'_>,
+fn get<S: Stage>(
+    cx: &AcceptContext<'_, '_, S>,
     name: Symbol,
     param_span: Span,
     arg: &ArgParser<'_>,
@@ -41,19 +41,12 @@ fn get(
     }
 }
 
-impl SingleAttributeParser for DeprecationParser {
-    const PATH: &'static [Symbol] = &[sym::deprecated];
+impl<S: Stage> SingleAttributeParser<S> for DeprecationParser {
+    const PATH: &[Symbol] = &[sym::deprecated];
+    const ATTRIBUTE_ORDER: AttributeOrder = AttributeOrder::KeepFirst;
+    const ON_DUPLICATE: OnDuplicate<S> = OnDuplicate::Error;
 
-    fn on_duplicate(cx: &AcceptContext<'_>, first_span: Span) {
-        // FIXME(jdonszelmann): merge with errors from check_attrs.rs
-        cx.emit_err(session_diagnostics::UnusedMultiple {
-            this: cx.attr_span,
-            other: first_span,
-            name: sym::deprecated,
-        });
-    }
-
-    fn convert(cx: &AcceptContext<'_>, args: &ArgParser<'_>) -> Option<AttributeKind> {
+    fn convert(cx: &mut AcceptContext<'_, '_, S>, args: &ArgParser<'_>) -> Option<AttributeKind> {
         let features = cx.features();
 
         let mut since = None;