Skip to content

Commit ac7beab

Browse files
matthiaskrgrmatthiaskrgr
authored
Rollup merge of rust-lang#146535 - joshtriplett:mbe-unsafe-attr, r=petrochenkov
mbe: Implement `unsafe` attribute rules This implements `unsafe attr` rules for declarative `macro_rules!` attributes, as specified in [RFC 3697](rust-lang/rfcs#3697). An invocation of an attribute that uses an `unsafe attr` rule requires the `unsafe(attr(...))` syntax. An invocation of an attribute that uses an ordinary `attr` rule must *not* use the `unsafe(attr(...))` syntax. `unsafe` is only supported on an `attr` rule, not any other kind of `macro_rules!` rule. Tracking issue for `macro_rules!` attributes: rust-lang#143547
2 parents 92aac1b + 4fc0a0d commit ac7beab

16 files changed

+161
-38
lines changed

‎compiler/rustc_attr_parsing/src/validate_attr.rs‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -207,10 +207,9 @@ pub fn check_attribute_safety(
207207
}
208208
}
209209

210-
// - Normal builtin attribute, or any non-builtin attribute
211-
// - All non-builtin attributes are currently considered safe; writing `#[unsafe(..)]` is
212-
// not permitted on non-builtin attributes or normal builtin attributes
213-
(Some(AttributeSafety::Normal) | None, Safety::Unsafe(unsafe_span)) => {
210+
// - Normal builtin attribute
211+
// - Writing `#[unsafe(..)]` is not permitted on normal builtin attributes
212+
(Some(AttributeSafety::Normal), Safety::Unsafe(unsafe_span)) => {
214213
psess.dcx().emit_err(errors::InvalidAttrUnsafe {
215214
span: unsafe_span,
216215
name: attr_item.path.clone(),
@@ -224,9 +223,8 @@ pub fn check_attribute_safety(
224223
}
225224

226225
// - Non-builtin attribute
227-
// - No explicit `#[unsafe(..)]` written.
228-
(None, Safety::Default) => {
229-
// OK
226+
(None, Safety::Unsafe(_) | Safety::Default) => {
227+
// OK (not checked here)
230228
}
231229

232230
(

‎compiler/rustc_expand/src/base.rs‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use rustc_ast::attr::{AttributeExt, MarkedAttrs};
1010
use rustc_ast::token::MetaVarKind;
1111
use rustc_ast::tokenstream::TokenStream;
1212
use rustc_ast::visit::{AssocCtxt, Visitor};
13-
use rustc_ast::{self as ast, AttrVec, Attribute, HasAttrs, Item, NodeId, PatKind};
13+
use rustc_ast::{self as ast, AttrVec, Attribute, HasAttrs, Item, NodeId, PatKind, Safety};
1414
use rustc_data_structures::fx::{FxHashMap, FxIndexMap};
1515
use rustc_data_structures::sync;
1616
use rustc_errors::{BufferedEarlyLint, DiagCtxtHandle, ErrorGuaranteed, PResult};
@@ -345,6 +345,21 @@ pub trait AttrProcMacro {
345345
annotation: TokenStream,
346346
annotated: TokenStream,
347347
) -> Result<TokenStream, ErrorGuaranteed>;
348+
349+
// Default implementation for safe attributes; override if the attribute can be unsafe.
350+
fn expand_with_safety<'cx>(
351+
&self,
352+
ecx: &'cx mut ExtCtxt<'_>,
353+
safety: Safety,
354+
span: Span,
355+
annotation: TokenStream,
356+
annotated: TokenStream,
357+
) -> Result<TokenStream, ErrorGuaranteed> {
358+
if let Safety::Unsafe(span) = safety {
359+
ecx.dcx().span_err(span, "unnecessary `unsafe` on safe attribute");
360+
}
361+
self.expand(ecx, span, annotation, annotated)
362+
}
348363
}
349364

350365
impl<F> AttrProcMacro for F

‎compiler/rustc_expand/src/expand.rs‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -812,11 +812,12 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
812812
_ => item.to_tokens(),
813813
};
814814
let attr_item = attr.get_normal_item();
815+
let safety = attr_item.unsafety;
815816
if let AttrArgs::Eq { .. } = attr_item.args {
816817
self.cx.dcx().emit_err(UnsupportedKeyValue { span });
817818
}
818819
let inner_tokens = attr_item.args.inner_tokens();
819-
match expander.expand(self.cx, span, inner_tokens, tokens) {
820+
match expander.expand_with_safety(self.cx, safety, span, inner_tokens, tokens) {
820821
Ok(tok_result) => {
821822
let fragment = self.parse_ast_fragment(
822823
tok_result,
@@ -840,6 +841,9 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
840841
Err(guar) => return ExpandResult::Ready(fragment_kind.dummy(span, guar)),
841842
}
842843
} else if let SyntaxExtensionKind::LegacyAttr(expander) = ext {
844+
// `LegacyAttr` is only used for builtin attribute macros, which have their
845+
// safety checked by `check_builtin_meta_item`, so we don't need to check
846+
// `unsafety` here.
843847
match validate_attr::parse_meta(&self.cx.sess.psess, &attr) {
844848
Ok(meta) => {
845849
let item_clone = macro_stats.then(|| item.clone());
@@ -882,6 +886,9 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
882886
}
883887
}
884888
} else if let SyntaxExtensionKind::NonMacroAttr = ext {
889+
if let ast::Safety::Unsafe(span) = attr.get_normal_item().unsafety {
890+
self.cx.dcx().span_err(span, "unnecessary `unsafe` on safe attribute");
891+
}
885892
// `-Zmacro-stats` ignores these because they don't do any real expansion.
886893
self.cx.expanded_inert_attrs.mark(&attr);
887894
item.visit_attrs(|attrs| attrs.insert(pos, attr));

‎compiler/rustc_expand/src/mbe/macro_rules.rs‎

Lines changed: 49 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use rustc_ast::token::NtPatKind::*;
88
use rustc_ast::token::TokenKind::*;
99
use rustc_ast::token::{self, Delimiter, NonterminalKind, Token, TokenKind};
1010
use rustc_ast::tokenstream::{self, DelimSpan, TokenStream};
11-
use rustc_ast::{self as ast, DUMMY_NODE_ID, NodeId};
11+
use rustc_ast::{self as ast, DUMMY_NODE_ID, NodeId, Safety};
1212
use rustc_ast_pretty::pprust;
1313
use rustc_data_structures::fx::{FxHashMap, FxIndexMap};
1414
use rustc_errors::{Applicability, Diag, ErrorGuaranteed, MultiSpan};
@@ -131,6 +131,7 @@ pub(super) enum MacroRule {
131131
Func { lhs: Vec<MatcherLoc>, lhs_span: Span, rhs: mbe::TokenTree },
132132
/// An attr rule, for use with `#[m]`
133133
Attr {
134+
unsafe_rule: bool,
134135
args: Vec<MatcherLoc>,
135136
args_span: Span,
136137
body: Vec<MatcherLoc>,
@@ -247,8 +248,19 @@ impl TTMacroExpander for MacroRulesMacroExpander {
247248

248249
impl AttrProcMacro for MacroRulesMacroExpander {
249250
fn expand(
251+
&self,
252+
_cx: &mut ExtCtxt<'_>,
253+
_sp: Span,
254+
_args: TokenStream,
255+
_body: TokenStream,
256+
) -> Result<TokenStream, ErrorGuaranteed> {
257+
unreachable!("`expand` called on `MacroRulesMacroExpander`, expected `expand_with_safety`")
258+
}
259+
260+
fn expand_with_safety(
250261
&self,
251262
cx: &mut ExtCtxt<'_>,
263+
safety: Safety,
252264
sp: Span,
253265
args: TokenStream,
254266
body: TokenStream,
@@ -260,6 +272,7 @@ impl AttrProcMacro for MacroRulesMacroExpander {
260272
self.node_id,
261273
self.name,
262274
self.transparency,
275+
safety,
263276
args,
264277
body,
265278
&self.rules,
@@ -408,6 +421,7 @@ fn expand_macro_attr(
408421
node_id: NodeId,
409422
name: Ident,
410423
transparency: Transparency,
424+
safety: Safety,
411425
args: TokenStream,
412426
body: TokenStream,
413427
rules: &[MacroRule],
@@ -429,13 +443,26 @@ fn expand_macro_attr(
429443
// Track nothing for the best performance.
430444
match try_match_macro_attr(psess, name, &args, &body, rules, &mut NoopTracker) {
431445
Ok((i, rule, named_matches)) => {
432-
let MacroRule::Attr { rhs, .. } = rule else {
446+
let MacroRule::Attr { rhs, unsafe_rule, .. } = rule else {
433447
panic!("try_macro_match_attr returned non-attr rule");
434448
};
435449
let mbe::TokenTree::Delimited(rhs_span, _, rhs) = rhs else {
436450
cx.dcx().span_bug(sp, "malformed macro rhs");
437451
};
438452

453+
match (safety, unsafe_rule) {
454+
(Safety::Default, false) | (Safety::Unsafe(_), true) => {}
455+
(Safety::Default, true) => {
456+
cx.dcx().span_err(sp, "unsafe attribute invocation requires `unsafe`");
457+
}
458+
(Safety::Unsafe(span), false) => {
459+
cx.dcx().span_err(span, "unnecessary `unsafe` on safe attribute invocation");
460+
}
461+
(Safety::Safe(span), _) => {
462+
cx.dcx().span_bug(span, "unexpected `safe` keyword");
463+
}
464+
}
465+
439466
let id = cx.current_expansion.id;
440467
let tts = transcribe(psess, &named_matches, rhs, *rhs_span, transparency, id)
441468
.map_err(|e| e.emit())?;
@@ -681,6 +708,11 @@ pub fn compile_declarative_macro(
681708
let mut rules = Vec::new();
682709

683710
while p.token != token::Eof {
711+
let unsafe_rule = p.eat_keyword_noexpect(kw::Unsafe);
712+
let unsafe_keyword_span = p.prev_token.span;
713+
if unsafe_rule && let Some(guar) = check_no_eof(sess, &p, "expected `attr`") {
714+
return dummy_syn_ext(guar);
715+
}
684716
let (args, is_derive) = if p.eat_keyword_noexpect(sym::attr) {
685717
kinds |= MacroKinds::ATTR;
686718
if !features.macro_attr() {
@@ -705,6 +737,10 @@ pub fn compile_declarative_macro(
705737
feature_err(sess, sym::macro_derive, span, "`macro_rules!` derives are unstable")
706738
.emit();
707739
}
740+
if unsafe_rule {
741+
sess.dcx()
742+
.span_err(unsafe_keyword_span, "`unsafe` is only supported on `attr` rules");
743+
}
708744
if let Some(guar) = check_no_eof(sess, &p, "expected `()` after `derive`") {
709745
return dummy_syn_ext(guar);
710746
}
@@ -730,6 +766,10 @@ pub fn compile_declarative_macro(
730766
(None, true)
731767
} else {
732768
kinds |= MacroKinds::BANG;
769+
if unsafe_rule {
770+
sess.dcx()
771+
.span_err(unsafe_keyword_span, "`unsafe` is only supported on `attr` rules");
772+
}
733773
(None, false)
734774
};
735775
let lhs_tt = p.parse_token_tree();
@@ -741,10 +781,10 @@ pub fn compile_declarative_macro(
741781
if let Some(guar) = check_no_eof(sess, &p, "expected right-hand side of macro rule") {
742782
return dummy_syn_ext(guar);
743783
}
744-
let rhs_tt = p.parse_token_tree();
745-
let rhs_tt = parse_one_tt(rhs_tt, RulePart::Body, sess, node_id, features, edition);
746-
check_emission(check_rhs(sess, &rhs_tt));
747-
check_emission(check_meta_variables(&sess.psess, node_id, args.as_ref(), &lhs_tt, &rhs_tt));
784+
let rhs = p.parse_token_tree();
785+
let rhs = parse_one_tt(rhs, RulePart::Body, sess, node_id, features, edition);
786+
check_emission(check_rhs(sess, &rhs));
787+
check_emission(check_meta_variables(&sess.psess, node_id, args.as_ref(), &lhs_tt, &rhs));
748788
let lhs_span = lhs_tt.span();
749789
// Convert the lhs into `MatcherLoc` form, which is better for doing the
750790
// actual matching.
@@ -760,11 +800,11 @@ pub fn compile_declarative_macro(
760800
};
761801
let args = mbe::macro_parser::compute_locs(&delimited.tts);
762802
let body_span = lhs_span;
763-
rules.push(MacroRule::Attr { args, args_span, body: lhs, body_span, rhs: rhs_tt });
803+
rules.push(MacroRule::Attr { unsafe_rule, args, args_span, body: lhs, body_span, rhs });
764804
} else if is_derive {
765-
rules.push(MacroRule::Derive { body: lhs, body_span: lhs_span, rhs: rhs_tt });
805+
rules.push(MacroRule::Derive { body: lhs, body_span: lhs_span, rhs });
766806
} else {
767-
rules.push(MacroRule::Func { lhs, lhs_span, rhs: rhs_tt });
807+
rules.push(MacroRule::Func { lhs, lhs_span, rhs });
768808
}
769809
if p.token == token::Eof {
770810
break;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
#[unsafe(unsafe(no_mangle))]
22
//~^ ERROR expected identifier, found keyword `unsafe`
33
//~| ERROR cannot find attribute `r#unsafe` in this scope
4-
//~| ERROR `r#unsafe` is not an unsafe attribute
4+
//~| ERROR unnecessary `unsafe`
55
fn a() {}
66

77
fn main() {}

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,11 @@ help: escape `unsafe` to use it as an identifier
99
LL | #[unsafe(r#unsafe(no_mangle))]
1010
| ++
1111

12-
error: `r#unsafe` is not an unsafe attribute
12+
error: unnecessary `unsafe` on safe attribute
1313
--> $DIR/double-unsafe-attributes.rs:1:3
1414
|
1515
LL | #[unsafe(unsafe(no_mangle))]
16-
| ^^^^^^ this is not an unsafe attribute
17-
|
18-
= note: extraneous unsafe is not allowed in attributes
16+
| ^^^^^^
1917

2018
error: cannot find attribute `r#unsafe` in this scope
2119
--> $DIR/double-unsafe-attributes.rs:1:10

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
#[unsafe(diagnostic::on_unimplemented( //~ ERROR: is not an unsafe attribute
1+
#[unsafe(diagnostic::on_unimplemented( //~ ERROR: unnecessary `unsafe`
22
message = "testing",
33
))]
44
trait Foo {}

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
1-
error: `diagnostic::on_unimplemented` is not an unsafe attribute
1+
error: unnecessary `unsafe` on safe attribute
22
--> $DIR/unsafe-safe-attribute_diagnostic.rs:1:3
33
|
44
LL | #[unsafe(diagnostic::on_unimplemented(
5-
| ^^^^^^ this is not an unsafe attribute
6-
|
7-
= note: extraneous unsafe is not allowed in attributes
5+
| ^^^^^^
86

97
error: aborting due to 1 previous error
108

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,3 +50,22 @@ macro_rules! forward_referenced_attr {
5050
macro_rules! cyclic_attr {
5151
attr() {} => {}
5252
}
53+
54+
macro_rules! attr_with_safety {
55+
unsafe attr() { struct RequiresUnsafe; } => {};
56+
attr() { struct SafeInvocation; } => {};
57+
}
58+
59+
#[attr_with_safety]
60+
struct SafeInvocation;
61+
62+
//~v ERROR: unnecessary `unsafe` on safe attribute invocation
63+
#[unsafe(attr_with_safety)]
64+
struct SafeInvocation;
65+
66+
//~v ERROR: unsafe attribute invocation requires `unsafe`
67+
#[attr_with_safety]
68+
struct RequiresUnsafe;
69+
70+
#[unsafe(attr_with_safety)]
71+
struct RequiresUnsafe;

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,18 @@ LL | #[local_attr]
99
|
1010
= note: this error originates in the attribute macro `local_attr` (in Nightly builds, run with -Z macro-backtrace for more info)
1111

12+
error: unnecessary `unsafe` on safe attribute invocation
13+
--> $DIR/macro-rules-attr-error.rs:63:3
14+
|
15+
LL | #[unsafe(attr_with_safety)]
16+
| ^^^^^^
17+
18+
error: unsafe attribute invocation requires `unsafe`
19+
--> $DIR/macro-rules-attr-error.rs:67:1
20+
|
21+
LL | #[attr_with_safety]
22+
| ^^^^^^^^^^^^^^^^^^^
23+
1224
error: cannot find macro `local_attr` in this scope
1325
--> $DIR/macro-rules-attr-error.rs:27:5
1426
|
@@ -59,5 +71,5 @@ note: a macro with the same name exists, but it appears later
5971
LL | macro_rules! cyclic_attr {
6072
| ^^^^^^^^^^^
6173

62-
error: aborting due to 6 previous errors
74+
error: aborting due to 8 previous errors
6375

0 commit comments

Comments
 (0)