From eb3a8e5b81db989764a2e8a7568318c6bc8aa986 Mon Sep 17 00:00:00 2001 From: Jieyou Xu Date: Sat, 3 May 2025 21:57:19 +0800 Subject: [PATCH] Make attribute safety validation logic more obvious --- compiler/rustc_expand/src/config.rs | 2 +- compiler/rustc_parse/src/validate_attr.rs | 61 ++++++++++++++----- .../unsafe-attributes/auxiliary/safe_attr.rs | 7 +++ .../safe-proc-macro-attribute.rs | 22 +++++++ ...e-proc-macro-attribute.unknown_attr.stderr | 14 +++++ 5 files changed, 91 insertions(+), 15 deletions(-) create mode 100644 tests/ui/rust-2024/unsafe-attributes/auxiliary/safe_attr.rs create mode 100644 tests/ui/rust-2024/unsafe-attributes/safe-proc-macro-attribute.rs create mode 100644 tests/ui/rust-2024/unsafe-attributes/safe-proc-macro-attribute.unknown_attr.stderr diff --git a/compiler/rustc_expand/src/config.rs b/compiler/rustc_expand/src/config.rs index 02af26b015677..0994813ecb9c5 100644 --- a/compiler/rustc_expand/src/config.rs +++ b/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 { validate_attr::check_attribute_safety( &self.sess.psess, - AttributeSafety::Normal, + Some(AttributeSafety::Normal), &cfg_attr, ast::CRATE_NODE_ID, ); diff --git a/compiler/rustc_parse/src/validate_attr.rs b/compiler/rustc_parse/src/validate_attr.rs index aa29b24fe910c..378cbb846379d 100644 --- a/compiler/rustc_parse/src/validate_attr.rs +++ b/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, 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", + ); + } } } diff --git a/tests/ui/rust-2024/unsafe-attributes/auxiliary/safe_attr.rs b/tests/ui/rust-2024/unsafe-attributes/auxiliary/safe_attr.rs new file mode 100644 index 0000000000000..161b71b9737bc --- /dev/null +++ b/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 +} diff --git a/tests/ui/rust-2024/unsafe-attributes/safe-proc-macro-attribute.rs b/tests/ui/rust-2024/unsafe-attributes/safe-proc-macro-attribute.rs new file mode 100644 index 0000000000000..56b7001bdbf02 --- /dev/null +++ b/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() {} diff --git a/tests/ui/rust-2024/unsafe-attributes/safe-proc-macro-attribute.unknown_attr.stderr b/tests/ui/rust-2024/unsafe-attributes/safe-proc-macro-attribute.unknown_attr.stderr new file mode 100644 index 0000000000000..93c6d75e52ffc --- /dev/null +++ b/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 +