Rollup merge of #140619 - jieyouxu:validate_attr_cleanups, r=Urgau

Small adjustments to `check_attribute_safety` to make the logic more obvious

Follow-up to #140617.

Author: tgross35

compiler/rustc_expand/src/config.rs

@@ -276,7 +276,7 @@ impl<'a> StripUnconfigured<'a> {
     pub(crate) fn expand_cfg_attr(&self, cfg_attr: &Attribute, recursive: bool) -> Vec<Attribute> {
         validate_attr::check_attribute_safety(
             &self.sess.psess,
-            AttributeSafety::Normal,
+            Some(AttributeSafety::Normal),
             &cfg_attr,
             ast::CRATE_NODE_ID,
         );

compiler/rustc_parse/src/validate_attr.rs

@@ -22,15 +22,13 @@ pub fn check_attr(psess: &ParseSess, attr: &Attribute, id: NodeId) {
         return;
     }
 
-    let attr_info = attr.ident().and_then(|ident| BUILTIN_ATTRIBUTE_MAP.get(&ident.name));
-    let attr_item = attr.get_normal_item();
+    let builtin_attr_info = attr.ident().and_then(|ident| BUILTIN_ATTRIBUTE_MAP.get(&ident.name));
 
-    // All non-builtin attributes are considered safe
-    let safety = attr_info.map(|x| x.safety).unwrap_or(AttributeSafety::Normal);
-    check_attribute_safety(psess, safety, attr, id);
+    let builtin_attr_safety = builtin_attr_info.map(|x| x.safety);
+    check_attribute_safety(psess, builtin_attr_safety, attr, id);
 
     // Check input tokens for built-in and key-value attributes.
-    match attr_info {
+    match builtin_attr_info {
         // `rustc_dummy` doesn't have any restrictions specific to built-in attributes.
         Some(BuiltinAttribute { name, template, .. }) if *name != sym::rustc_dummy => {
             match parse_meta(psess, attr) {
@@ -44,6 +42,7 @@ pub fn check_attr(psess: &ParseSess, attr: &Attribute, id: NodeId) {
             }
         }
         _ => {
+            let attr_item = attr.get_normal_item();
             if let AttrArgs::Eq { .. } = attr_item.args {
                 // All key-value attributes are restricted to meta-item syntax.
                 match parse_meta(psess, attr) {
@@ -157,14 +156,21 @@ fn is_attr_template_compatible(template: &AttributeTemplate, meta: &ast::MetaIte
 
 pub fn check_attribute_safety(
     psess: &ParseSess,
-    safety: AttributeSafety,
+    builtin_attr_safety: Option<AttributeSafety>,
     attr: &Attribute,
     id: NodeId,
 ) {
     let attr_item = attr.get_normal_item();
+    match (builtin_attr_safety, attr_item.unsafety) {
+        // - Unsafe builtin attribute
+        // - User wrote `#[unsafe(..)]`, which is permitted on any edition
+        (Some(AttributeSafety::Unsafe { .. }), Safety::Unsafe(..)) => {
+            // OK
+        }
 
-    if let AttributeSafety::Unsafe { unsafe_since } = safety {
-        if let ast::Safety::Default = attr_item.unsafety {
+        // - Unsafe builtin attribute
+        // - User did not write `#[unsafe(..)]`
+        (Some(AttributeSafety::Unsafe { unsafe_since }), Safety::Default) => {
             let path_span = attr_item.path.span;
 
             // If the `attr_item`'s span is not from a macro, then just suggest
@@ -199,11 +205,38 @@ pub fn check_attribute_safety(
                 );
             }
         }
-    } else if let Safety::Unsafe(unsafe_span) = attr_item.unsafety {
-        psess.dcx().emit_err(errors::InvalidAttrUnsafe {
-            span: unsafe_span,
-            name: attr_item.path.clone(),
-        });
+
+        // - Normal builtin attribute, or any non-builtin attribute
+        // - All non-builtin attributes are currently considered safe; writing `#[unsafe(..)]` is
+        //   not permitted on non-builtin attributes or normal builtin attributes
+        (Some(AttributeSafety::Normal) | None, Safety::Unsafe(unsafe_span)) => {
+            psess.dcx().emit_err(errors::InvalidAttrUnsafe {
+                span: unsafe_span,
+                name: attr_item.path.clone(),
+            });
+        }
+
+        // - Normal builtin attribute
+        // - No explicit `#[unsafe(..)]` written.
+        (Some(AttributeSafety::Normal), Safety::Default) => {
+            // OK
+        }
+
+        // - Non-builtin attribute
+        // - No explicit `#[unsafe(..)]` written.
+        (None, Safety::Default) => {
+            // OK
+        }
+
+        (
+            Some(AttributeSafety::Unsafe { .. } | AttributeSafety::Normal) | None,
+            Safety::Safe(..),
+        ) => {
+            psess.dcx().span_delayed_bug(
+                attr_item.span(),
+                "`check_attribute_safety` does not expect `Safety::Safe` on attributes",
+            );
+        }
     }
 }
 

tests/ui/rust-2024/unsafe-attributes/auxiliary/safe_attr.rs

@@ -0,0 +1,7 @@
+extern crate proc_macro;
+use proc_macro::TokenStream;
+
+#[proc_macro_attribute]
+pub fn safe(_attr: TokenStream, item: TokenStream) -> TokenStream {
+    item
+}

tests/ui/rust-2024/unsafe-attributes/safe-proc-macro-attribute.rs

@@ -0,0 +1,22 @@
+//! Anti-regression test for `#[safe]` proc-macro attribute.
+
+//@ revisions: unknown_attr proc_macro_attr
+//@[proc_macro_attr] proc-macro: safe_attr.rs
+//@[proc_macro_attr] check-pass
+
+#![warn(unsafe_attr_outside_unsafe)]
+
+#[cfg(proc_macro_attr)]
+extern crate safe_attr;
+#[cfg(proc_macro_attr)]
+use safe_attr::safe;
+
+#[safe]
+//[unknown_attr]~^ ERROR cannot find attribute `safe` in this scope
+fn foo() {}
+
+#[safe(no_mangle)]
+//[unknown_attr]~^ ERROR cannot find attribute `safe` in this scope
+fn bar() {}
+
+fn main() {}

tests/ui/rust-2024/unsafe-attributes/safe-proc-macro-attribute.unknown_attr.stderr

@@ -0,0 +1,14 @@
+error: cannot find attribute `safe` in this scope
+  --> $DIR/safe-proc-macro-attribute.rs:18:3
+   |
+LL | #[safe(no_mangle)]
+   |   ^^^^
+
+error: cannot find attribute `safe` in this scope
+  --> $DIR/safe-proc-macro-attribute.rs:14:3
+   |
+LL | #[safe]
+   |   ^^^^
+
+error: aborting due to 2 previous errors
+