Move attribute safety checking to rustc_attr_parsing

This commit is contained in:
Jonathan Brouwer 2025-12-01 20:44:18 +01:00
parent eca9d93f90
commit 884fb5aef2
No known key found for this signature in database
GPG key ID: 13619B051B673C52
17 changed files with 187 additions and 176 deletions

View file

@ -1067,7 +1067,7 @@ fn validate_generic_param_order(dcx: DiagCtxtHandle<'_>, generics: &[GenericPara
impl<'a> Visitor<'a> for AstValidator<'a> {
fn visit_attribute(&mut self, attr: &Attribute) {
validate_attr::check_attr(&self.sess.psess, attr, self.lint_node_id);
validate_attr::check_attr(&self.sess.psess, attr);
}
fn visit_ty(&mut self, ty: &'a Ty) {

View file

@ -398,6 +398,7 @@ fn parse_cfg_attr_internal<'a>(
.into_boxed_slice(),
span: attribute.span,
},
Some(attribute.get_normal_item().unsafety),
ParsedDescription::Attribute,
pred_span,
CRATE_NODE_ID,

View file

@ -63,6 +63,7 @@ pub fn parse_cfg_select(
segments: vec![Ident::from_str("cfg_select")].into_boxed_slice(),
span: cfg_span,
},
None,
ParsedDescription::Macro,
cfg_span,
lint_node_id,

View file

@ -1,7 +1,7 @@
use std::borrow::Cow;
use rustc_ast as ast;
use rustc_ast::{AttrStyle, NodeId};
use rustc_ast::{AttrStyle, NodeId, Safety};
use rustc_errors::DiagCtxtHandle;
use rustc_feature::{AttributeTemplate, Features};
use rustc_hir::attrs::AttributeKind;
@ -146,6 +146,7 @@ impl<'sess> AttributeParser<'sess, Early> {
normal_attr.item.span(),
attr.style,
path.get_attribute_path(),
Some(normal_attr.item.unsafety),
ParsedDescription::Attribute,
target_span,
target_node_id,
@ -165,6 +166,7 @@ impl<'sess> AttributeParser<'sess, Early> {
inner_span: Span,
attr_style: AttrStyle,
attr_path: AttrPath,
attr_safety: Option<Safety>,
parsed_description: ParsedDescription,
target_span: Span,
target_node_id: NodeId,
@ -181,14 +183,24 @@ impl<'sess> AttributeParser<'sess, Early> {
sess,
stage: Early { emit_errors },
};
let mut emit_lint = |lint| {
crate::lints::emit_attribute_lint(&lint, sess);
};
if let Some(safety) = attr_safety {
parser.check_attribute_safety(
&attr_path,
inner_span,
safety,
&mut emit_lint,
target_node_id,
)
}
let mut cx: AcceptContext<'_, 'sess, Early> = AcceptContext {
shared: SharedContext {
cx: &mut parser,
target_span,
target_id: target_node_id,
emit_lint: &mut |lint| {
crate::lints::emit_attribute_lint(&lint, sess);
},
emit_lint: &mut emit_lint,
},
attr_span,
inner_span,
@ -289,6 +301,14 @@ impl<'sess, S: Stage> AttributeParser<'sess, S> {
ast::AttrKind::Normal(n) => {
attr_paths.push(PathParser(Cow::Borrowed(&n.item.path)));
self.check_attribute_safety(
&AttrPath::from_ast(&n.item.path),
lower_span(n.item.span()),
n.item.unsafety,
&mut emit_lint,
target_id,
);
let parts =
n.item.path.segments.iter().map(|seg| seg.ident.name).collect::<Vec<_>>();
@ -312,7 +332,7 @@ impl<'sess, S: Stage> AttributeParser<'sess, S> {
emit_lint: &mut emit_lint,
},
attr_span: lower_span(attr.span),
inner_span: lower_span(attr.get_normal_item().span()),
inner_span: lower_span(n.item.span()),
attr_style: attr.style,
parsed_description: ParsedDescription::Attribute,
template: &accept.template,

View file

@ -98,6 +98,7 @@ mod interface;
pub mod parser;
mod lints;
mod safety;
mod session_diagnostics;
mod target_checking;
pub mod validate_attr;

View file

@ -98,5 +98,17 @@ pub fn emit_attribute_lint<L: LintEmitter>(lint: &AttributeLint<L::Id>, lint_emi
},
)
}
&AttributeLintKind::UnsafeAttrOutsideUnsafe {
attribute_name_span,
sugg_spans: (left, right),
} => lint_emitter.emit_node_span_lint(
rustc_session::lint::builtin::UNSAFE_ATTR_OUTSIDE_UNSAFE,
*id,
*span,
session_diagnostics::UnsafeAttrOutsideUnsafeLint {
span: attribute_name_span,
suggestion: session_diagnostics::UnsafeAttrOutsideUnsafeSuggestion { left, right },
},
),
}
}

View file

@ -0,0 +1,116 @@
use rustc_ast::Safety;
use rustc_feature::{AttributeSafety, BUILTIN_ATTRIBUTE_MAP};
use rustc_hir::AttrPath;
use rustc_hir::lints::{AttributeLint, AttributeLintKind};
use rustc_span::{Span, sym};
use crate::context::Stage;
use crate::{AttributeParser, ShouldEmit};
impl<'sess, S: Stage> AttributeParser<'sess, S> {
pub fn check_attribute_safety(
&mut self,
attr_path: &AttrPath,
attr_span: Span,
attr_safety: Safety,
emit_lint: &mut impl FnMut(AttributeLint<S::Id>),
target_id: S::Id,
) {
if matches!(self.stage.should_emit(), ShouldEmit::Nothing) {
return;
}
let name = (attr_path.segments.len() == 1).then_some(attr_path.segments[0].name);
if let Some(name) = name
&& [sym::cfg_trace, sym::cfg_attr_trace].contains(&name)
{
return;
}
// FIXME: We should retrieve this information from the attribute parsers instead of from `BUILTIN_ATTRIBUTE_MAP`
let builtin_attr_info = name.and_then(|name| BUILTIN_ATTRIBUTE_MAP.get(&name));
let builtin_attr_safety = builtin_attr_info.map(|x| x.safety);
match (builtin_attr_safety, attr_safety) {
// - Unsafe builtin attribute
// - User wrote `#[unsafe(..)]`, which is permitted on any edition
(Some(AttributeSafety::Unsafe { .. }), Safety::Unsafe(..)) => {
// OK
}
// - Unsafe builtin attribute
// - User did not write `#[unsafe(..)]`
(Some(AttributeSafety::Unsafe { unsafe_since }), Safety::Default) => {
let path_span = attr_path.span;
// If the `attr_item`'s span is not from a macro, then just suggest
// wrapping it in `unsafe(...)`. Otherwise, we suggest putting the
// `unsafe(`, `)` right after and right before the opening and closing
// square bracket respectively.
let diag_span = attr_span;
// Attributes can be safe in earlier editions, and become unsafe in later ones.
//
// Use the span of the attribute's name to determine the edition: the span of the
// attribute as a whole may be inaccurate if it was emitted by a macro.
//
// See https://github.com/rust-lang/rust/issues/142182.
let emit_error = match unsafe_since {
None => true,
Some(unsafe_since) => path_span.edition() >= unsafe_since,
};
if emit_error {
self.stage.emit_err(
self.sess,
crate::session_diagnostics::UnsafeAttrOutsideUnsafe {
span: path_span,
suggestion:
crate::session_diagnostics::UnsafeAttrOutsideUnsafeSuggestion {
left: diag_span.shrink_to_lo(),
right: diag_span.shrink_to_hi(),
},
},
);
} else {
emit_lint(AttributeLint {
id: target_id,
span: path_span,
kind: AttributeLintKind::UnsafeAttrOutsideUnsafe {
attribute_name_span: path_span,
sugg_spans: (diag_span.shrink_to_lo(), diag_span.shrink_to_hi()),
},
})
}
}
// - Normal builtin attribute
// - Writing `#[unsafe(..)]` is not permitted on normal builtin attributes
(None | Some(AttributeSafety::Normal), Safety::Unsafe(unsafe_span)) => {
self.stage.emit_err(
self.sess,
crate::session_diagnostics::InvalidAttrUnsafe {
span: unsafe_span,
name: attr_path.clone(),
},
);
}
// - Normal builtin attribute
// - No explicit `#[unsafe(..)]` written.
(None | Some(AttributeSafety::Normal), Safety::Default) => {
// OK
}
(
Some(AttributeSafety::Unsafe { .. } | AttributeSafety::Normal) | None,
Safety::Safe(..),
) => {
self.sess.dcx().span_delayed_bug(
attr_span,
"`check_attribute_safety` does not expect `Safety::Safe` on attributes",
);
}
}
}
}

View file

@ -1,6 +1,6 @@
use std::num::IntErrorKind;
use rustc_ast::{self as ast, Path};
use rustc_ast::{self as ast};
use rustc_errors::codes::*;
use rustc_errors::{
Applicability, Diag, DiagArgValue, DiagCtxtHandle, Diagnostic, EmissionGuarantee, Level,
@ -790,7 +790,7 @@ pub(crate) struct InvalidAttrUnsafe {
#[primary_span]
#[label]
pub span: Span,
pub name: Path,
pub name: AttrPath,
}
#[derive(Diagnostic)]
@ -803,6 +803,15 @@ pub(crate) struct UnsafeAttrOutsideUnsafe {
pub suggestion: UnsafeAttrOutsideUnsafeSuggestion,
}
#[derive(LintDiagnostic)]
#[diag(attr_parsing_unsafe_attr_outside_unsafe)]
pub(crate) struct UnsafeAttrOutsideUnsafeLint {
#[label]
pub span: Span,
#[subdiagnostic]
pub suggestion: UnsafeAttrOutsideUnsafeSuggestion,
}
#[derive(Subdiagnostic)]
#[multipart_suggestion(
attr_parsing_unsafe_attr_outside_unsafe_suggestion,

View file

@ -5,21 +5,21 @@ use std::slice;
use rustc_ast::token::Delimiter;
use rustc_ast::tokenstream::DelimSpan;
use rustc_ast::{
self as ast, AttrArgs, Attribute, DelimArgs, MetaItem, MetaItemInner, MetaItemKind, NodeId,
Path, Safety,
self as ast, AttrArgs, Attribute, DelimArgs, MetaItem, MetaItemInner, MetaItemKind, Safety,
};
use rustc_errors::{Applicability, DiagCtxtHandle, FatalError, PResult};
use rustc_feature::{AttributeSafety, AttributeTemplate, BUILTIN_ATTRIBUTE_MAP, BuiltinAttribute};
use rustc_errors::{Applicability, FatalError, PResult};
use rustc_feature::{AttributeTemplate, BUILTIN_ATTRIBUTE_MAP, BuiltinAttribute};
use rustc_hir::AttrPath;
use rustc_parse::parse_in;
use rustc_session::errors::report_lit_error;
use rustc_session::lint::BuiltinLintDiag;
use rustc_session::lint::builtin::{ILL_FORMED_ATTRIBUTE_INPUT, UNSAFE_ATTR_OUTSIDE_UNSAFE};
use rustc_session::lint::builtin::ILL_FORMED_ATTRIBUTE_INPUT;
use rustc_session::parse::ParseSess;
use rustc_span::{Span, Symbol, sym};
use crate::{AttributeParser, Late, session_diagnostics as errors};
pub fn check_attr(psess: &ParseSess, attr: &Attribute, id: NodeId) {
pub fn check_attr(psess: &ParseSess, attr: &Attribute) {
if attr.is_doc_comment() || attr.has_name(sym::cfg_trace) || attr.has_name(sym::cfg_attr_trace)
{
return;
@ -27,9 +27,6 @@ pub fn check_attr(psess: &ParseSess, attr: &Attribute, id: NodeId) {
let builtin_attr_info = attr.ident().and_then(|ident| BUILTIN_ATTRIBUTE_MAP.get(&ident.name));
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 builtin_attr_info {
// `rustc_dummy` doesn't have any restrictions specific to built-in attributes.
@ -150,101 +147,6 @@ fn is_attr_template_compatible(template: &AttributeTemplate, meta: &ast::MetaIte
}
}
pub fn check_attribute_safety(
psess: &ParseSess,
builtin_attr_safety: Option<AttributeSafety>,
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
}
// - 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
// wrapping it in `unsafe(...)`. Otherwise, we suggest putting the
// `unsafe(`, `)` right after and right before the opening and closing
// square bracket respectively.
let diag_span = attr_item.span();
// Attributes can be safe in earlier editions, and become unsafe in later ones.
//
// Use the span of the attribute's name to determine the edition: the span of the
// attribute as a whole may be inaccurate if it was emitted by a macro.
//
// See https://github.com/rust-lang/rust/issues/142182.
let emit_error = match unsafe_since {
None => true,
Some(unsafe_since) => path_span.edition() >= unsafe_since,
};
if emit_error {
psess.dcx().emit_err(errors::UnsafeAttrOutsideUnsafe {
span: path_span,
suggestion: errors::UnsafeAttrOutsideUnsafeSuggestion {
left: diag_span.shrink_to_lo(),
right: diag_span.shrink_to_hi(),
},
});
} else {
psess.buffer_lint(
UNSAFE_ATTR_OUTSIDE_UNSAFE,
path_span,
id,
BuiltinLintDiag::UnsafeAttrOutsideUnsafe {
attribute_name_span: path_span,
sugg_spans: (diag_span.shrink_to_lo(), diag_span.shrink_to_hi()),
},
);
}
}
// - Normal builtin attribute
// - Writing `#[unsafe(..)]` is not permitted on normal builtin attributes
(None | Some(AttributeSafety::Normal), 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.
(None | Some(AttributeSafety::Normal), 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",
);
}
}
}
// Called by `check_builtin_meta_item` and code that manually denies
// `unsafe(...)` in `cfg`
pub fn deny_builtin_meta_unsafety(diag: DiagCtxtHandle<'_>, unsafety: Safety, name: &Path) {
// This only supports denying unsafety right now - making builtin attributes
// support unsafety will requite us to thread the actual `Attribute` through
// for the nice diagnostics.
if let Safety::Unsafe(unsafe_span) = unsafety {
diag.emit_err(errors::InvalidAttrUnsafe { span: unsafe_span, name: name.clone() });
}
}
pub fn check_builtin_meta_item(
psess: &ParseSess,
meta: &MetaItem,
@ -258,8 +160,11 @@ pub fn check_builtin_meta_item(
emit_malformed_attribute(psess, style, meta.span, name, template);
}
if deny_unsafety {
deny_builtin_meta_unsafety(psess.dcx(), meta.unsafety, &meta.path);
if deny_unsafety && let Safety::Unsafe(unsafe_span) = meta.unsafety {
psess.dcx().emit_err(errors::InvalidAttrUnsafe {
span: unsafe_span,
name: AttrPath::from_ast(&meta.path),
});
}
}

View file

@ -54,6 +54,7 @@ fn parse_cfg(cx: &ExtCtxt<'_>, span: Span, tts: TokenStream) -> Result<CfgEntry,
span,
AttrStyle::Inner,
AttrPath { segments: vec![Ident::from_str("cfg")].into_boxed_slice(), span },
None,
ParsedDescription::Macro,
span,
cx.current_expansion.lint_node_id,

View file

@ -11,15 +11,13 @@ use rustc_ast::{
NodeId, NormalAttr,
};
use rustc_attr_parsing as attr;
use rustc_attr_parsing::validate_attr::deny_builtin_meta_unsafety;
use rustc_attr_parsing::{
AttributeParser, CFG_TEMPLATE, EvalConfigResult, ShouldEmit, eval_config_entry, parse_cfg,
validate_attr,
};
use rustc_data_structures::flat_map_in_place::FlatMapInPlace;
use rustc_feature::{
ACCEPTED_LANG_FEATURES, AttributeSafety, EnabledLangFeature, EnabledLibFeature, Features,
REMOVED_LANG_FEATURES, UNSTABLE_LANG_FEATURES,
ACCEPTED_LANG_FEATURES, EnabledLangFeature, EnabledLibFeature, Features, REMOVED_LANG_FEATURES,
UNSTABLE_LANG_FEATURES,
};
use rustc_session::Session;
use rustc_session::parse::feature_err;
@ -291,13 +289,6 @@ 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.sess.psess,
Some(AttributeSafety::Normal),
&cfg_attr,
ast::CRATE_NODE_ID,
);
// A trace attribute left in AST in place of the original `cfg_attr` attribute.
// It can later be used by lints or other diagnostics.
let trace_attr = attr_into_trace(cfg_attr.clone(), sym::cfg_attr_trace);
@ -421,13 +412,6 @@ impl<'a> StripUnconfigured<'a> {
node: NodeId,
emit_errors: ShouldEmit,
) -> EvalConfigResult {
// Unsafety check needs to be done explicitly here because this attribute will be removed before the normal check
deny_builtin_meta_unsafety(
self.sess.dcx(),
attr.get_normal_item().unsafety,
&rustc_ast::Path::from_ident(attr.ident().unwrap()),
);
let Some(cfg) = AttributeParser::parse_single(
self.sess,
attr,

View file

@ -2158,11 +2158,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
let mut span: Option<Span> = None;
while let Some(attr) = attrs.next() {
rustc_ast_passes::feature_gate::check_attribute(attr, self.cx.sess, features);
validate_attr::check_attr(
&self.cx.sess.psess,
attr,
self.cx.current_expansion.lint_node_id,
);
validate_attr::check_attr(&self.cx.sess.psess, attr);
AttributeParser::parse_limited_all(
self.cx.sess,
slice::from_ref(attr),

View file

@ -62,4 +62,8 @@ pub enum AttributeLintKind {
target: Target,
target_span: Span,
},
UnsafeAttrOutsideUnsafe {
attribute_name_span: Span,
sugg_spans: (Span, Span),
},
}

View file

@ -890,10 +890,6 @@ lint_unpredictable_fn_pointer_comparisons = function pointer comparisons do not
lint_unqualified_local_imports = `use` of a local item without leading `self::`, `super::`, or `crate::`
lint_unsafe_attr_outside_unsafe = unsafe attribute used without unsafe
.label = usage of unsafe attribute
lint_unsafe_attr_outside_unsafe_suggestion = wrap the attribute in `unsafe(...)`
lint_unsupported_group = `{$lint_group}` lint group is not supported with ´--force-warn´
lint_untranslatable_diag = diagnostics should be created using translatable messages

View file

@ -283,16 +283,6 @@ pub fn decorate_builtin_lint(
BuiltinLintDiag::UnusedQualifications { removal_span } => {
lints::UnusedQualifications { removal_span }.decorate_lint(diag);
}
BuiltinLintDiag::UnsafeAttrOutsideUnsafe {
attribute_name_span,
sugg_spans: (left, right),
} => {
lints::UnsafeAttrOutsideUnsafe {
span: attribute_name_span,
suggestion: lints::UnsafeAttrOutsideUnsafeSuggestion { left, right },
}
.decorate_lint(diag);
}
BuiltinLintDiag::AssociatedConstElidedLifetime {
elided,
span: lt_span,

View file

@ -2880,27 +2880,6 @@ pub(crate) struct AssociatedConstElidedLifetime {
pub lifetimes_in_scope: MultiSpan,
}
#[derive(LintDiagnostic)]
#[diag(lint_unsafe_attr_outside_unsafe)]
pub(crate) struct UnsafeAttrOutsideUnsafe {
#[label]
pub span: Span,
#[subdiagnostic]
pub suggestion: UnsafeAttrOutsideUnsafeSuggestion,
}
#[derive(Subdiagnostic)]
#[multipart_suggestion(
lint_unsafe_attr_outside_unsafe_suggestion,
applicability = "machine-applicable"
)]
pub(crate) struct UnsafeAttrOutsideUnsafeSuggestion {
#[suggestion_part(code = "unsafe(")]
pub left: Span,
#[suggestion_part(code = ")")]
pub right: Span,
}
#[derive(LintDiagnostic)]
#[diag(lint_static_mut_refs_lint)]
pub(crate) struct RefOfMutStatic<'a> {

View file

@ -687,10 +687,6 @@ pub enum BuiltinLintDiag {
/// The span of the unnecessarily-qualified path to remove.
removal_span: Span,
},
UnsafeAttrOutsideUnsafe {
attribute_name_span: Span,
sugg_spans: (Span, Span),
},
AssociatedConstElidedLifetime {
elided: bool,
span: Span,