diff options
| author | carbotaniuman <41451839+carbotaniuman@users.noreply.github.com> | 2024-06-08 18:39:09 -0500 |
|---|---|---|
| committer | carbotaniuman <41451839+carbotaniuman@users.noreply.github.com> | 2024-06-23 19:02:14 -0500 |
| commit | a23917cfd07b309f32603c6acb81b63aad8504e7 (patch) | |
| tree | 7ead8d9df7c78555088b9e034441f202ab30d94e /compiler | |
| parent | 33422e72c8a66bdb5ee21246a948a1a02ca91674 (diff) | |
| download | rust-a23917cfd07b309f32603c6acb81b63aad8504e7.tar.gz rust-a23917cfd07b309f32603c6acb81b63aad8504e7.zip | |
Add hard error and migration lint for unsafe attrs
Diffstat (limited to 'compiler')
| -rw-r--r-- | compiler/rustc_ast_passes/src/ast_validation.rs | 2 | ||||
| -rw-r--r-- | compiler/rustc_expand/src/expand.rs | 2 | ||||
| -rw-r--r-- | compiler/rustc_feature/src/builtin_attrs.rs | 4 | ||||
| -rw-r--r-- | compiler/rustc_feature/src/lib.rs | 2 | ||||
| -rw-r--r-- | compiler/rustc_lint/messages.ftl | 4 | ||||
| -rw-r--r-- | compiler/rustc_lint/src/context/diagnostics.rs | 10 | ||||
| -rw-r--r-- | compiler/rustc_lint/src/lints.rs | 21 | ||||
| -rw-r--r-- | compiler/rustc_lint_defs/src/builtin.rs | 43 | ||||
| -rw-r--r-- | compiler/rustc_lint_defs/src/lib.rs | 4 | ||||
| -rw-r--r-- | compiler/rustc_parse/messages.ftl | 9 | ||||
| -rw-r--r-- | compiler/rustc_parse/src/errors.rs | 31 | ||||
| -rw-r--r-- | compiler/rustc_parse/src/validate_attr.rs | 64 | ||||
| -rw-r--r-- | compiler/rustc_passes/messages.ftl | 4 | ||||
| -rw-r--r-- | compiler/rustc_passes/src/check_attr.rs | 21 | ||||
| -rw-r--r-- | compiler/rustc_passes/src/errors.rs | 11 |
15 files changed, 185 insertions, 47 deletions
diff --git a/compiler/rustc_ast_passes/src/ast_validation.rs b/compiler/rustc_ast_passes/src/ast_validation.rs index 79717c969d7..941bb78c0dd 100644 --- a/compiler/rustc_ast_passes/src/ast_validation.rs +++ b/compiler/rustc_ast_passes/src/ast_validation.rs @@ -899,7 +899,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.session.psess, attr); + validate_attr::check_attr(&self.features, &self.session.psess, attr); } fn visit_ty(&mut self, ty: &'a Ty) { diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index 716bfc8c26b..5a807d1dba8 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/compiler/rustc_expand/src/expand.rs @@ -1882,7 +1882,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); + validate_attr::check_attr(features, &self.cx.sess.psess, attr); let current_span = if let Some(sp) = span { sp.to(attr.span) } else { attr.span }; span = Some(current_span); diff --git a/compiler/rustc_feature/src/builtin_attrs.rs b/compiler/rustc_feature/src/builtin_attrs.rs index 9e2756f07ed..5e83e0d27e1 100644 --- a/compiler/rustc_feature/src/builtin_attrs.rs +++ b/compiler/rustc_feature/src/builtin_attrs.rs @@ -1145,10 +1145,6 @@ pub fn is_valid_for_get_attr(name: Symbol) -> bool { }) } -pub fn is_unsafe_attr(name: Symbol) -> bool { - BUILTIN_ATTRIBUTE_MAP.get(&name).is_some_and(|attr| attr.safety == AttributeSafety::Unsafe) -} - pub static BUILTIN_ATTRIBUTE_MAP: LazyLock<FxHashMap<Symbol, &BuiltinAttribute>> = LazyLock::new(|| { let mut map = FxHashMap::default(); diff --git a/compiler/rustc_feature/src/lib.rs b/compiler/rustc_feature/src/lib.rs index fb3b7c0a127..bf429364318 100644 --- a/compiler/rustc_feature/src/lib.rs +++ b/compiler/rustc_feature/src/lib.rs @@ -125,7 +125,7 @@ pub use accepted::ACCEPTED_FEATURES; pub use builtin_attrs::AttributeDuplicates; pub use builtin_attrs::{ deprecated_attributes, encode_cross_crate, find_gated_cfg, is_builtin_attr_name, - is_unsafe_attr, is_valid_for_get_attr, AttributeGate, AttributeTemplate, AttributeType, + is_valid_for_get_attr, AttributeGate, AttributeSafety, AttributeTemplate, AttributeType, BuiltinAttribute, GatedCfg, BUILTIN_ATTRIBUTES, BUILTIN_ATTRIBUTE_MAP, }; pub use removed::REMOVED_FEATURES; diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index fdedf2c2e6d..eac5083ffbf 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -825,6 +825,10 @@ lint_unnameable_test_items = cannot test inner items lint_unnecessary_qualification = unnecessary qualification .suggestion = remove the unnecessary path segments +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 diff --git a/compiler/rustc_lint/src/context/diagnostics.rs b/compiler/rustc_lint/src/context/diagnostics.rs index 290bb5173db..adb2a3275c0 100644 --- a/compiler/rustc_lint/src/context/diagnostics.rs +++ b/compiler/rustc_lint/src/context/diagnostics.rs @@ -319,6 +319,16 @@ pub(super) fn decorate_lint(sess: &Session, diagnostic: BuiltinLintDiag, diag: & 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, diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index b26d04d0618..6df3a11deb0 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -2890,3 +2890,24 @@ pub struct RedundantImportVisibility { pub import_vis: String, pub max_vis: String, } + +#[derive(LintDiagnostic)] +#[diag(lint_unsafe_attr_outside_unsafe)] +pub struct UnsafeAttrOutsideUnsafe { + #[label] + pub span: Span, + #[subdiagnostic] + pub suggestion: UnsafeAttrOutsideUnsafeSuggestion, +} + +#[derive(Subdiagnostic)] +#[multipart_suggestion( + lint_unsafe_attr_outside_unsafe_suggestion, + applicability = "machine-applicable" +)] +pub struct UnsafeAttrOutsideUnsafeSuggestion { + #[suggestion_part(code = "unsafe(")] + pub left: Span, + #[suggestion_part(code = ")")] + pub right: Span, +} diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index 1913b9d6a1c..265779c9374 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -115,6 +115,7 @@ declare_lint_pass! { UNNAMEABLE_TYPES, UNREACHABLE_CODE, UNREACHABLE_PATTERNS, + UNSAFE_ATTR_OUTSIDE_UNSAFE, UNSAFE_OP_IN_UNSAFE_FN, UNSTABLE_NAME_COLLISIONS, UNSTABLE_SYNTAX_PRE_EXPANSION, @@ -4902,3 +4903,45 @@ declare_lint! { reference: "issue #123743 <https://github.com/rust-lang/rust/issues/123743>", }; } + +declare_lint! { + /// The `unsafe_attr_outside_unsafe` lint detects a missing unsafe keyword + /// on attributes considered unsafe. + /// + /// ### Example + /// + /// ```rust + /// #![feature(unsafe_attributes)] + /// #![warn(unsafe_attr_outside_unsafe)] + /// + /// #[no_mangle] + /// extern "C" fn foo() {} + /// + /// fn main() {} + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// Some attributes (e.g. `no_mangle`, `export_name`, `link_section` -- see + /// [issue #82499] for a more complete list) are considered "unsafe" attributes. + /// An unsafe attribute must only be used inside unsafe(...). + /// + /// This lint can automatically wrap the attributes in `unsafe(...)` , but this + /// obviously cannot verify that the preconditions of the `unsafe` + /// attributes are fulfilled, so that is still up to the user. + /// + /// The lint is currently "allow" by default, but that might change in the + /// future. + /// + /// [editions]: https://doc.rust-lang.org/edition-guide/ + /// [issue #82499]: https://github.com/rust-lang/rust/issues/82499 + pub UNSAFE_ATTR_OUTSIDE_UNSAFE, + Allow, + "detects unsafe attributes outside of unsafe", + @future_incompatible = FutureIncompatibleInfo { + reason: FutureIncompatibilityReason::EditionError(Edition::Edition2024), + reference: "issue #123757 <https://github.com/rust-lang/rust/issues/123757>", + }; +} diff --git a/compiler/rustc_lint_defs/src/lib.rs b/compiler/rustc_lint_defs/src/lib.rs index b3838f915f6..f33aadfbbc8 100644 --- a/compiler/rustc_lint_defs/src/lib.rs +++ b/compiler/rustc_lint_defs/src/lib.rs @@ -691,6 +691,10 @@ 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, diff --git a/compiler/rustc_parse/messages.ftl b/compiler/rustc_parse/messages.ftl index efb9526eabc..f08efe60d96 100644 --- a/compiler/rustc_parse/messages.ftl +++ b/compiler/rustc_parse/messages.ftl @@ -366,6 +366,10 @@ parse_inner_doc_comment_not_permitted = expected outer doc comment .label_does_not_annotate_this = the inner doc comment doesn't annotate this {$item} .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 + .suggestion = remove the `unsafe(...)` + .note = extraneous unsafe is not allowed in attributes + parse_invalid_block_macro_segment = cannot use a `block` macro fragment here .label = the `block` fragment is within this context .suggestion = wrap this in another block @@ -866,6 +870,11 @@ parse_unmatched_angle_brackets = {$num_extra_brackets -> *[other] remove extra angle brackets } +parse_unsafe_attr_outside_unsafe = unsafe attribute used without unsafe + .label = usage of unsafe attribute +parse_unsafe_attr_outside_unsafe_suggestion = wrap the attribute in `unsafe(...)` + + parse_unskipped_whitespace = whitespace symbol '{$ch}' is not skipped .label = {parse_unskipped_whitespace} diff --git a/compiler/rustc_parse/src/errors.rs b/compiler/rustc_parse/src/errors.rs index 46e15734853..8d49887f164 100644 --- a/compiler/rustc_parse/src/errors.rs +++ b/compiler/rustc_parse/src/errors.rs @@ -2997,3 +2997,34 @@ pub(crate) struct DotDotRangeAttribute { #[primary_span] pub span: Span, } + +#[derive(Diagnostic)] +#[diag(parse_invalid_attr_unsafe)] +#[note] +pub struct InvalidAttrUnsafe { + #[primary_span] + pub span: Span, + pub name: Path, +} + +#[derive(Diagnostic)] +#[diag(parse_unsafe_attr_outside_unsafe)] +pub struct UnsafeAttrOutsideUnsafe { + #[primary_span] + #[label] + pub span: Span, + #[subdiagnostic] + pub suggestion: UnsafeAttrOutsideUnsafeSuggestion, +} + +#[derive(Subdiagnostic)] +#[multipart_suggestion( + parse_unsafe_attr_outside_unsafe_suggestion, + applicability = "machine-applicable" +)] +pub struct UnsafeAttrOutsideUnsafeSuggestion { + #[suggestion_part(code = "unsafe(")] + pub left: Span, + #[suggestion_part(code = ")")] + pub right: Span, +} diff --git a/compiler/rustc_parse/src/validate_attr.rs b/compiler/rustc_parse/src/validate_attr.rs index 4ca52146039..bcb1131cc19 100644 --- a/compiler/rustc_parse/src/validate_attr.rs +++ b/compiler/rustc_parse/src/validate_attr.rs @@ -5,21 +5,73 @@ use crate::{errors, parse_in}; use rustc_ast::token::Delimiter; use rustc_ast::tokenstream::DelimSpan; use rustc_ast::MetaItemKind; -use rustc_ast::{self as ast, AttrArgs, AttrArgsEq, Attribute, DelimArgs, MetaItem}; +use rustc_ast::{self as ast, AttrArgs, AttrArgsEq, Attribute, DelimArgs, MetaItem, Safety}; use rustc_errors::{Applicability, FatalError, PResult}; -use rustc_feature::{AttributeTemplate, BuiltinAttribute, BUILTIN_ATTRIBUTE_MAP}; +use rustc_feature::{ + AttributeSafety, AttributeTemplate, BuiltinAttribute, Features, BUILTIN_ATTRIBUTE_MAP, +}; use rustc_session::errors::report_lit_error; -use rustc_session::lint::builtin::ILL_FORMED_ATTRIBUTE_INPUT; +use rustc_session::lint::builtin::{ILL_FORMED_ATTRIBUTE_INPUT, UNSAFE_ATTR_OUTSIDE_UNSAFE}; use rustc_session::lint::BuiltinLintDiag; use rustc_session::parse::ParseSess; -use rustc_span::{sym, Span, Symbol}; +use rustc_span::{sym, BytePos, Span, Symbol}; -pub fn check_attr(psess: &ParseSess, attr: &Attribute) { +pub fn check_attr(features: &Features, psess: &ParseSess, attr: &Attribute) { if attr.is_doc_comment() { return; } let attr_info = attr.ident().and_then(|ident| BUILTIN_ATTRIBUTE_MAP.get(&ident.name)); + let attr_item = attr.get_normal_item(); + + let is_unsafe_attr = attr_info.is_some_and(|attr| attr.safety == AttributeSafety::Unsafe); + + if features.unsafe_attributes { + if is_unsafe_attr { + if let ast::Safety::Default = attr_item.unsafety { + 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 = if attr_item.span().can_be_used_for_suggestions() { + attr_item.span() + } else { + attr.span + .with_lo(attr.span.lo() + BytePos(2)) + .with_hi(attr.span.hi() - BytePos(1)) + }; + + if attr.span.at_least_rust_2024() { + 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, + ast::CRATE_NODE_ID, + BuiltinLintDiag::UnsafeAttrOutsideUnsafe { + attribute_name_span: path_span, + sugg_spans: (diag_span.shrink_to_lo(), diag_span.shrink_to_hi()), + }, + ); + } + } + } else { + if let Safety::Unsafe(unsafe_span) = attr_item.unsafety { + psess.dcx().emit_err(errors::InvalidAttrUnsafe { + span: unsafe_span, + name: attr_item.path.clone(), + }); + } + } + } // Check input tokens for built-in and key-value attributes. match attr_info { @@ -32,7 +84,7 @@ pub fn check_attr(psess: &ParseSess, attr: &Attribute) { } } } - _ if let AttrArgs::Eq(..) = attr.get_normal_item().args => { + _ if let AttrArgs::Eq(..) = attr_item.args => { // All key-value attributes are restricted to meta-item syntax. match parse_meta(psess, attr) { Ok(_) => {} diff --git a/compiler/rustc_passes/messages.ftl b/compiler/rustc_passes/messages.ftl index 84c71c4bed2..9a830b0f49b 100644 --- a/compiler/rustc_passes/messages.ftl +++ b/compiler/rustc_passes/messages.ftl @@ -384,10 +384,6 @@ passes_invalid_attr_at_crate_level = passes_invalid_attr_at_crate_level_item = the inner attribute doesn't annotate this {$kind} -passes_invalid_attr_unsafe = `{$name}` is not an unsafe attribute - .suggestion = remove the `unsafe(...)` - .note = extraneous unsafe is not allowed in attributes - passes_invalid_macro_export_arguments = `{$name}` isn't a valid `#[macro_export]` argument passes_invalid_macro_export_arguments_too_many_items = `#[macro_export]` can only take 1 or 0 arguments diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs index a0b3470df6d..2ed5bba85c6 100644 --- a/compiler/rustc_passes/src/check_attr.rs +++ b/compiler/rustc_passes/src/check_attr.rs @@ -10,9 +10,7 @@ use rustc_ast::{MetaItemKind, MetaItemLit, NestedMetaItem}; use rustc_data_structures::fx::FxHashMap; use rustc_errors::{Applicability, IntoDiagArg, MultiSpan}; use rustc_errors::{DiagCtxtHandle, StashKey}; -use rustc_feature::{ - is_unsafe_attr, AttributeDuplicates, AttributeType, BuiltinAttribute, BUILTIN_ATTRIBUTE_MAP, -}; +use rustc_feature::{AttributeDuplicates, AttributeType, BuiltinAttribute, BUILTIN_ATTRIBUTE_MAP}; use rustc_hir::def_id::LocalModDefId; use rustc_hir::intravisit::{self, Visitor}; use rustc_hir::{self as hir}; @@ -116,8 +114,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> { let mut seen = FxHashMap::default(); let attrs = self.tcx.hir().attrs(hir_id); for attr in attrs { - self.check_unsafe_attr(attr); - match attr.path().as_slice() { [sym::diagnostic, sym::do_not_recommend] => { self.check_do_not_recommend(attr.span, hir_id, target) @@ -312,21 +308,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> { true } - /// Checks if `unsafe()` is applied to an invalid attribute. - fn check_unsafe_attr(&self, attr: &Attribute) { - if !attr.is_doc_comment() { - let attr_item = attr.get_normal_item(); - if let ast::Safety::Unsafe(unsafe_span) = attr_item.unsafety { - if !is_unsafe_attr(attr.name_or_empty()) { - self.dcx().emit_err(errors::InvalidAttrUnsafe { - span: unsafe_span, - name: attr_item.path.clone(), - }); - } - } - } - } - /// Checks if `#[diagnostic::on_unimplemented]` is applied to a trait definition fn check_diagnostic_on_unimplemented( &self, diff --git a/compiler/rustc_passes/src/errors.rs b/compiler/rustc_passes/src/errors.rs index d27b94ebd22..f0596568092 100644 --- a/compiler/rustc_passes/src/errors.rs +++ b/compiler/rustc_passes/src/errors.rs @@ -4,7 +4,7 @@ use std::{ }; use crate::fluent_generated as fluent; -use rustc_ast::{ast, Label}; +use rustc_ast::Label; use rustc_errors::{ codes::*, Applicability, Diag, DiagCtxtHandle, DiagSymbolList, Diagnostic, EmissionGuarantee, Level, MultiSpan, SubdiagMessageOp, Subdiagnostic, @@ -863,15 +863,6 @@ pub struct InvalidAttrAtCrateLevel { pub item: Option<ItemFollowingInnerAttr>, } -#[derive(Diagnostic)] -#[diag(passes_invalid_attr_unsafe)] -#[note] -pub struct InvalidAttrUnsafe { - #[primary_span] - pub span: Span, - pub name: ast::Path, -} - #[derive(Clone, Copy)] pub struct ItemFollowingInnerAttr { pub span: Span, |
