Skip to content

Commit 49db8a5

Browse files
carbotaniumancarbotaniuman
committed
Add toggle for parse_meta_item unsafe parsing
This makes it possible for the `unsafe(...)` syntax to only be valid at the top level, and the `NestedMetaItem`s will automatically reject `unsafe(...)`.
1 parent d8bc876 commit 49db8a5

19 files changed

+226
-96
lines changed

‎compiler/rustc_builtin_macros/messages.ftl

-3
Original file line numberDiff line numberDiff line change
@@ -115,9 +115,6 @@ builtin_macros_derive_path_args_list = traits in `#[derive(...)]` don't accept a
115115
builtin_macros_derive_path_args_value = traits in `#[derive(...)]` don't accept values
116116
.suggestion = remove the value
117117
118-
builtin_macros_derive_unsafe_path = traits in `#[derive(...)]` don't accept `unsafe(...)`
119-
.suggestion = remove the `unsafe(...)`
120-
121118
builtin_macros_env_not_defined = environment variable `{$var}` not defined at compile time
122119
.cargo = Cargo sets build script variables at run time. Use `std::env::var({$var_expr})` instead
123120
.custom = use `std::env::var({$var_expr})` to read the variable at run time

‎compiler/rustc_builtin_macros/src/cfg.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use rustc_ast::token;
66
use rustc_ast::tokenstream::TokenStream;
77
use rustc_errors::PResult;
88
use rustc_expand::base::{DummyResult, ExpandResult, ExtCtxt, MacEager, MacroExpanderResult};
9+
use rustc_parse::parser::attr::AllowLeadingUnsafe;
910
use rustc_span::Span;
1011
use {rustc_ast as ast, rustc_attr as attr};
1112

@@ -42,7 +43,7 @@ fn parse_cfg<'a>(cx: &ExtCtxt<'a>, span: Span, tts: TokenStream) -> PResult<'a,
4243
return Err(cx.dcx().create_err(errors::RequiresCfgPattern { span }));
4344
}
4445

45-
let cfg = p.parse_meta_item()?;
46+
let cfg = p.parse_meta_item(AllowLeadingUnsafe::Yes)?;
4647

4748
let _ = p.eat(&token::Comma);
4849

‎compiler/rustc_builtin_macros/src/derive.rs

+1-12
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use rustc_ast as ast;
2-
use rustc_ast::{GenericParamKind, ItemKind, MetaItemKind, NestedMetaItem, Safety, StmtKind};
2+
use rustc_ast::{GenericParamKind, ItemKind, MetaItemKind, NestedMetaItem, StmtKind};
33
use rustc_expand::base::{
44
Annotatable, DeriveResolution, ExpandResult, ExtCtxt, Indeterminate, MultiItemModifier,
55
};
@@ -62,7 +62,6 @@ impl MultiItemModifier for Expander {
6262
// Reject `#[derive(Debug = "value", Debug(abc))]`, but recover the
6363
// paths.
6464
report_path_args(sess, meta);
65-
report_unsafe_args(sess, meta);
6665
meta.path.clone()
6766
})
6867
.map(|path| DeriveResolution {
@@ -162,13 +161,3 @@ fn report_path_args(sess: &Session, meta: &ast::MetaItem) {
162161
}
163162
}
164163
}
165-
166-
fn report_unsafe_args(sess: &Session, meta: &ast::MetaItem) {
167-
match meta.unsafety {
168-
Safety::Unsafe(span) => {
169-
sess.dcx().emit_err(errors::DeriveUnsafePath { span });
170-
}
171-
Safety::Default => {}
172-
Safety::Safe(_) => unreachable!(),
173-
}
174-
}

‎compiler/rustc_builtin_macros/src/errors.rs

-7
Original file line numberDiff line numberDiff line change
@@ -297,13 +297,6 @@ pub(crate) struct DerivePathArgsValue {
297297
pub(crate) span: Span,
298298
}
299299

300-
#[derive(Diagnostic)]
301-
#[diag(builtin_macros_derive_unsafe_path)]
302-
pub(crate) struct DeriveUnsafePath {
303-
#[primary_span]
304-
pub(crate) span: Span,
305-
}
306-
307300
#[derive(Diagnostic)]
308301
#[diag(builtin_macros_no_default_variant)]
309302
#[help]

‎compiler/rustc_expand/src/config.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@ use rustc_ast::tokenstream::{
88
use rustc_ast::{self as ast, AttrStyle, Attribute, HasAttrs, HasTokens, MetaItem, NodeId};
99
use rustc_attr as attr;
1010
use rustc_data_structures::flat_map_in_place::FlatMapInPlace;
11-
use rustc_feature::{AttributeSafety, Features, ACCEPTED_FEATURES, REMOVED_FEATURES, UNSTABLE_FEATURES};
11+
use rustc_feature::{
12+
AttributeSafety, Features, ACCEPTED_FEATURES, REMOVED_FEATURES, UNSTABLE_FEATURES,
13+
};
1214
use rustc_lint_defs::BuiltinLintDiag;
1315
use rustc_parse::validate_attr;
1416
use rustc_session::parse::feature_err;

‎compiler/rustc_interface/src/interface.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ use rustc_middle::ty;
1515
use rustc_middle::ty::CurrentGcx;
1616
use rustc_middle::util::Providers;
1717
use rustc_parse::new_parser_from_source_str;
18+
use rustc_parse::parser::attr::AllowLeadingUnsafe;
1819
use rustc_query_impl::QueryCtxt;
1920
use rustc_query_system::query::print_query_stack;
2021
use rustc_session::config::{self, Cfg, CheckCfg, ExpectedValues, Input, OutFileName};
@@ -67,7 +68,7 @@ pub(crate) fn parse_cfg(dcx: DiagCtxtHandle<'_>, cfgs: Vec<String>) -> Cfg {
6768
}
6869

6970
match new_parser_from_source_str(&psess, filename, s.to_string()) {
70-
Ok(mut parser) => match parser.parse_meta_item() {
71+
Ok(mut parser) => match parser.parse_meta_item(AllowLeadingUnsafe::No) {
7172
Ok(meta_item) if parser.token == token::Eof => {
7273
if meta_item.path.segments.len() != 1 {
7374
error!("argument key must be an identifier");
@@ -173,7 +174,7 @@ pub(crate) fn parse_check_cfg(dcx: DiagCtxtHandle<'_>, specs: Vec<String>) -> Ch
173174
}
174175
};
175176

176-
let meta_item = match parser.parse_meta_item() {
177+
let meta_item = match parser.parse_meta_item(AllowLeadingUnsafe::Yes) {
177178
Ok(meta_item) if parser.token == token::Eof => meta_item,
178179
Ok(..) => expected_error(),
179180
Err(err) => {

‎compiler/rustc_parse/messages.ftl

+1
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,7 @@ parse_inner_doc_comment_not_permitted = expected outer doc comment
365365
.sugg_change_inner_to_outer = to annotate the {$item}, change the doc comment from inner to outer style
366366
367367
parse_invalid_attr_unsafe = `{$name}` is not an unsafe attribute
368+
.label = this is not an unsafe attribute
368369
.suggestion = remove the `unsafe(...)`
369370
.note = extraneous unsafe is not allowed in attributes
370371

‎compiler/rustc_parse/src/errors.rs

+1
Original file line numberDiff line numberDiff line change
@@ -3188,6 +3188,7 @@ pub(crate) struct DotDotRangeAttribute {
31883188
#[note]
31893189
pub struct InvalidAttrUnsafe {
31903190
#[primary_span]
3191+
#[label]
31913192
pub span: Span,
31923193
pub name: Path,
31933194
}

‎compiler/rustc_parse/src/parser/attr.rs

+17-4
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,12 @@ enum OuterAttributeType {
3131
Attribute,
3232
}
3333

34+
#[derive(Clone, Copy, PartialEq, Eq)]
35+
pub enum AllowLeadingUnsafe {
36+
Yes,
37+
No,
38+
}
39+
3440
impl<'a> Parser<'a> {
3541
/// Parses attributes that appear before an item.
3642
pub(super) fn parse_outer_attributes(&mut self) -> PResult<'a, AttrWrapper> {
@@ -332,7 +338,7 @@ impl<'a> Parser<'a> {
332338

333339
/// Parses `cfg_attr(pred, attr_item_list)` where `attr_item_list` is comma-delimited.
334340
pub fn parse_cfg_attr(&mut self) -> PResult<'a, (ast::MetaItem, Vec<(ast::AttrItem, Span)>)> {
335-
let cfg_predicate = self.parse_meta_item()?;
341+
let cfg_predicate = self.parse_meta_item(AllowLeadingUnsafe::No)?;
336342
self.expect(&token::Comma)?;
337343

338344
// Presumably, the majority of the time there will only be one attr.
@@ -368,7 +374,10 @@ impl<'a> Parser<'a> {
368374
/// MetaItem = SimplePath ( '=' UNSUFFIXED_LIT | '(' MetaSeq? ')' )? ;
369375
/// MetaSeq = MetaItemInner (',' MetaItemInner)* ','? ;
370376
/// ```
371-
pub fn parse_meta_item(&mut self) -> PResult<'a, ast::MetaItem> {
377+
pub fn parse_meta_item(
378+
&mut self,
379+
unsafe_allowed: AllowLeadingUnsafe,
380+
) -> PResult<'a, ast::MetaItem> {
372381
// We can't use `maybe_whole` here because it would bump in the `None`
373382
// case, which we don't want.
374383
if let token::Interpolated(nt) = &self.token.kind
@@ -384,7 +393,11 @@ impl<'a> Parser<'a> {
384393
}
385394

386395
let lo = self.token.span;
387-
let is_unsafe = self.eat_keyword(kw::Unsafe);
396+
let is_unsafe = if unsafe_allowed == AllowLeadingUnsafe::Yes {
397+
self.eat_keyword(kw::Unsafe)
398+
} else {
399+
false
400+
};
388401
let unsafety = if is_unsafe {
389402
let unsafe_span = self.prev_token.span;
390403
self.psess.gated_spans.gate(sym::unsafe_attributes, unsafe_span);
@@ -427,7 +440,7 @@ impl<'a> Parser<'a> {
427440
Err(err) => err.cancel(), // we provide a better error below
428441
}
429442

430-
match self.parse_meta_item() {
443+
match self.parse_meta_item(AllowLeadingUnsafe::No) {
431444
Ok(mi) => return Ok(ast::NestedMetaItem::MetaItem(mi)),
432445
Err(err) => err.cancel(), // we provide a better error below
433446
}

‎compiler/rustc_parse/src/validate_attr.rs

+41-41
Original file line numberDiff line numberDiff line change
@@ -163,53 +163,53 @@ pub fn check_attribute_safety(
163163
safety: AttributeSafety,
164164
attr: &Attribute,
165165
) {
166-
if features.unsafe_attributes {
167-
let attr_item = attr.get_normal_item();
166+
if !features.unsafe_attributes {
167+
return;
168+
}
168169

169-
if safety == AttributeSafety::Unsafe {
170-
if let ast::Safety::Default = attr_item.unsafety {
171-
let path_span = attr_item.path.span;
170+
let attr_item = attr.get_normal_item();
172171

173-
// If the `attr_item`'s span is not from a macro, then just suggest
174-
// wrapping it in `unsafe(...)`. Otherwise, we suggest putting the
175-
// `unsafe(`, `)` right after and right before the opening and closing
176-
// square bracket respectively.
177-
let diag_span = if attr_item.span().can_be_used_for_suggestions() {
178-
attr_item.span()
179-
} else {
180-
attr.span
181-
.with_lo(attr.span.lo() + BytePos(2))
182-
.with_hi(attr.span.hi() - BytePos(1))
183-
};
172+
if safety == AttributeSafety::Unsafe {
173+
if let ast::Safety::Default = attr_item.unsafety {
174+
let path_span = attr_item.path.span;
184175

185-
if attr.span.at_least_rust_2024() {
186-
psess.dcx().emit_err(errors::UnsafeAttrOutsideUnsafe {
187-
span: path_span,
188-
suggestion: errors::UnsafeAttrOutsideUnsafeSuggestion {
189-
left: diag_span.shrink_to_lo(),
190-
right: diag_span.shrink_to_hi(),
191-
},
192-
});
193-
} else {
194-
psess.buffer_lint(
195-
UNSAFE_ATTR_OUTSIDE_UNSAFE,
196-
path_span,
197-
ast::CRATE_NODE_ID,
198-
BuiltinLintDiag::UnsafeAttrOutsideUnsafe {
199-
attribute_name_span: path_span,
200-
sugg_spans: (diag_span.shrink_to_lo(), diag_span.shrink_to_hi()),
201-
},
202-
);
203-
}
204-
}
205-
} else {
206-
if let Safety::Unsafe(unsafe_span) = attr_item.unsafety {
207-
psess.dcx().emit_err(errors::InvalidAttrUnsafe {
208-
span: unsafe_span,
209-
name: attr_item.path.clone(),
176+
// If the `attr_item`'s span is not from a macro, then just suggest
177+
// wrapping it in `unsafe(...)`. Otherwise, we suggest putting the
178+
// `unsafe(`, `)` right after and right before the opening and closing
179+
// square bracket respectively.
180+
let diag_span = if attr_item.span().can_be_used_for_suggestions() {
181+
attr_item.span()
182+
} else {
183+
attr.span.with_lo(attr.span.lo() + BytePos(2)).with_hi(attr.span.hi() - BytePos(1))
184+
};
185+
186+
if attr.span.at_least_rust_2024() {
187+
psess.dcx().emit_err(errors::UnsafeAttrOutsideUnsafe {
188+
span: path_span,
189+
suggestion: errors::UnsafeAttrOutsideUnsafeSuggestion {
190+
left: diag_span.shrink_to_lo(),
191+
right: diag_span.shrink_to_hi(),
192+
},
210193
});
194+
} else {
195+
psess.buffer_lint(
196+
UNSAFE_ATTR_OUTSIDE_UNSAFE,
197+
path_span,
198+
ast::CRATE_NODE_ID,
199+
BuiltinLintDiag::UnsafeAttrOutsideUnsafe {
200+
attribute_name_span: path_span,
201+
sugg_spans: (diag_span.shrink_to_lo(), diag_span.shrink_to_hi()),
202+
},
203+
);
211204
}
212205
}
206+
} else {
207+
if let Safety::Unsafe(unsafe_span) = attr_item.unsafety {
208+
psess.dcx().emit_err(errors::InvalidAttrUnsafe {
209+
span: unsafe_span,
210+
name: attr_item.path.clone(),
211+
});
212+
}
213213
}
214214
}
215215

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

+7-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
#![feature(unsafe_attributes)]
22

3-
#[derive(unsafe(Debug))] //~ ERROR: traits in `#[derive(...)]` don't accept `unsafe(...)`
3+
#[derive(unsafe(Debug))]
4+
//~^ ERROR: expected identifier, found keyword `unsafe`
5+
//~| ERROR: traits in `#[derive(...)]` don't accept arguments
6+
//~| ERROR: expected identifier, found keyword `unsafe`
7+
//~| ERROR: expected identifier, found keyword `unsafe`
8+
//~| ERROR: cannot find derive macro `r#unsafe` in this scope
9+
//~| ERROR: cannot find derive macro `r#unsafe` in this scope
410
struct Foo;
511

612
#[unsafe(derive(Debug))] //~ ERROR: is not an unsafe attribute

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

+54-5
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,65 @@
1-
error: traits in `#[derive(...)]` don't accept `unsafe(...)`
1+
error: expected identifier, found keyword `unsafe`
22
--> $DIR/derive-unsafe-attributes.rs:3:10
33
|
44
LL | #[derive(unsafe(Debug))]
5-
| ^^^^^^
5+
| ^^^^^^ expected identifier, found keyword
6+
|
7+
help: escape `unsafe` to use it as an identifier
8+
|
9+
LL | #[derive(r#unsafe(Debug))]
10+
| ++
11+
12+
error: traits in `#[derive(...)]` don't accept arguments
13+
--> $DIR/derive-unsafe-attributes.rs:3:16
14+
|
15+
LL | #[derive(unsafe(Debug))]
16+
| ^^^^^^^ help: remove the arguments
617

718
error: `derive` is not an unsafe attribute
8-
--> $DIR/derive-unsafe-attributes.rs:6:3
19+
--> $DIR/derive-unsafe-attributes.rs:12:3
920
|
1021
LL | #[unsafe(derive(Debug))]
11-
| ^^^^^^
22+
| ^^^^^^ this is not an unsafe attribute
1223
|
1324
= note: extraneous unsafe is not allowed in attributes
1425

15-
error: aborting due to 2 previous errors
26+
error: expected identifier, found keyword `unsafe`
27+
--> $DIR/derive-unsafe-attributes.rs:3:10
28+
|
29+
LL | #[derive(unsafe(Debug))]
30+
| ^^^^^^ expected identifier, found keyword
31+
|
32+
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
33+
help: escape `unsafe` to use it as an identifier
34+
|
35+
LL | #[derive(r#unsafe(Debug))]
36+
| ++
37+
38+
error: expected identifier, found keyword `unsafe`
39+
--> $DIR/derive-unsafe-attributes.rs:3:10
40+
|
41+
LL | #[derive(unsafe(Debug))]
42+
| ^^^^^^ expected identifier, found keyword
43+
|
44+
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
45+
help: escape `unsafe` to use it as an identifier
46+
|
47+
LL | #[derive(r#unsafe(Debug))]
48+
| ++
49+
50+
error: cannot find derive macro `r#unsafe` in this scope
51+
--> $DIR/derive-unsafe-attributes.rs:3:10
52+
|
53+
LL | #[derive(unsafe(Debug))]
54+
| ^^^^^^
55+
56+
error: cannot find derive macro `r#unsafe` in this scope
57+
--> $DIR/derive-unsafe-attributes.rs:3:10
58+
|
59+
LL | #[derive(unsafe(Debug))]
60+
| ^^^^^^
61+
|
62+
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
63+
64+
error: aborting due to 7 previous errors
1665

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ error: `r#unsafe` is not an unsafe attribute
1313
--> $DIR/double-unsafe-attributes.rs:3:3
1414
|
1515
LL | #[unsafe(unsafe(no_mangle))]
16-
| ^^^^^^
16+
| ^^^^^^ this is not an unsafe attribute
1717
|
1818
= note: extraneous unsafe is not allowed in attributes
1919

0 commit comments

Comments
 (0)