lint on duplicates during attribute parsing

To do this we stuff them in the diagnostic context to be emitted after
hir is constructed

Author: jdonszelmann

compiler/rustc_ast_lowering/src/expr.rs

@@ -74,12 +74,13 @@ 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 new_attrs = self.lower_attrs_vec(&e.attrs, e.span, ex.hir_id)
+                                    .into_iter()
+                                    .chain(old_attrs.iter().cloned());
                         self.attrs.insert(
                             ex.hir_id.local_id,
                             &*self.arena.alloc_from_iter(
-                                self.lower_attrs_vec(&e.attrs, e.span)
-                                    .into_iter()
-                                    .chain(old_attrs.iter().cloned()),
+                                new_attrs,
                             ),
                         );
                     }
@@ -2025,7 +2026,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/item.rs

@@ -58,7 +58,7 @@ impl<'a, 'hir> ItemLowerer<'a, 'hir> {
         &mut self,
         owner: NodeId,
         f: impl FnOnce(&mut LoweringContext<'_, 'hir>) -> hir::OwnerNode<'hir>,
-    ) {
+    )  {
         let mut lctx = LoweringContext::new(self.tcx, self.resolver);
         lctx.with_hir_id_owner(owner, |lctx| f(lctx));
 

compiler/rustc_ast_lowering/src/lib.rs

@@ -51,7 +51,7 @@ use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
 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::def_id::{LocalDefId, CRATE_DEF_ID, LOCAL_CRATE};
 use rustc_hir::{
     self as hir, ConstArg, GenericArg, HirId, ItemLocalMap, LangItem, ParamName, TraitCandidate,
 };
@@ -188,7 +188,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             // interact with `gen`/`async gen` blocks
             allow_async_iterator: [sym::gen_future, sym::async_iterator].into(),
 
-            attribute_parser: AttributeParser::new(tcx.sess, tcx.features(), registered_tools),
+            attribute_parser: AttributeParser::new(tcx, tcx.features(), registered_tools),
         }
     }
 
@@ -197,6 +197,22 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
     }
 }
 
+struct SpanLowerer {
+    is_incremental: bool,
+    defid: LocalDefId,
+}
+
+impl SpanLowerer {
+    fn lower(&self, span: Span) -> Span {
+        if self.is_incremental {
+            span.with_parent(Some(self.defid))
+        } 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>> {
@@ -740,15 +756,17 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         })
     }
 
+    fn span_lowerer(&self) -> SpanLowerer {
+        SpanLowerer {
+            is_incremental: self.tcx.sess.opts.incremental.is_some(),
+            defid: 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 {
@@ -871,7 +889,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);
@@ -891,9 +909,20 @@ 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),
+        )
     }
 
     fn alias_attrs(&mut self, id: HirId, target_id: HirId) {

compiler/rustc_attr_parsing/messages.ftl

@@ -131,7 +131,17 @@ 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 = {-attr_parsing_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 {
+impl<S: Stage> CombineAttributeParser<S> for AllowInternalUnstableParser {
     const PATH: &'static [rustc_span::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 {
+impl<S: Stage> CombineAttributeParser<S> for AllowConstFnUnstableParser {
     const PATH: &'static [rustc_span::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

@@ -2,18 +2,17 @@ use rustc_attr_data_structures::{AttributeKind, DeprecatedSince, Deprecation};
 use rustc_span::symbol::Ident;
 use rustc_span::{Span, Symbol, sym};
 
-use super::{AttributeDuplicates, OnDuplicate, SingleAttributeParser};
 use super::util::parse_version;
-use super::{AttributeDuplicates, SingleAttributeParser};
-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>,
     ident: Ident,
     param_span: Span,
     arg: &ArgParser<'_>,
@@ -46,12 +45,12 @@ fn get(
     }
 }
 
-impl SingleAttributeParser for DeprecationParser {
+impl<S: Stage> SingleAttributeParser<S> for DeprecationParser {
     const PATH: &'static [rustc_span::Symbol] = &[sym::deprecated];
-    const ON_DUPLICATE_STRATEGY: AttributeDuplicates = AttributeDuplicates::ErrorFollowing;
-    const ON_DUPLICATE: OnDuplicate = OnDuplicate::Error;
+    const ATTRIBUTE_ORDER: AttributeOrder = AttributeOrder::KeepFirst;
+    const ON_DUPLICATE: OnDuplicate<S> = OnDuplicate::Error;
 
-    fn convert(cx: &AcceptContext<'_>, args: &ArgParser<'_>) -> Option<AttributeKind> {
+    fn convert(cx: &AcceptContext<'_, '_, S>, args: &ArgParser<'_>) -> Option<AttributeKind> {
         let features = cx.features();
 
         let mut since = None;