Rework illformed attribute check for unparsed attributes

Signed-off-by: Jonathan Brouwer <jonathantbrouwer@gmail.com>

Author: JonathanBrouwer

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,12 @@ mod private {
 #[allow(private_interfaces)]
 pub trait Stage: Sized + 'static + Sealed {
     type Id: Copy;
-    const SHOULD_EMIT_LINTS: bool;
+    const IS_LATE: bool;
 
     fn parsers() -> &'static group_type!(Self);
 
+    fn should_emit_errors(&self) -> bool;
+
     fn emit_err<'sess>(
         &self,
         sess: &'sess Session,
@@ -218,11 +220,16 @@ pub trait Stage: Sized + 'static + Sealed {
 #[allow(private_interfaces)]
 impl Stage for Early {
     type Id = NodeId;
-    const SHOULD_EMIT_LINTS: bool = false;
+    const IS_LATE: bool = false;
 
     fn parsers() -> &'static group_type!(Self) {
         &early::ATTRIBUTE_PARSERS
     }
+
+    fn should_emit_errors(&self) -> bool {
+        self.emit_errors.should_emit()
+    }
+
     fn emit_err<'sess>(
         &self,
         sess: &'sess Session,
@@ -240,11 +247,16 @@ impl Stage for Early {
 #[allow(private_interfaces)]
 impl Stage for Late {
     type Id = HirId;
-    const SHOULD_EMIT_LINTS: bool = true;
+    const IS_LATE: bool = true;
 
     fn parsers() -> &'static group_type!(Self) {
         &late::ATTRIBUTE_PARSERS
     }
+
+    fn should_emit_errors(&self) -> bool {
+        true
+    }
+
     fn emit_err<'sess>(
         &self,
         tcx: &'sess Session,
@@ -290,7 +302,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 !S::IS_LATE {
             return;
         }
         let id = self.target_id;
@@ -696,6 +708,29 @@ 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 |_lint| {
+                panic!("can't emit lints here for now (nothing uses this atm)");
+            },
+            true,
+        )
+    }
 }
 
 impl<'sess, S: Stage> AttributeParser<'sess, S> {
@@ -712,6 +747,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 +824,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 +862,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(),
+                    },
+                },
+            )
+        }
     }
 }