From ce7cdf209a0b6ba53cfa209ee0c13b61e31028e3 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 1 May 2025 16:09:53 +1000 Subject: [PATCH 1/4] Introduce some typedefs to improve readability. --- compiler/rustc_attr_parsing/src/context.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_attr_parsing/src/context.rs b/compiler/rustc_attr_parsing/src/context.rs index 55c3df003fe16..6edb9133eabda 100644 --- a/compiler/rustc_attr_parsing/src/context.rs +++ b/compiler/rustc_attr_parsing/src/context.rs @@ -26,12 +26,16 @@ macro_rules! attribute_groups { ( pub(crate) static $name: ident = [$($names: ty),* $(,)?]; ) => { - pub(crate) static $name: LazyLock<( - BTreeMap<&'static [Symbol], Vec, &ArgParser<'_>) + Send + Sync>>>, - Vec) -> Option>> - )> = LazyLock::new(|| { - let mut accepts = BTreeMap::<_, Vec, &ArgParser<'_>) + Send + Sync>>>::new(); - let mut finalizes = Vec::) -> Option>>::new(); + type Accepts = BTreeMap< + &'static [Symbol], + Vec, &ArgParser<'_>)>> + >; + type Finalizes = Vec< + Box) -> Option> + >; + pub(crate) static $name: LazyLock<(Accepts, Finalizes)> = LazyLock::new(|| { + let mut accepts = Accepts::new(); + let mut finalizes = Finalizes::new(); $( { thread_local! { From 92be9b0696c1a7e836de57edc2a47246f6db3185 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 1 May 2025 15:56:03 +1000 Subject: [PATCH 2/4] Simplify `Accepts`, part 1. Every slice of symbols has a single entry. Let's just use a symbol directly. --- .../src/attributes/allow_unstable.rs | 8 ++++---- .../src/attributes/confusables.rs | 2 +- .../src/attributes/deprecation.rs | 4 ++-- .../rustc_attr_parsing/src/attributes/mod.rs | 8 ++++---- .../rustc_attr_parsing/src/attributes/repr.rs | 2 +- .../src/attributes/stability.rs | 16 ++++++++-------- .../src/attributes/transparency.rs | 6 +++--- compiler/rustc_attr_parsing/src/context.rs | 6 ++++-- 8 files changed, 27 insertions(+), 25 deletions(-) diff --git a/compiler/rustc_attr_parsing/src/attributes/allow_unstable.rs b/compiler/rustc_attr_parsing/src/attributes/allow_unstable.rs index d37ede86cfd2a..513ffe9b0e7dd 100644 --- a/compiler/rustc_attr_parsing/src/attributes/allow_unstable.rs +++ b/compiler/rustc_attr_parsing/src/attributes/allow_unstable.rs @@ -10,7 +10,7 @@ use crate::session_diagnostics; pub(crate) struct AllowInternalUnstableParser; impl CombineAttributeParser for AllowInternalUnstableParser { - const PATH: &'static [rustc_span::Symbol] = &[sym::allow_internal_unstable]; + const PATH: Symbol = sym::allow_internal_unstable; type Item = (Symbol, Span); const CONVERT: ConvertFn = AttributeKind::AllowInternalUnstable; @@ -18,13 +18,13 @@ impl CombineAttributeParser for AllowInternalUnstableParser { cx: &'a AcceptContext<'a>, args: &'a ArgParser<'a>, ) -> impl IntoIterator + 'a { - parse_unstable(cx, args, Self::PATH[0]).into_iter().zip(iter::repeat(cx.attr_span)) + parse_unstable(cx, args, Self::PATH).into_iter().zip(iter::repeat(cx.attr_span)) } } pub(crate) struct AllowConstFnUnstableParser; impl CombineAttributeParser for AllowConstFnUnstableParser { - const PATH: &'static [rustc_span::Symbol] = &[sym::rustc_allow_const_fn_unstable]; + const PATH: Symbol = sym::rustc_allow_const_fn_unstable; type Item = Symbol; const CONVERT: ConvertFn = AttributeKind::AllowConstFnUnstable; @@ -32,7 +32,7 @@ impl CombineAttributeParser for AllowConstFnUnstableParser { cx: &'a AcceptContext<'a>, args: &'a ArgParser<'a>, ) -> impl IntoIterator + 'a { - parse_unstable(cx, args, Self::PATH[0]) + parse_unstable(cx, args, Self::PATH) } } diff --git a/compiler/rustc_attr_parsing/src/attributes/confusables.rs b/compiler/rustc_attr_parsing/src/attributes/confusables.rs index 6cff952fcf229..fa85a7178ecea 100644 --- a/compiler/rustc_attr_parsing/src/attributes/confusables.rs +++ b/compiler/rustc_attr_parsing/src/attributes/confusables.rs @@ -13,7 +13,7 @@ pub(crate) struct ConfusablesParser { } impl AttributeParser for ConfusablesParser { - const ATTRIBUTES: AcceptMapping = &[(&[sym::rustc_confusables], |this, cx, args| { + const ATTRIBUTES: AcceptMapping = &[(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 diff --git a/compiler/rustc_attr_parsing/src/attributes/deprecation.rs b/compiler/rustc_attr_parsing/src/attributes/deprecation.rs index 7d1417446b21d..c6ae30cdf1c5c 100644 --- a/compiler/rustc_attr_parsing/src/attributes/deprecation.rs +++ b/compiler/rustc_attr_parsing/src/attributes/deprecation.rs @@ -46,9 +46,9 @@ fn get( } impl SingleAttributeParser for DeprecationParser { - const PATH: &'static [rustc_span::Symbol] = &[sym::deprecated]; + const PATH: Symbol = sym::deprecated; - fn on_duplicate(cx: &AcceptContext<'_>, first_span: rustc_span::Span) { + 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, diff --git a/compiler/rustc_attr_parsing/src/attributes/mod.rs b/compiler/rustc_attr_parsing/src/attributes/mod.rs index 6ecd6b4d7dbb7..3632265c935d2 100644 --- a/compiler/rustc_attr_parsing/src/attributes/mod.rs +++ b/compiler/rustc_attr_parsing/src/attributes/mod.rs @@ -17,7 +17,7 @@ use std::marker::PhantomData; use rustc_attr_data_structures::AttributeKind; -use rustc_span::Span; +use rustc_span::{Span, Symbol}; use thin_vec::ThinVec; use crate::context::{AcceptContext, FinalizeContext}; @@ -33,7 +33,7 @@ pub(crate) mod transparency; pub(crate) mod util; type AcceptFn = fn(&mut T, &AcceptContext<'_>, &ArgParser<'_>); -type AcceptMapping = &'static [(&'static [rustc_span::Symbol], AcceptFn)]; +type AcceptMapping = &'static [(Symbol, AcceptFn)]; /// An [`AttributeParser`] is a type which searches for syntactic attributes. /// @@ -72,7 +72,7 @@ pub(crate) trait AttributeParser: Default + 'static { /// [`SingleAttributeParser`] can only convert attributes one-to-one, and cannot combine multiple /// attributes together like is necessary for `#[stable()]` and `#[unstable()]` for example. pub(crate) trait SingleAttributeParser: 'static { - const PATH: &'static [rustc_span::Symbol]; + const PATH: Symbol; /// Caled when a duplicate attribute is found. /// @@ -119,7 +119,7 @@ type ConvertFn = fn(ThinVec) -> AttributeKind; /// [`CombineAttributeParser`] can only convert a single kind of attribute, and cannot combine multiple /// attributes together like is necessary for `#[stable()]` and `#[unstable()]` for example. pub(crate) trait CombineAttributeParser: 'static { - const PATH: &'static [rustc_span::Symbol]; + const PATH: Symbol; type Item; const CONVERT: ConvertFn; diff --git a/compiler/rustc_attr_parsing/src/attributes/repr.rs b/compiler/rustc_attr_parsing/src/attributes/repr.rs index 26ca637faec68..8efb27117c955 100644 --- a/compiler/rustc_attr_parsing/src/attributes/repr.rs +++ b/compiler/rustc_attr_parsing/src/attributes/repr.rs @@ -21,7 +21,7 @@ pub(crate) struct ReprParser; impl CombineAttributeParser for ReprParser { type Item = (ReprAttr, Span); - const PATH: &'static [rustc_span::Symbol] = &[sym::repr]; + const PATH: Symbol = sym::repr; const CONVERT: ConvertFn = AttributeKind::Repr; fn extend<'a>( diff --git a/compiler/rustc_attr_parsing/src/attributes/stability.rs b/compiler/rustc_attr_parsing/src/attributes/stability.rs index bdad6b50186dd..8da756eadd78b 100644 --- a/compiler/rustc_attr_parsing/src/attributes/stability.rs +++ b/compiler/rustc_attr_parsing/src/attributes/stability.rs @@ -43,7 +43,7 @@ impl StabilityParser { impl AttributeParser for StabilityParser { const ATTRIBUTES: AcceptMapping = &[ - (&[sym::stable], |this, cx, args| { + (sym::stable, |this, cx, args| { reject_outside_std!(cx); if !this.check_duplicate(cx) && let Some((feature, level)) = parse_stability(cx, args) @@ -51,7 +51,7 @@ impl AttributeParser for StabilityParser { this.stability = Some((Stability { level, feature }, cx.attr_span)); } }), - (&[sym::unstable], |this, cx, args| { + (sym::unstable, |this, cx, args| { reject_outside_std!(cx); if !this.check_duplicate(cx) && let Some((feature, level)) = parse_unstability(cx, args) @@ -59,7 +59,7 @@ impl AttributeParser for StabilityParser { this.stability = Some((Stability { level, feature }, cx.attr_span)); } }), - (&[sym::rustc_allowed_through_unstable_modules], |this, cx, args| { + (sym::rustc_allowed_through_unstable_modules, |this, cx, args| { reject_outside_std!(cx); this.allowed_through_unstable_modules = args.name_value().and_then(|i| i.value_as_str()) }), @@ -97,7 +97,7 @@ pub(crate) struct BodyStabilityParser { impl AttributeParser for BodyStabilityParser { const ATTRIBUTES: AcceptMapping = - &[(&[sym::rustc_default_body_unstable], |this, cx, args| { + &[(sym::rustc_default_body_unstable, |this, cx, args| { reject_outside_std!(cx); if this.stability.is_some() { cx.dcx() @@ -117,7 +117,7 @@ impl AttributeParser for BodyStabilityParser { pub(crate) struct ConstStabilityIndirectParser; // FIXME(jdonszelmann): single word attribute group when we have these impl SingleAttributeParser for ConstStabilityIndirectParser { - const PATH: &'static [rustc_span::Symbol] = &[sym::rustc_const_stable_indirect]; + const PATH: Symbol = sym::rustc_const_stable_indirect; // ignore fn on_duplicate(_cx: &AcceptContext<'_>, _first_span: Span) {} @@ -147,7 +147,7 @@ impl ConstStabilityParser { impl AttributeParser for ConstStabilityParser { const ATTRIBUTES: AcceptMapping = &[ - (&[sym::rustc_const_stable], |this, cx, args| { + (sym::rustc_const_stable, |this, cx, args| { reject_outside_std!(cx); if !this.check_duplicate(cx) @@ -159,7 +159,7 @@ impl AttributeParser for ConstStabilityParser { )); } }), - (&[sym::rustc_const_unstable], |this, cx, args| { + (sym::rustc_const_unstable, |this, cx, args| { reject_outside_std!(cx); if !this.check_duplicate(cx) && let Some((feature, level)) = parse_unstability(cx, args) @@ -170,7 +170,7 @@ impl AttributeParser for ConstStabilityParser { )); } }), - (&[sym::rustc_promotable], |this, cx, _| { + (sym::rustc_promotable, |this, cx, _| { reject_outside_std!(cx); this.promotable = true; }), diff --git a/compiler/rustc_attr_parsing/src/attributes/transparency.rs b/compiler/rustc_attr_parsing/src/attributes/transparency.rs index ce42b0507ed57..04c9d2471527d 100644 --- a/compiler/rustc_attr_parsing/src/attributes/transparency.rs +++ b/compiler/rustc_attr_parsing/src/attributes/transparency.rs @@ -1,6 +1,6 @@ use rustc_attr_data_structures::AttributeKind; use rustc_span::hygiene::Transparency; -use rustc_span::sym; +use rustc_span::{Span, Symbol, sym}; use super::{AcceptContext, SingleAttributeParser}; use crate::parser::ArgParser; @@ -11,9 +11,9 @@ pub(crate) struct TransparencyParser; #[allow(rustc::untranslatable_diagnostic)] #[allow(rustc::diagnostic_outside_of_impl)] impl SingleAttributeParser for TransparencyParser { - const PATH: &'static [rustc_span::Symbol] = &[sym::rustc_macro_transparency]; + const PATH: Symbol = sym::rustc_macro_transparency; - fn on_duplicate(cx: &crate::context::AcceptContext<'_>, first_span: rustc_span::Span) { + fn on_duplicate(cx: &crate::context::AcceptContext<'_>, first_span: Span) { cx.dcx().span_err(vec![first_span, cx.attr_span], "multiple macro transparency attributes"); } diff --git a/compiler/rustc_attr_parsing/src/context.rs b/compiler/rustc_attr_parsing/src/context.rs index 6edb9133eabda..71b3db39aaca6 100644 --- a/compiler/rustc_attr_parsing/src/context.rs +++ b/compiler/rustc_attr_parsing/src/context.rs @@ -27,7 +27,7 @@ macro_rules! attribute_groups { pub(crate) static $name: ident = [$($names: ty),* $(,)?]; ) => { type Accepts = BTreeMap< - &'static [Symbol], + Symbol, Vec, &ArgParser<'_>)>> >; type Finalizes = Vec< @@ -267,7 +267,9 @@ impl<'sess> AttributeParser<'sess> { let (path, args) = parser.deconstruct(); let parts = path.segments().map(|i| i.name).collect::>(); - if let Some(accepts) = ATTRIBUTE_MAPPING.0.get(parts.as_slice()) { + if let [part] = &parts[..] + && let Some(accepts) = ATTRIBUTE_MAPPING.0.get(part) + { for f in accepts { let cx = AcceptContext { group_cx: &group_cx, From ea1d9eb8b0d0ee6a3336a3b496e1f71bdf29cc77 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 1 May 2025 16:09:16 +1000 Subject: [PATCH 3/4] Simplify `Accepts`, part 2. There only needs to be one `Fn` per symbol, not multiple. --- compiler/rustc_attr_parsing/src/context.rs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_attr_parsing/src/context.rs b/compiler/rustc_attr_parsing/src/context.rs index 71b3db39aaca6..f4280bfab1d4f 100644 --- a/compiler/rustc_attr_parsing/src/context.rs +++ b/compiler/rustc_attr_parsing/src/context.rs @@ -28,7 +28,7 @@ macro_rules! attribute_groups { ) => { type Accepts = BTreeMap< Symbol, - Vec, &ArgParser<'_>)>> + Box, &ArgParser<'_>)> >; type Finalizes = Vec< Box) -> Option> @@ -43,11 +43,12 @@ macro_rules! attribute_groups { }; for (k, v) in <$names>::ATTRIBUTES { - accepts.entry(*k).or_default().push(Box::new(|cx, args| { + let old = accepts.insert(*k, Box::new(|cx, args| { STATE_OBJECT.with_borrow_mut(|s| { v(s, cx, args) }) })); + assert!(old.is_none()); } finalizes.push(Box::new(|cx| { @@ -268,16 +269,12 @@ impl<'sess> AttributeParser<'sess> { let parts = path.segments().map(|i| i.name).collect::>(); if let [part] = &parts[..] - && let Some(accepts) = ATTRIBUTE_MAPPING.0.get(part) + && let Some(accept) = ATTRIBUTE_MAPPING.0.get(part) { - for f in accepts { - let cx = AcceptContext { - group_cx: &group_cx, - attr_span: lower_span(attr.span), - }; - - f(&cx, &args) - } + let cx = + AcceptContext { group_cx: &group_cx, attr_span: lower_span(attr.span) }; + + accept(&cx, &args) } else { // if we're here, we must be compiling a tool attribute... Or someone forgot to // parse their fancy new attribute. Let's warn them in any case. If you are that From e1ab1169d592f80e695e3f4f6ceabfb25343511d Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 1 May 2025 16:24:48 +1000 Subject: [PATCH 4/4] Fix up some comments. Some are too long (> 100 chars), some are twoo short, some are missing full stops, some are missing upper-case letters at the start of sentences. --- compiler/rustc_attr_parsing/src/context.rs | 31 +++++++++++----------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_attr_parsing/src/context.rs b/compiler/rustc_attr_parsing/src/context.rs index f4280bfab1d4f..9ed3a6f2c7571 100644 --- a/compiler/rustc_attr_parsing/src/context.rs +++ b/compiler/rustc_attr_parsing/src/context.rs @@ -115,7 +115,8 @@ impl<'a> Deref for AcceptContext<'a> { /// Context given to every attribute parser during finalization. /// -/// Gives [`AttributeParser`](crate::attributes::AttributeParser)s enough information to create errors, for example. +/// Gives [`AttributeParser`](crate::attributes::AttributeParser)s enough information to create +/// errors, for example. pub(crate) struct FinalizeContext<'a> { /// The parse context, gives access to the session and the /// diagnostics context. @@ -146,10 +147,9 @@ pub struct AttributeParser<'sess> { sess: &'sess Session, features: Option<&'sess Features>, - /// *only* parse attributes with this symbol. + /// *Only* parse attributes with this symbol. /// - /// Used in cases where we want the lowering infrastructure for - /// parse just a single attribute. + /// Used in cases where we want the lowering infrastructure for parse just a single attribute. parse_only: Option, /// Can be used to instruct parsers to reduce the number of diagnostics it emits. @@ -162,9 +162,9 @@ impl<'sess> AttributeParser<'sess> { /// One example where this is necessary, is to parse `feature` attributes themselves for /// example. /// - /// Try to use this as little as possible. Attributes *should* be lowered during `rustc_ast_lowering`. - /// Some attributes require access to features to parse, which would crash if you tried to do so - /// through [`parse_limited`](Self::parse_limited). + /// Try to use this as little as possible. Attributes *should* be lowered during + /// `rustc_ast_lowering`. Some attributes require access to features to parse, which would + /// crash if you tried to do so through [`parse_limited`](Self::parse_limited). /// /// To make sure use is limited, supply a `Symbol` you'd like to parse. Only attributes with /// that symbol are picked out of the list of instructions and parsed. Those are returned. @@ -222,19 +222,18 @@ impl<'sess> AttributeParser<'sess> { let group_cx = FinalizeContext { cx: self, target_span }; for attr in attrs { - // if we're only looking for a single attribute, - // skip all the ones we don't care about + // If we're only looking for a single attribute, skip all the ones we don't care about. if let Some(expected) = self.parse_only { if !attr.has_name(expected) { continue; } } - // sometimes, for example for `#![doc = include_str!("readme.md")]`, + // Sometimes, for example for `#![doc = include_str!("readme.md")]`, // doc still contains a non-literal. You might say, when we're lowering attributes // that's expanded right? But no, sometimes, when parsing attributes on macros, // we already use the lowering logic and these are still there. So, when `omit_doc` - // is set we *also* want to ignore these + // is set we *also* want to ignore these. if omit_doc == OmitDoc::Skip && attr.has_name(sym::doc) { continue; } @@ -276,11 +275,11 @@ impl<'sess> AttributeParser<'sess> { accept(&cx, &args) } else { - // if we're here, we must be compiling a tool attribute... Or someone forgot to - // parse their fancy new attribute. Let's warn them in any case. If you are that - // person, and you really your attribute should remain unparsed, carefully read the - // documentation in this module and if you still think so you can add an exception - // to this assertion. + // If we're here, we must be compiling a tool attribute... Or someone + // forgot to parse their fancy new attribute. Let's warn them in any case. + // If you are that person, and you really think your attribute should + // remain unparsed, carefully read the documentation in this module and if + // you still think so you can add an exception to this assertion. // FIXME(jdonszelmann): convert other attributes, and check with this that // we caught em all