Skip to content

Commit

Permalink
Rollup merge of rust-lang#128771 - carbotaniuman:stabilize_unsafe_att…
Browse files Browse the repository at this point in the history
…r, r=nnethercote

Stabilize `unsafe_attributes`

# Stabilization report

## Summary

This is a tracking issue for the RFC 3325: unsafe attributes

We are stabilizing `#![feature(unsafe_attributes)]`,  which makes certain attributes considered 'unsafe', meaning that they must be surrounded by an `unsafe(...)`, as in `#[unsafe(no_mangle)]`.

RFC: rust-lang/rfcs#3325
Tracking issue: rust-lang#123757

## What is stabilized

### Summary of stabilization

Certain attributes will now be designated as unsafe attributes, namely, `no_mangle`, `export_name`, and `link_section` (stable only), and these attributes will need to be called by surrounding them in `unsafe(...)` syntax. On editions prior to 2024, this is simply an edition lint, but it will become a hard error in 2024. This also works in `cfg_attr`, but `unsafe` is not allowed for any other attributes, including proc-macros ones.

```rust
#[unsafe(no_mangle)]
fn a() {}

#[cfg_attr(any(), unsafe(export_name = "c"))]
fn b() {}
```

For a table showing the attributes that were considered to be included in the list to require unsafe, and subsequent reasoning about why each such attribute was or was not included, see [this comment here](rust-lang#124214 (comment))

## Tests

The relevant tests are in `tests/ui/rust-2024/unsafe-attributes` and `tests/ui/attributes/unsafe`.
  • Loading branch information
tgross35 authored Aug 17, 2024
2 parents 0f26ee4 + de9b5c3 commit ec82970
Show file tree
Hide file tree
Showing 37 changed files with 64 additions and 134 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_ast_passes/src/ast_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -887,7 +887,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.features, &self.session.psess, attr);
validate_attr::check_attr(&self.session.psess, attr);
}

fn visit_ty(&mut self, ty: &'a Ty) {
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_ast_passes/src/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -559,7 +559,6 @@ pub fn check_crate(krate: &ast::Crate, sess: &Session, features: &Features) {
gate_all!(mut_ref, "mutable by-reference bindings are experimental");
gate_all!(precise_capturing, "precise captures on `impl Trait` are experimental");
gate_all!(global_registration, "global registration is experimental");
gate_all!(unsafe_attributes, "`#[unsafe()]` markers for attributes are experimental");
gate_all!(return_type_notation, "return type notation is experimental");

if !visitor.features.never_patterns {
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_builtin_macros/src/cfg_accessible.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ 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,
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_builtin_macros/src/derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ 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,
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_builtin_macros/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ 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,
Expand Down
13 changes: 2 additions & 11 deletions compiler/rustc_expand/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,12 +265,7 @@ 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,
);
validate_attr::check_attribute_safety(&self.sess.psess, AttributeSafety::Normal, &cfg_attr);

let Some((cfg_predicate, expanded_attrs)) =
rustc_parse::parse_cfg_attr(cfg_attr, &self.sess.psess)
Expand Down Expand Up @@ -395,11 +390,7 @@ impl<'a> StripUnconfigured<'a> {
}
};

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

(
parse_cfg(&meta_item, self.sess).map_or(true, |meta_item| {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_expand/src/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(features, &self.cx.sess.psess, attr);
validate_attr::check_attr(&self.cx.sess.psess, attr);

let current_span = if let Some(sp) = span { sp.to(attr.span) } else { attr.span };
span = Some(current_span);
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_feature/src/accepted.rs
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,8 @@ declare_features! (
(accepted, universal_impl_trait, "1.26.0", Some(34511)),
/// Allows arbitrary delimited token streams in non-macro attributes.
(accepted, unrestricted_attribute_tokens, "1.34.0", Some(55208)),
/// Allows unsafe attributes.
(accepted, unsafe_attributes, "CURRENT_RUSTC_VERSION", Some(123757)),
/// The `unsafe_op_in_unsafe_fn` lint (allowed by default): no longer treat an unsafe function as an unsafe block.
(accepted, unsafe_block_in_unsafe_fn, "1.52.0", Some(71668)),
/// Allows unsafe on extern declarations and safety qualifiers over internal items.
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_feature/src/unstable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -622,8 +622,6 @@ declare_features! (
(unstable, type_changing_struct_update, "1.58.0", Some(86555)),
/// Allows unnamed fields of struct and union type
(incomplete, unnamed_fields, "1.74.0", Some(49804)),
/// Allows unsafe attributes.
(unstable, unsafe_attributes, "1.80.0", Some(123757)),
/// Allows const generic parameters to be defined with types that
/// are not `Sized`, e.g. `fn foo<const N: [u8]>() {`.
(incomplete, unsized_const_params, "CURRENT_RUSTC_VERSION", Some(95174)),
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4971,7 +4971,6 @@ declare_lint! {
/// ### Example
///
/// ```rust
/// #![feature(unsafe_attributes)]
/// #![warn(unsafe_attr_outside_unsafe)]
///
/// #[no_mangle]
Expand Down
4 changes: 1 addition & 3 deletions compiler/rustc_parse/src/parser/attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use rustc_ast::token::{self, Delimiter};
use rustc_errors::codes::*;
use rustc_errors::{Diag, PResult};
use rustc_span::symbol::kw;
use rustc_span::{sym, BytePos, Span};
use rustc_span::{BytePos, Span};
use thin_vec::ThinVec;
use tracing::debug;

Expand Down Expand Up @@ -265,7 +265,6 @@ impl<'a> Parser<'a> {
let is_unsafe = this.eat_keyword(kw::Unsafe);
let unsafety = if is_unsafe {
let unsafe_span = this.prev_token.span;
this.psess.gated_spans.gate(sym::unsafe_attributes, unsafe_span);
this.expect(&token::OpenDelim(Delimiter::Parenthesis))?;
ast::Safety::Unsafe(unsafe_span)
} else {
Expand Down Expand Up @@ -406,7 +405,6 @@ impl<'a> Parser<'a> {
};
let unsafety = if is_unsafe {
let unsafe_span = self.prev_token.span;
self.psess.gated_spans.gate(sym::unsafe_attributes, unsafe_span);
self.expect(&token::OpenDelim(Delimiter::Parenthesis))?;

ast::Safety::Unsafe(unsafe_span)
Expand Down
40 changes: 13 additions & 27 deletions compiler/rustc_parse/src/validate_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@ use rustc_ast::{
NestedMetaItem, Safety,
};
use rustc_errors::{Applicability, FatalError, PResult};
use rustc_feature::{
AttributeSafety, AttributeTemplate, BuiltinAttribute, Features, BUILTIN_ATTRIBUTE_MAP,
};
use rustc_feature::{AttributeSafety, AttributeTemplate, BuiltinAttribute, BUILTIN_ATTRIBUTE_MAP};
use rustc_session::errors::report_lit_error;
use rustc_session::lint::builtin::{ILL_FORMED_ATTRIBUTE_INPUT, UNSAFE_ATTR_OUTSIDE_UNSAFE};
use rustc_session::lint::BuiltinLintDiag;
Expand All @@ -18,7 +16,7 @@ use rustc_span::{sym, BytePos, Span, Symbol};

use crate::{errors, parse_in};

pub fn check_attr(features: &Features, psess: &ParseSess, attr: &Attribute) {
pub fn check_attr(psess: &ParseSess, attr: &Attribute) {
if attr.is_doc_comment() {
return;
}
Expand All @@ -28,17 +26,17 @@ pub fn check_attr(features: &Features, psess: &ParseSess, attr: &Attribute) {

// All non-builtin attributes are considered safe
let safety = attr_info.map(|x| x.safety).unwrap_or(AttributeSafety::Normal);
check_attribute_safety(features, psess, safety, attr);
check_attribute_safety(psess, safety, attr);

// Check input tokens for built-in and key-value attributes.
match attr_info {
// `rustc_dummy` doesn't have any restrictions specific to built-in attributes.
Some(BuiltinAttribute { name, template, .. }) if *name != sym::rustc_dummy => {
match parse_meta(psess, attr) {
// Don't check safety again, we just did that
Ok(meta) => check_builtin_meta_item(
features, psess, &meta, attr.style, *name, *template, false,
),
Ok(meta) => {
check_builtin_meta_item(psess, &meta, attr.style, *name, *template, false)
}
Err(err) => {
err.emit();
}
Expand Down Expand Up @@ -157,16 +155,7 @@ fn is_attr_template_compatible(template: &AttributeTemplate, meta: &ast::MetaIte
}
}

pub fn check_attribute_safety(
features: &Features,
psess: &ParseSess,
safety: AttributeSafety,
attr: &Attribute,
) {
if !features.unsafe_attributes {
return;
}

pub fn check_attribute_safety(psess: &ParseSess, safety: AttributeSafety, attr: &Attribute) {
let attr_item = attr.get_normal_item();

if safety == AttributeSafety::Unsafe {
Expand Down Expand Up @@ -215,21 +204,18 @@ pub fn check_attribute_safety(

// Called by `check_builtin_meta_item` and code that manually denies
// `unsafe(...)` in `cfg`
pub fn deny_builtin_meta_unsafety(features: &Features, psess: &ParseSess, meta: &MetaItem) {
pub fn deny_builtin_meta_unsafety(psess: &ParseSess, meta: &MetaItem) {
// 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 features.unsafe_attributes {
if let Safety::Unsafe(unsafe_span) = meta.unsafety {
psess
.dcx()
.emit_err(errors::InvalidAttrUnsafe { span: unsafe_span, name: meta.path.clone() });
}
if let Safety::Unsafe(unsafe_span) = meta.unsafety {
psess
.dcx()
.emit_err(errors::InvalidAttrUnsafe { span: unsafe_span, name: meta.path.clone() });
}
}

pub fn check_builtin_meta_item(
features: &Features,
psess: &ParseSess,
meta: &MetaItem,
style: ast::AttrStyle,
Expand All @@ -246,7 +232,7 @@ pub fn check_builtin_meta_item(
}

if deny_unsafety {
deny_builtin_meta_unsafety(features, psess, meta);
deny_builtin_meta_unsafety(psess, meta);
}
}

Expand Down
1 change: 0 additions & 1 deletion src/tools/rustfmt/tests/target/unsafe_attributes.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
#![feature(unsafe_attributes)]
// https://github.com/rust-lang/rust/issues/123757
//
#![simple_ident]
Expand Down
1 change: 0 additions & 1 deletion tests/ui/attributes/unsafe/cfg-unsafe-attributes.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
//@ build-pass
#![feature(unsafe_attributes)]

#[cfg_attr(all(), unsafe(no_mangle))]
fn a() {}
Expand Down
2 changes: 0 additions & 2 deletions tests/ui/attributes/unsafe/derive-unsafe-attributes.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
#![feature(unsafe_attributes)]

#[derive(unsafe(Debug))]
//~^ ERROR: expected identifier, found keyword `unsafe`
//~| ERROR: traits in `#[derive(...)]` don't accept arguments
Expand Down
14 changes: 7 additions & 7 deletions tests/ui/attributes/unsafe/derive-unsafe-attributes.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: expected identifier, found keyword `unsafe`
--> $DIR/derive-unsafe-attributes.rs:3:10
--> $DIR/derive-unsafe-attributes.rs:1:10
|
LL | #[derive(unsafe(Debug))]
| ^^^^^^ expected identifier, found keyword
Expand All @@ -10,21 +10,21 @@ LL | #[derive(r#unsafe(Debug))]
| ++

error: traits in `#[derive(...)]` don't accept arguments
--> $DIR/derive-unsafe-attributes.rs:3:16
--> $DIR/derive-unsafe-attributes.rs:1:16
|
LL | #[derive(unsafe(Debug))]
| ^^^^^^^ help: remove the arguments

error: `derive` is not an unsafe attribute
--> $DIR/derive-unsafe-attributes.rs:12:3
--> $DIR/derive-unsafe-attributes.rs:10:3
|
LL | #[unsafe(derive(Debug))]
| ^^^^^^ this is not an unsafe attribute
|
= note: extraneous unsafe is not allowed in attributes

error: expected identifier, found keyword `unsafe`
--> $DIR/derive-unsafe-attributes.rs:3:10
--> $DIR/derive-unsafe-attributes.rs:1:10
|
LL | #[derive(unsafe(Debug))]
| ^^^^^^ expected identifier, found keyword
Expand All @@ -36,7 +36,7 @@ LL | #[derive(r#unsafe(Debug))]
| ++

error: expected identifier, found keyword `unsafe`
--> $DIR/derive-unsafe-attributes.rs:3:10
--> $DIR/derive-unsafe-attributes.rs:1:10
|
LL | #[derive(unsafe(Debug))]
| ^^^^^^ expected identifier, found keyword
Expand All @@ -48,13 +48,13 @@ LL | #[derive(r#unsafe(Debug))]
| ++

error: cannot find derive macro `r#unsafe` in this scope
--> $DIR/derive-unsafe-attributes.rs:3:10
--> $DIR/derive-unsafe-attributes.rs:1:10
|
LL | #[derive(unsafe(Debug))]
| ^^^^^^

error: cannot find derive macro `r#unsafe` in this scope
--> $DIR/derive-unsafe-attributes.rs:3:10
--> $DIR/derive-unsafe-attributes.rs:1:10
|
LL | #[derive(unsafe(Debug))]
| ^^^^^^
Expand Down
2 changes: 0 additions & 2 deletions tests/ui/attributes/unsafe/double-unsafe-attributes.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
#![feature(unsafe_attributes)]

#[unsafe(unsafe(no_mangle))]
//~^ ERROR expected identifier, found keyword `unsafe`
//~| ERROR cannot find attribute `r#unsafe` in this scope
Expand Down
6 changes: 3 additions & 3 deletions tests/ui/attributes/unsafe/double-unsafe-attributes.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: expected identifier, found keyword `unsafe`
--> $DIR/double-unsafe-attributes.rs:3:10
--> $DIR/double-unsafe-attributes.rs:1:10
|
LL | #[unsafe(unsafe(no_mangle))]
| ^^^^^^ expected identifier, found keyword
Expand All @@ -10,15 +10,15 @@ LL | #[unsafe(r#unsafe(no_mangle))]
| ++

error: `r#unsafe` is not an unsafe attribute
--> $DIR/double-unsafe-attributes.rs:3:3
--> $DIR/double-unsafe-attributes.rs:1:3
|
LL | #[unsafe(unsafe(no_mangle))]
| ^^^^^^ this is not an unsafe attribute
|
= note: extraneous unsafe is not allowed in attributes

error: cannot find attribute `r#unsafe` in this scope
--> $DIR/double-unsafe-attributes.rs:3:10
--> $DIR/double-unsafe-attributes.rs:1:10
|
LL | #[unsafe(unsafe(no_mangle))]
| ^^^^^^
Expand Down
1 change: 0 additions & 1 deletion tests/ui/attributes/unsafe/extraneous-unsafe-attributes.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
//@ edition: 2024
//@ compile-flags: -Zunstable-options
#![feature(unsafe_attributes)]

#[unsafe(cfg(any()))] //~ ERROR: is not an unsafe attribute
fn a() {}
Expand Down
Loading

0 comments on commit ec82970

Please sign in to comment.