mbe: Implement unsafe attribute rules

compiler/rustc_attr_parsing/src/validate_attr.rs

@@ -207,10 +207,9 @@ pub fn check_attribute_safety(
             }
         }
 
-        // - 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)) => {
+        // - Normal builtin attribute
+        // - Writing `#[unsafe(..)]` is not permitted on normal builtin attributes
+        (Some(AttributeSafety::Normal), Safety::Unsafe(unsafe_span)) => {
             psess.dcx().emit_err(errors::InvalidAttrUnsafe {
                 span: unsafe_span,
                 name: attr_item.path.clone(),
@@ -224,9 +223,8 @@ pub fn check_attribute_safety(
         }
 
         // - Non-builtin attribute
-        // - No explicit `#[unsafe(..)]` written.
-        (None, Safety::Default) => {
-            // OK
+        (None, Safety::Unsafe(_) | Safety::Default) => {
+            // OK (not checked here)
         }
 
         (

compiler/rustc_expand/src/base.rs

@@ -10,7 +10,7 @@ use rustc_ast::attr::{AttributeExt, MarkedAttrs};
 use rustc_ast::token::MetaVarKind;
 use rustc_ast::tokenstream::TokenStream;
 use rustc_ast::visit::{AssocCtxt, Visitor};
-use rustc_ast::{self as ast, AttrVec, Attribute, HasAttrs, Item, NodeId, PatKind};
+use rustc_ast::{self as ast, AttrVec, Attribute, HasAttrs, Item, NodeId, PatKind, Safety};
 use rustc_data_structures::fx::{FxHashMap, FxIndexMap};
 use rustc_data_structures::sync;
 use rustc_errors::{BufferedEarlyLint, DiagCtxtHandle, ErrorGuaranteed, PResult};
@@ -345,6 +345,21 @@ pub trait AttrProcMacro {
         annotation: TokenStream,
         annotated: TokenStream,
     ) -> Result<TokenStream, ErrorGuaranteed>;
+
+    // Default implementation for safe attributes; override if the attribute can be unsafe.
+    fn expand_with_safety<'cx>(
+        &self,
+        ecx: &'cx mut ExtCtxt<'_>,
+        safety: Safety,
+        span: Span,
+        annotation: TokenStream,
+        annotated: TokenStream,
+    ) -> Result<TokenStream, ErrorGuaranteed> {
+        if let Safety::Unsafe(span) = safety {
+            ecx.dcx().span_err(span, "unnecessary `unsafe` on safe attribute");
+        }
+        self.expand(ecx, span, annotation, annotated)
+    }
 }
 
 impl<F> AttrProcMacro for F

compiler/rustc_expand/src/expand.rs

@@ -812,11 +812,12 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
                         _ => item.to_tokens(),
                     };
                     let attr_item = attr.get_normal_item();
+                    let safety = attr_item.unsafety;
                     if let AttrArgs::Eq { .. } = attr_item.args {
                         self.cx.dcx().emit_err(UnsupportedKeyValue { span });
                     }
                     let inner_tokens = attr_item.args.inner_tokens();
-                    match expander.expand(self.cx, span, inner_tokens, tokens) {
+                    match expander.expand_with_safety(self.cx, safety, span, inner_tokens, tokens) {
                         Ok(tok_result) => {
                             let fragment = self.parse_ast_fragment(
                                 tok_result,
@@ -840,6 +841,9 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
                         Err(guar) => return ExpandResult::Ready(fragment_kind.dummy(span, guar)),
                     }
                 } else if let SyntaxExtensionKind::LegacyAttr(expander) = ext {
+                    // `LegacyAttr` is only used for builtin attribute macros, which have their
+                    // safety checked by `check_builtin_meta_item`, so we don't need to check
+                    // `unsafety` here.
                     match validate_attr::parse_meta(&self.cx.sess.psess, &attr) {
                         Ok(meta) => {
                             let item_clone = macro_stats.then(|| item.clone());
@@ -882,6 +886,9 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
                         }
                     }
                 } else if let SyntaxExtensionKind::NonMacroAttr = ext {
+                    if let ast::Safety::Unsafe(span) = attr.get_normal_item().unsafety {
+                        self.cx.dcx().span_err(span, "unnecessary `unsafe` on safe attribute");
+                    }
                     // `-Zmacro-stats` ignores these because they don't do any real expansion.
                     self.cx.expanded_inert_attrs.mark(&attr);
                     item.visit_attrs(|attrs| attrs.insert(pos, attr));

compiler/rustc_expand/src/mbe/macro_rules.rs

@@ -8,7 +8,7 @@ use rustc_ast::token::NtPatKind::*;
 use rustc_ast::token::TokenKind::*;
 use rustc_ast::token::{self, Delimiter, NonterminalKind, Token, TokenKind};
 use rustc_ast::tokenstream::{self, DelimSpan, TokenStream};
-use rustc_ast::{self as ast, DUMMY_NODE_ID, NodeId};
+use rustc_ast::{self as ast, DUMMY_NODE_ID, NodeId, Safety};
 use rustc_ast_pretty::pprust;
 use rustc_data_structures::fx::{FxHashMap, FxIndexMap};
 use rustc_errors::{Applicability, Diag, ErrorGuaranteed, MultiSpan};
@@ -131,6 +131,7 @@ pub(super) enum MacroRule {
     Func { lhs: Vec<MatcherLoc>, lhs_span: Span, rhs: mbe::TokenTree },
     /// An attr rule, for use with `#[m]`
     Attr {
+        unsafe_rule: bool,
         args: Vec<MatcherLoc>,
         args_span: Span,
         body: Vec<MatcherLoc>,
@@ -247,8 +248,19 @@ impl TTMacroExpander for MacroRulesMacroExpander {
 
 impl AttrProcMacro for MacroRulesMacroExpander {
     fn expand(
+        &self,
+        _cx: &mut ExtCtxt<'_>,
+        _sp: Span,
+        _args: TokenStream,
+        _body: TokenStream,
+    ) -> Result<TokenStream, ErrorGuaranteed> {
+        unreachable!("`expand` called on `MacroRulesMacroExpander`, expected `expand_with_safety`")
+    }
+
+    fn expand_with_safety(
         &self,
         cx: &mut ExtCtxt<'_>,
+        safety: Safety,
         sp: Span,
         args: TokenStream,
         body: TokenStream,
@@ -260,6 +272,7 @@ impl AttrProcMacro for MacroRulesMacroExpander {
             self.node_id,
             self.name,
             self.transparency,
+            safety,
             args,
             body,
             &self.rules,
@@ -408,6 +421,7 @@ fn expand_macro_attr(
     node_id: NodeId,
     name: Ident,
     transparency: Transparency,
+    safety: Safety,
     args: TokenStream,
     body: TokenStream,
     rules: &[MacroRule],
@@ -429,13 +443,26 @@ fn expand_macro_attr(
     // Track nothing for the best performance.
     match try_match_macro_attr(psess, name, &args, &body, rules, &mut NoopTracker) {
         Ok((i, rule, named_matches)) => {
-            let MacroRule::Attr { rhs, .. } = rule else {
+            let MacroRule::Attr { rhs, unsafe_rule, .. } = rule else {
                 panic!("try_macro_match_attr returned non-attr rule");
             };
             let mbe::TokenTree::Delimited(rhs_span, _, rhs) = rhs else {
                 cx.dcx().span_bug(sp, "malformed macro rhs");
             };
 
+            match (safety, unsafe_rule) {
+                (Safety::Default, false) | (Safety::Unsafe(_), true) => {}
+                (Safety::Default, true) => {
+                    cx.dcx().span_err(sp, "unsafe attribute invocation requires `unsafe`");
+                }
+                (Safety::Unsafe(span), false) => {
+                    cx.dcx().span_err(span, "unnecessary `unsafe` on safe attribute invocation");
+                }
+                (Safety::Safe(span), _) => {
+                    cx.dcx().span_bug(span, "unexpected `safe` keyword");
+                }
+            }
+
             let id = cx.current_expansion.id;
             let tts = transcribe(psess, &named_matches, rhs, *rhs_span, transparency, id)
                 .map_err(|e| e.emit())?;
@@ -681,6 +708,11 @@ pub fn compile_declarative_macro(
     let mut rules = Vec::new();
 
     while p.token != token::Eof {
+        let unsafe_rule = p.eat_keyword_noexpect(kw::Unsafe);
+        let unsafe_keyword_span = p.prev_token.span;
+        if unsafe_rule && let Some(guar) = check_no_eof(sess, &p, "expected `attr`") {
+            return dummy_syn_ext(guar);
+        }
         let (args, is_derive) = if p.eat_keyword_noexpect(sym::attr) {
             kinds |= MacroKinds::ATTR;
             if !features.macro_attr() {
@@ -705,6 +737,10 @@ pub fn compile_declarative_macro(
                 feature_err(sess, sym::macro_derive, span, "`macro_rules!` derives are unstable")
                     .emit();
             }
+            if unsafe_rule {
+                sess.dcx()
+                    .span_err(unsafe_keyword_span, "`unsafe` is only supported on `attr` rules");
+            }
             if let Some(guar) = check_no_eof(sess, &p, "expected `()` after `derive`") {
                 return dummy_syn_ext(guar);
             }
@@ -730,6 +766,10 @@ pub fn compile_declarative_macro(
             (None, true)
         } else {
             kinds |= MacroKinds::BANG;
+            if unsafe_rule {
+                sess.dcx()
+                    .span_err(unsafe_keyword_span, "`unsafe` is only supported on `attr` rules");
+            }
             (None, false)
         };
         let lhs_tt = p.parse_token_tree();
@@ -741,10 +781,10 @@ pub fn compile_declarative_macro(
         if let Some(guar) = check_no_eof(sess, &p, "expected right-hand side of macro rule") {
             return dummy_syn_ext(guar);
         }
-        let rhs_tt = p.parse_token_tree();
-        let rhs_tt = parse_one_tt(rhs_tt, RulePart::Body, sess, node_id, features, edition);
-        check_emission(check_rhs(sess, &rhs_tt));
-        check_emission(check_meta_variables(&sess.psess, node_id, args.as_ref(), &lhs_tt, &rhs_tt));
+        let rhs = p.parse_token_tree();
+        let rhs = parse_one_tt(rhs, RulePart::Body, sess, node_id, features, edition);
+        check_emission(check_rhs(sess, &rhs));
+        check_emission(check_meta_variables(&sess.psess, node_id, args.as_ref(), &lhs_tt, &rhs));
         let lhs_span = lhs_tt.span();
         // Convert the lhs into `MatcherLoc` form, which is better for doing the
         // actual matching.
@@ -760,11 +800,11 @@ pub fn compile_declarative_macro(
             };
             let args = mbe::macro_parser::compute_locs(&delimited.tts);
             let body_span = lhs_span;
-            rules.push(MacroRule::Attr { args, args_span, body: lhs, body_span, rhs: rhs_tt });
+            rules.push(MacroRule::Attr { unsafe_rule, args, args_span, body: lhs, body_span, rhs });
         } else if is_derive {
-            rules.push(MacroRule::Derive { body: lhs, body_span: lhs_span, rhs: rhs_tt });
+            rules.push(MacroRule::Derive { body: lhs, body_span: lhs_span, rhs });
         } else {
-            rules.push(MacroRule::Func { lhs, lhs_span, rhs: rhs_tt });
+            rules.push(MacroRule::Func { lhs, lhs_span, rhs });
         }
         if p.token == token::Eof {
             break;

tests/ui/attributes/unsafe/double-unsafe-attributes.rs

@@ -1,7 +1,7 @@
 #[unsafe(unsafe(no_mangle))]
 //~^ ERROR expected identifier, found keyword `unsafe`
 //~| ERROR cannot find attribute `r#unsafe` in this scope
-//~| ERROR `r#unsafe` is not an unsafe attribute
+//~| ERROR unnecessary `unsafe`
 fn a() {}
 
 fn main() {}

tests/ui/attributes/unsafe/double-unsafe-attributes.stderr

@@ -9,13 +9,11 @@ help: escape `unsafe` to use it as an identifier
 LL | #[unsafe(r#unsafe(no_mangle))]
    |          ++
 
-error: `r#unsafe` is not an unsafe attribute
+error: unnecessary `unsafe` on safe attribute
   --> $DIR/double-unsafe-attributes.rs:1:3
    |
 LL | #[unsafe(unsafe(no_mangle))]
-   |   ^^^^^^ this is not an unsafe attribute
-   |
-   = note: extraneous unsafe is not allowed in attributes
+   |   ^^^^^^
 
 error: cannot find attribute `r#unsafe` in this scope
   --> $DIR/double-unsafe-attributes.rs:1:10

tests/ui/attributes/unsafe/unsafe-safe-attribute_diagnostic.rs

@@ -1,4 +1,4 @@
-#[unsafe(diagnostic::on_unimplemented( //~ ERROR: is not an unsafe attribute
+#[unsafe(diagnostic::on_unimplemented( //~ ERROR: unnecessary `unsafe`
     message = "testing",
 ))]
 trait Foo {}

tests/ui/attributes/unsafe/unsafe-safe-attribute_diagnostic.stderr

@@ -1,10 +1,8 @@
-error: `diagnostic::on_unimplemented` is not an unsafe attribute
+error: unnecessary `unsafe` on safe attribute
   --> $DIR/unsafe-safe-attribute_diagnostic.rs:1:3
    |
 LL | #[unsafe(diagnostic::on_unimplemented(
-   |   ^^^^^^ this is not an unsafe attribute
-   |
-   = note: extraneous unsafe is not allowed in attributes
+   |   ^^^^^^
 
 error: aborting due to 1 previous error
 

tests/ui/macros/macro-rules-attr-error.rs

@@ -50,3 +50,22 @@ macro_rules! forward_referenced_attr {
 macro_rules! cyclic_attr {
     attr() {} => {}
 }
+
+macro_rules! attr_with_safety {
+    unsafe attr() { struct RequiresUnsafe; } => {};
+    attr() { struct SafeInvocation; } => {};
+}
+
+#[attr_with_safety]
+struct SafeInvocation;
+
+//~v ERROR: unnecessary `unsafe` on safe attribute invocation
+#[unsafe(attr_with_safety)]
+struct SafeInvocation;
+
+//~v ERROR: unsafe attribute invocation requires `unsafe`
+#[attr_with_safety]
+struct RequiresUnsafe;
+
+#[unsafe(attr_with_safety)]
+struct RequiresUnsafe;

tests/ui/macros/macro-rules-attr-error.stderr

@@ -9,6 +9,18 @@ LL |     #[local_attr]
    |
    = note: this error originates in the attribute macro `local_attr` (in Nightly builds, run with -Z macro-backtrace for more info)
 
+error: unnecessary `unsafe` on safe attribute invocation
+  --> $DIR/macro-rules-attr-error.rs:63:3
+   |
+LL | #[unsafe(attr_with_safety)]
+   |   ^^^^^^
+
+error: unsafe attribute invocation requires `unsafe`
+  --> $DIR/macro-rules-attr-error.rs:67:1
+   |
+LL | #[attr_with_safety]
+   | ^^^^^^^^^^^^^^^^^^^
+
 error: cannot find macro `local_attr` in this scope
   --> $DIR/macro-rules-attr-error.rs:27:5
    |
@@ -59,5 +71,5 @@ note: a macro with the same name exists, but it appears later
 LL | macro_rules! cyclic_attr {
    |              ^^^^^^^^^^^
 
-error: aborting due to 6 previous errors
+error: aborting due to 8 previous errors
 

tests/ui/parser/macro/bad-macro-definition.rs

@@ -20,3 +20,6 @@ macro_rules! e { {} }
 
 macro_rules! f {}
 //~^ ERROR: macros must contain at least one rule
+
+macro_rules! g { unsafe {} => {} }
+//~^ ERROR: `unsafe` is only supported on `attr` rules

tests/ui/parser/macro/bad-macro-definition.stderr

@@ -52,5 +52,11 @@ error: macros must contain at least one rule
 LL | macro_rules! f {}
    | ^^^^^^^^^^^^^^^^^
 
-error: aborting due to 9 previous errors
+error: `unsafe` is only supported on `attr` rules
+  --> $DIR/bad-macro-definition.rs:24:18
+   |
+LL | macro_rules! g { unsafe {} => {} }
+   |                  ^^^^^^
+
+error: aborting due to 10 previous errors
 

tests/ui/parser/macro/macro-attr-bad.rs

@@ -13,6 +13,12 @@ macro_rules! attr_incomplete_3 { attr() {} }
 macro_rules! attr_incomplete_4 { attr() {} => }
 //~^ ERROR macro definition ended unexpectedly
 
+macro_rules! attr_incomplete_5 { unsafe }
+//~^ ERROR macro definition ended unexpectedly
+
+macro_rules! attr_incomplete_6 { unsafe attr }
+//~^ ERROR macro definition ended unexpectedly
+
 macro_rules! attr_noparens_1 { attr{} {} => {} }
 //~^ ERROR `attr` rule argument matchers require parentheses