Auto merge of #143963 - JonathanBrouwer:rework-illformed-attr, r=<try>

Move attribute validation from `rustc_parse` to `rustc_attr_parsing`

This moves the old attribute validation from `rustc_parse` to `rustc_attr_parsing`.
This PR is easiest to review commit-by-commit.

This is part 1 of 2, of a fix for #143940, the perfomance regression caused by #143460.
This PR does not yet fix the performance regression but does the refactoring work to prepare for fixing it. After all these changes there should be no functional change to the compiler, other than some re-ordering of the errors.

Author: rust-bors[bot]

Cargo.lock

@@ -3275,7 +3275,6 @@ dependencies = [
  "rustc_feature",
  "rustc_fluent_macro",
  "rustc_macros",
- "rustc_parse",
  "rustc_session",
  "rustc_span",
  "rustc_target",
@@ -3321,6 +3320,7 @@ dependencies = [
  "rustc_hir",
  "rustc_lexer",
  "rustc_macros",
+ "rustc_parse",
  "rustc_session",
  "rustc_span",
  "thin-vec",

compiler/rustc_ast_passes/Cargo.toml

@@ -15,7 +15,6 @@ rustc_errors = { path = "../rustc_errors" }
 rustc_feature = { path = "../rustc_feature" }
 rustc_fluent_macro = { path = "../rustc_fluent_macro" }
 rustc_macros = { path = "../rustc_macros" }
-rustc_parse = { path = "../rustc_parse" }
 rustc_session = { path = "../rustc_session" }
 rustc_span = { path = "../rustc_span" }
 rustc_target = { path = "../rustc_target" }

compiler/rustc_ast_passes/src/ast_validation.rs

@@ -29,7 +29,6 @@ use rustc_ast_pretty::pprust::{self, State};
 use rustc_data_structures::fx::FxIndexMap;
 use rustc_errors::DiagCtxtHandle;
 use rustc_feature::Features;
-use rustc_parse::validate_attr;
 use rustc_session::Session;
 use rustc_session::lint::builtin::{
     DEPRECATED_WHERE_CLAUSE_LOCATION, MISSING_ABI, MISSING_UNSAFE_ON_EXTERN,
@@ -928,10 +927,6 @@ fn validate_generic_param_order(dcx: DiagCtxtHandle<'_>, generics: &[GenericPara
 }
 
 impl<'a> Visitor<'a> for AstValidator<'a> {
-    fn visit_attribute(&mut self, attr: &Attribute) {
-        validate_attr::check_attr(&self.sess.psess, attr, self.lint_node_id);
-    }
-
     fn visit_ty(&mut self, ty: &'a Ty) {
         self.visit_ty_common(ty);
         self.walk_ty(ty)

compiler/rustc_attr_data_structures/src/lints.rs

@@ -13,4 +13,5 @@ pub enum AttributeLintKind {
     UnusedDuplicate { this: Span, other: Span, warning: bool },
     IllFormedAttributeInput { suggestions: Vec<String> },
     EmptyAttribute { first_span: Span },
+    UnsafeAttrOutsideUnsafe { attribute_name_span: Span, sugg_spans: (Span, Span) },
 }

compiler/rustc_attr_parsing/Cargo.toml

@@ -15,6 +15,7 @@ rustc_fluent_macro = { path = "../rustc_fluent_macro" }
 rustc_hir = { path = "../rustc_hir" }
 rustc_lexer = { path = "../rustc_lexer" }
 rustc_macros = { path = "../rustc_macros" }
+rustc_parse = { path = "../rustc_parse" }
 rustc_session = { path = "../rustc_session" }
 rustc_span = { path = "../rustc_span" }
 thin-vec = "0.2.12"

compiler/rustc_attr_parsing/messages.ftl

@@ -161,3 +161,12 @@ attr_parsing_unused_multiple =
 
 -attr_parsing_previously_accepted =
     this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+
+attr_parsing_unsafe_attr_outside_unsafe = unsafe attribute used without unsafe
+    .label = usage of unsafe attribute
+attr_parsing_unsafe_attr_outside_unsafe_suggestion = wrap the attribute in `unsafe(...)`
+
+attr_parsing_invalid_attr_unsafe = `{$name}` is not an unsafe attribute
+    .label = this is not an unsafe attribute
+    .suggestion = remove the `unsafe(...)`
+    .note = extraneous unsafe is not allowed in attributes

compiler/rustc_attr_parsing/src/attributes/stability.rs

@@ -247,7 +247,12 @@ pub(crate) fn parse_stability<S: Stage>(
     let mut feature = None;
     let mut since = None;
 
-    for param in args.list()?.mixed() {
+    let ArgParser::List(list) = args else {
+        cx.expected_list(cx.attr_span);
+        return None;
+    };
+
+    for param in list.mixed() {
         let param_span = param.span();
         let Some(param) = param.meta_item() else {
             cx.emit_err(session_diagnostics::UnsupportedLiteral {
@@ -322,7 +327,13 @@ pub(crate) fn parse_unstability<S: Stage>(
     let mut is_soft = false;
     let mut implied_by = None;
     let mut old_name = None;
-    for param in args.list()?.mixed() {
+
+    let ArgParser::List(list) = args else {
+        cx.expected_list(cx.attr_span);
+        return None;
+    };
+
+    for param in list.mixed() {
         let Some(param) = param.meta_item() else {
             cx.emit_err(session_diagnostics::UnsupportedLiteral {
                 span: param.span(),

compiler/rustc_attr_parsing/src/context.rs

@@ -203,10 +203,11 @@ mod private {
 #[allow(private_interfaces)]
 pub trait Stage: Sized + 'static + Sealed {
     type Id: Copy;
-    const SHOULD_EMIT_LINTS: bool;
 
     fn parsers() -> &'static group_type!(Self);
 
+    fn should_emit_errors_and_lints(&self) -> bool;
+
     fn emit_err<'sess>(
         &self,
         sess: &'sess Session,
@@ -218,11 +219,15 @@ pub trait Stage: Sized + 'static + Sealed {
 #[allow(private_interfaces)]
 impl Stage for Early {
     type Id = NodeId;
-    const SHOULD_EMIT_LINTS: bool = false;
 
     fn parsers() -> &'static group_type!(Self) {
         &early::ATTRIBUTE_PARSERS
     }
+
+    fn should_emit_errors_and_lints(&self) -> bool {
+        self.emit_errors.should_emit()
+    }
+
     fn emit_err<'sess>(
         &self,
         sess: &'sess Session,
@@ -240,11 +245,15 @@ impl Stage for Early {
 #[allow(private_interfaces)]
 impl Stage for Late {
     type Id = HirId;
-    const SHOULD_EMIT_LINTS: bool = true;
 
     fn parsers() -> &'static group_type!(Self) {
         &late::ATTRIBUTE_PARSERS
     }
+
+    fn should_emit_errors_and_lints(&self) -> bool {
+        true
+    }
+
     fn emit_err<'sess>(
         &self,
         tcx: &'sess Session,
@@ -290,7 +299,7 @@ impl<'f, 'sess: 'f, S: Stage> SharedContext<'f, 'sess, S> {
     /// must be delayed until after HIR is built. This method will take care of the details of
     /// that.
     pub(crate) fn emit_lint(&mut self, lint: AttributeLintKind, span: Span) {
-        if !S::SHOULD_EMIT_LINTS {
+        if !self.stage.should_emit_errors_and_lints() {
             return;
         }
         let id = self.target_id;
@@ -696,6 +705,22 @@ impl<'sess> AttributeParser<'sess, Early> {
         };
         parse_fn(&mut cx, args)
     }
+
+    /// Returns whether the attribute is valid
+    pub fn validate_attribute_early(
+        sess: &'sess Session,
+        attr: &ast::Attribute,
+        target_node_id: NodeId,
+    ) -> bool {
+        let parser = Self {
+            features: None,
+            tools: Vec::new(),
+            parse_only: None,
+            sess,
+            stage: Early { emit_errors: ShouldEmit::ErrorsAndLints },
+        };
+        parser.validate_attribute(attr, target_node_id, &mut |_| {}, true)
+    }
 }
 
 impl<'sess, S: Stage> AttributeParser<'sess, S> {
@@ -712,6 +737,10 @@ impl<'sess, S: Stage> AttributeParser<'sess, S> {
         &self.sess
     }
 
+    pub(crate) fn stage(&self) -> &S {
+        &self.stage
+    }
+
     pub(crate) fn features(&self) -> &'sess Features {
         self.features.expect("features not available at this point in the compiler")
     }
@@ -785,12 +814,14 @@ impl<'sess, S: Stage> AttributeParser<'sess, S> {
                 ast::AttrKind::Normal(n) => {
                     attr_paths.push(PathParser::Ast(&n.item.path));
 
+                    // Parse attribute using new infra
                     let parser = MetaItemParser::from_attr(n, self.dcx());
                     let path = parser.path();
                     let args = parser.args();
                     let parts = path.segments().map(|i| i.name).collect::<Vec<_>>();
 
                     if let Some(accepts) = S::parsers().0.get(parts.as_slice()) {
+                        self.validate_attribute(attr, target_id, &mut emit_lint, true);
                         for (template, accept) in accepts {
                             let mut cx: AcceptContext<'_, 'sess, S> = AcceptContext {
                                 shared: SharedContext {
@@ -821,7 +852,7 @@ impl<'sess, S: Stage> AttributeParser<'sess, S> {
                         //     // || FIXME_TEMPORARY_ATTR_ALLOWLIST.contains(&parts[0]),
                         //     "attribute {path} wasn't parsed and isn't a know tool attribute",
                         // );
-
+                        self.validate_attribute(attr, target_id, &mut emit_lint, false);
                         attributes.push(Attribute::Unparsed(Box::new(AttrItem {
                             path: AttrPath::from_ast(&n.item.path),
                             args: self.lower_attr_args(&n.item.args, lower_span),

compiler/rustc_attr_parsing/src/lib.rs

@@ -89,6 +89,7 @@ pub(crate) mod context;
 mod lints;
 pub mod parser;
 mod session_diagnostics;
+mod validate_attr;
 
 pub use attributes::cfg::{CFG_TEMPLATE, EvalConfigResult, eval_config_entry, parse_cfg_attr};
 pub use attributes::cfg_old::*;
@@ -97,5 +98,8 @@ pub use attributes::util::{
 };
 pub use context::{AttributeParser, Early, Late, OmitDoc, ShouldEmit};
 pub use lints::emit_attribute_lint;
+pub use validate_attr::{
+    check_attribute_safety, check_builtin_meta_item, emit_fatal_malformed_builtin_attribute,
+};
 
 rustc_fluent_macro::fluent_messages! { "../messages.ftl" }

compiler/rustc_attr_parsing/src/lints.rs

@@ -3,6 +3,7 @@ use rustc_errors::{DiagArgValue, LintEmitter};
 use rustc_hir::HirId;
 
 use crate::session_diagnostics;
+use crate::session_diagnostics::{UnsafeAttrOutsideUnsafeLint, UnsafeAttrOutsideUnsafeSuggestion};
 
 pub fn emit_attribute_lint<L: LintEmitter>(lint: &AttributeLint<HirId>, lint_emitter: L) {
     let AttributeLint { id, span, kind } = lint;
@@ -34,5 +35,19 @@ pub fn emit_attribute_lint<L: LintEmitter>(lint: &AttributeLint<HirId>, lint_emi
             *first_span,
             session_diagnostics::EmptyAttributeList { attr_span: *first_span },
         ),
+        &AttributeLintKind::UnsafeAttrOutsideUnsafe { attribute_name_span, sugg_spans } => {
+            lint_emitter.emit_node_span_lint(
+                rustc_session::lint::builtin::UNSAFE_ATTR_OUTSIDE_UNSAFE,
+                *id,
+                *span,
+                UnsafeAttrOutsideUnsafeLint {
+                    span: attribute_name_span,
+                    suggestion: UnsafeAttrOutsideUnsafeSuggestion {
+                        left: sugg_spans.0.shrink_to_lo(),
+                        right: sugg_spans.1.shrink_to_hi(),
+                    },
+                },
+            )
+        }
     }
 }