Auto merge of #127543 - carbotaniuman:more_unsafe_attr_verification, …

…r=estebank,traviscross

More unsafe attr verification

This code denies unsafe on attributes such as `#[test]` and `#[ignore]`, while also changing the `MetaItem` parsing so `unsafe` in args like `#[allow(unsafe(dead_code))]` is not accidentally allowed.

Tracking:

- #123757

compiler/rustc_builtin_macros/messages.ftl

@@ -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

compiler/rustc_builtin_macros/src/cfg.rs

@@ -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};
 
@@ -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);
 

compiler/rustc_builtin_macros/src/cfg_accessible.rs

@@ -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 {

compiler/rustc_builtin_macros/src/derive.rs

@@ -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,
 };
@@ -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 {
@@ -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 {
@@ -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!(),
- }
-}

compiler/rustc_builtin_macros/src/errors.rs

@@ -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]

compiler/rustc_builtin_macros/src/util.rs

@@ -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,
  );
 }
 

compiler/rustc_expand/src/config.rs

@@ -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;
@@ -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 {
@@ -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)

compiler/rustc_interface/src/interface.rs

@@ -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};
@@ -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");
@@ -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) => {

compiler/rustc_parse/messages.ftl

@@ -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
 

compiler/rustc_parse/src/errors.rs

@@ -3183,6 +3183,7 @@ pub(crate) struct DotDotRangeAttribute {
 #[note]
 pub struct InvalidAttrUnsafe {
  #[primary_span]
+ #[label]
  pub span: Span,
  pub name: Path,
 }

compiler/rustc_parse/src/parser/attr.rs

@@ -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> {
@@ -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.
@@ -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
@@ -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);
@@ -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
  }