Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More unsafe attr verification #127543

Merged
merged 2 commits into from
Aug 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions compiler/rustc_builtin_macros/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,6 @@ builtin_macros_derive_path_args_list = traits in `#[derive(...)]` don't accept a
builtin_macros_derive_path_args_value = traits in `#[derive(...)]` don't accept values
.suggestion = remove the value
builtin_macros_derive_unsafe_path = traits in `#[derive(...)]` don't accept `unsafe(...)`
.suggestion = remove the `unsafe(...)`
builtin_macros_env_not_defined = environment variable `{$var}` not defined at compile time
.cargo = Cargo sets build script variables at run time. Use `std::env::var({$var_expr})` instead
.custom = use `std::env::var({$var_expr})` to read the variable at run time
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_builtin_macros/src/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use rustc_ast::token;
use rustc_ast::tokenstream::TokenStream;
use rustc_errors::PResult;
use rustc_expand::base::{DummyResult, ExpandResult, ExtCtxt, MacEager, MacroExpanderResult};
use rustc_parse::parser::attr::AllowLeadingUnsafe;
use rustc_span::Span;
use {rustc_ast as ast, rustc_attr as attr};

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

let cfg = p.parse_meta_item()?;
let cfg = p.parse_meta_item(AllowLeadingUnsafe::Yes)?;

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

Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_builtin_macros/src/cfg_accessible.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,13 @@ impl MultiItemModifier for Expander {
) -> ExpandResult<Vec<Annotatable>, Annotatable> {
let template = AttributeTemplate { list: Some("path"), ..Default::default() };
validate_attr::check_builtin_meta_item(
&ecx.ecfg.features,
&ecx.sess.psess,
meta_item,
ast::AttrStyle::Outer,
sym::cfg_accessible,
template,
true,
);

let Some(path) = validate_input(ecx, meta_item) else {
Expand Down
15 changes: 3 additions & 12 deletions compiler/rustc_builtin_macros/src/derive.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use rustc_ast as ast;
use rustc_ast::{GenericParamKind, ItemKind, MetaItemKind, NestedMetaItem, Safety, StmtKind};
use rustc_ast::{GenericParamKind, ItemKind, MetaItemKind, NestedMetaItem, StmtKind};
use rustc_expand::base::{
Annotatable, DeriveResolution, ExpandResult, ExtCtxt, Indeterminate, MultiItemModifier,
};
Expand Down Expand Up @@ -38,11 +38,13 @@ impl MultiItemModifier for Expander {
let template =
AttributeTemplate { list: Some("Trait1, Trait2, ..."), ..Default::default() };
validate_attr::check_builtin_meta_item(
features,
&sess.psess,
meta_item,
ast::AttrStyle::Outer,
sym::derive,
template,
true,
);

let mut resolutions = match &meta_item.kind {
Expand All @@ -60,7 +62,6 @@ impl MultiItemModifier for Expander {
// Reject `#[derive(Debug = "value", Debug(abc))]`, but recover the
// paths.
report_path_args(sess, meta);
report_unsafe_args(sess, meta);
meta.path.clone()
})
.map(|path| DeriveResolution {
Expand Down Expand Up @@ -160,13 +161,3 @@ fn report_path_args(sess: &Session, meta: &ast::MetaItem) {
}
}
}

fn report_unsafe_args(sess: &Session, meta: &ast::MetaItem) {
match meta.unsafety {
Safety::Unsafe(span) => {
sess.dcx().emit_err(errors::DeriveUnsafePath { span });
}
Safety::Default => {}
Safety::Safe(_) => unreachable!(),
}
}
7 changes: 0 additions & 7 deletions compiler/rustc_builtin_macros/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,13 +297,6 @@ pub(crate) struct DerivePathArgsValue {
pub(crate) span: Span,
}

#[derive(Diagnostic)]
#[diag(builtin_macros_derive_unsafe_path)]
pub(crate) struct DeriveUnsafePath {
#[primary_span]
pub(crate) span: Span,
}

#[derive(Diagnostic)]
#[diag(builtin_macros_no_default_variant)]
#[help]
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_builtin_macros/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@ pub(crate) fn check_builtin_macro_attribute(ecx: &ExtCtxt<'_>, meta_item: &MetaI
// All the built-in macro attributes are "words" at the moment.
let template = AttributeTemplate { word: true, ..Default::default() };
validate_attr::check_builtin_meta_item(
&ecx.ecfg.features,
&ecx.sess.psess,
meta_item,
AttrStyle::Outer,
name,
template,
true,
);
}

Expand Down
18 changes: 17 additions & 1 deletion compiler/rustc_expand/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ use rustc_ast::tokenstream::{
use rustc_ast::{self as ast, AttrStyle, Attribute, HasAttrs, HasTokens, MetaItem, NodeId};
use rustc_attr as attr;
use rustc_data_structures::flat_map_in_place::FlatMapInPlace;
use rustc_feature::{Features, ACCEPTED_FEATURES, REMOVED_FEATURES, UNSTABLE_FEATURES};
use rustc_feature::{
AttributeSafety, Features, ACCEPTED_FEATURES, REMOVED_FEATURES, UNSTABLE_FEATURES,
};
use rustc_lint_defs::BuiltinLintDiag;
use rustc_parse::validate_attr;
use rustc_session::parse::feature_err;
Expand Down Expand Up @@ -263,6 +265,13 @@ impl<'a> StripUnconfigured<'a> {
/// is in the original source file. Gives a compiler error if the syntax of
/// the attribute is incorrect.
pub(crate) fn expand_cfg_attr(&self, cfg_attr: &Attribute, recursive: bool) -> Vec<Attribute> {
validate_attr::check_attribute_safety(
self.features.unwrap_or(&Features::default()),
&self.sess.psess,
AttributeSafety::Normal,
&cfg_attr,
);

let Some((cfg_predicate, expanded_attrs)) =
rustc_parse::parse_cfg_attr(cfg_attr, &self.sess.psess)
else {
Expand Down Expand Up @@ -385,6 +394,13 @@ impl<'a> StripUnconfigured<'a> {
return (true, None);
}
};

validate_attr::deny_builtin_meta_unsafety(
self.features.unwrap_or(&Features::default()),
&self.sess.psess,
&meta_item,
);

(
parse_cfg(&meta_item, self.sess).map_or(true, |meta_item| {
attr::cfg_matches(meta_item, &self.sess, self.lint_node_id, self.features)
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_interface/src/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use rustc_middle::ty;
use rustc_middle::ty::CurrentGcx;
use rustc_middle::util::Providers;
use rustc_parse::new_parser_from_source_str;
use rustc_parse::parser::attr::AllowLeadingUnsafe;
use rustc_query_impl::QueryCtxt;
use rustc_query_system::query::print_query_stack;
use rustc_session::config::{self, Cfg, CheckCfg, ExpectedValues, Input, OutFileName};
Expand Down Expand Up @@ -67,7 +68,7 @@ pub(crate) fn parse_cfg(dcx: DiagCtxtHandle<'_>, cfgs: Vec<String>) -> Cfg {
}

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

let meta_item = match parser.parse_meta_item() {
let meta_item = match parser.parse_meta_item(AllowLeadingUnsafe::Yes) {
Ok(meta_item) if parser.token == token::Eof => meta_item,
Ok(..) => expected_error(),
Err(err) => {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_parse/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,7 @@ parse_inner_doc_comment_not_permitted = expected outer doc comment
.sugg_change_inner_to_outer = to annotate the {$item}, change the doc comment from inner to outer style
parse_invalid_attr_unsafe = `{$name}` is not an unsafe attribute
.label = this is not an unsafe attribute
.suggestion = remove the `unsafe(...)`
.note = extraneous unsafe is not allowed in attributes
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_parse/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3188,6 +3188,7 @@ pub(crate) struct DotDotRangeAttribute {
#[note]
pub struct InvalidAttrUnsafe {
#[primary_span]
#[label]
pub span: Span,
pub name: Path,
}
Expand Down
21 changes: 17 additions & 4 deletions compiler/rustc_parse/src/parser/attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@ enum OuterAttributeType {
Attribute,
}

#[derive(Clone, Copy, PartialEq, Eq)]
pub enum AllowLeadingUnsafe {
Yes,
No,
}

impl<'a> Parser<'a> {
/// Parses attributes that appear before an item.
pub(super) fn parse_outer_attributes(&mut self) -> PResult<'a, AttrWrapper> {
Expand Down Expand Up @@ -332,7 +338,7 @@ impl<'a> Parser<'a> {

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

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

let lo = self.token.span;
let is_unsafe = self.eat_keyword(kw::Unsafe);
let is_unsafe = if unsafe_allowed == AllowLeadingUnsafe::Yes {
self.eat_keyword(kw::Unsafe)
} else {
false
};
let unsafety = if is_unsafe {
let unsafe_span = self.prev_token.span;
self.psess.gated_spans.gate(sym::unsafe_attributes, unsafe_span);
Expand Down Expand Up @@ -427,7 +440,7 @@ impl<'a> Parser<'a> {
Err(err) => err.cancel(), // we provide a better error below
}

match self.parse_meta_item() {
match self.parse_meta_item(AllowLeadingUnsafe::No) {
Ok(mi) => return Ok(ast::NestedMetaItem::MetaItem(mi)),
Err(err) => err.cancel(), // we provide a better error below
}
Expand Down
Loading
Loading