mbe: Support unsafe attribute rules

Author: joshtriplett

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

@@ -813,11 +813,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,
@@ -883,6 +884,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};
@@ -132,6 +132,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>,
@@ -248,8 +249,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,
@@ -261,6 +273,7 @@ impl AttrProcMacro for MacroRulesMacroExpander {
             self.node_id,
             self.name,
             self.transparency,
+            safety,
             args,
             body,
             &self.rules,
@@ -409,6 +422,7 @@ fn expand_macro_attr(
     node_id: NodeId,
     name: Ident,
     transparency: Transparency,
+    safety: Safety,
     args: TokenStream,
     body: TokenStream,
     rules: &[MacroRule],
@@ -430,13 +444,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())?;
@@ -682,6 +709,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() {
@@ -706,6 +738,10 @@ pub fn compile_declarative_macro(
                 feature_err(sess, sym::macro_attr, 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);
             }
@@ -731,6 +767,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();
@@ -761,7 +801,7 @@ 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 });
+            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 });
         } else {

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