Skip to content

enforce unsafe attributes in pre-2024 editions by default #139718

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

Merged
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
4 changes: 2 additions & 2 deletions compiler/rustc_error_codes/src/error_codes/E0755.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ Erroneous code example:
```compile_fail,E0755
#![feature(ffi_pure)]
#[ffi_pure] // error!
#[unsafe(ffi_pure)] // error!
pub fn foo() {}
# fn main() {}
```
Expand All @@ -17,7 +17,7 @@ side effects or infinite loops:
#![feature(ffi_pure)]
extern "C" {
#[ffi_pure] // ok!
#[unsafe(ffi_pure)] // ok!
pub fn strlen(s: *const i8) -> isize;
}
# fn main() {}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_error_codes/src/error_codes/E0756.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Erroneous code example:
```compile_fail,E0756
#![feature(ffi_const)]
#[ffi_const] // error!
#[unsafe(ffi_const)] // error!
pub fn foo() {}
# fn main() {}
```
Expand All @@ -18,7 +18,7 @@ which have no side effects except for their return value:
#![feature(ffi_const)]
extern "C" {
#[ffi_const] // ok!
#[unsafe(ffi_const)] // ok!
pub fn strlen(s: *const i8) -> i32;
}
# fn main() {}
Expand Down
7 changes: 4 additions & 3 deletions compiler/rustc_error_codes/src/error_codes/E0757.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ Erroneous code example:
#![feature(ffi_const, ffi_pure)]

extern "C" {
#[ffi_const]
#[ffi_pure] // error: `#[ffi_const]` function cannot be `#[ffi_pure]`
#[unsafe(ffi_const)]
#[unsafe(ffi_pure)]
//~^ ERROR `#[ffi_const]` function cannot be `#[ffi_pure]`
pub fn square(num: i32) -> i32;
}
```
Expand All @@ -19,7 +20,7 @@ As `ffi_const` provides stronger guarantees than `ffi_pure`, remove the
#![feature(ffi_const)]

extern "C" {
#[ffi_const]
#[unsafe(ffi_const)]
pub fn square(num: i32) -> i32;
}
```
Expand Down
33 changes: 24 additions & 9 deletions compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use AttributeDuplicates::*;
use AttributeGate::*;
use AttributeType::*;
use rustc_data_structures::fx::FxHashMap;
use rustc_span::edition::Edition;
use rustc_span::{Symbol, sym};

use crate::{Features, Stability};
Expand Down Expand Up @@ -65,9 +66,12 @@ pub enum AttributeSafety {
/// Normal attribute that does not need `#[unsafe(...)]`
Normal,

/// Unsafe attribute that requires safety obligations
/// to be discharged
Unsafe,
/// Unsafe attribute that requires safety obligations to be discharged.
///
/// An error is emitted when `#[unsafe(...)]` is omitted, except when the attribute's edition
/// is less than the one stored in `unsafe_since`. This handles attributes that were safe in
/// earlier editions, but become unsafe in later ones.
Unsafe { unsafe_since: Option<Edition> },
}

#[derive(Clone, Copy)]
Expand Down Expand Up @@ -187,12 +191,23 @@ macro_rules! template {
}

macro_rules! ungated {
(unsafe($edition:ident) $attr:ident, $typ:expr, $tpl:expr, $duplicates:expr, $encode_cross_crate:expr $(,)?) => {
BuiltinAttribute {
name: sym::$attr,
encode_cross_crate: $encode_cross_crate,
type_: $typ,
safety: AttributeSafety::Unsafe { unsafe_since: Some(Edition::$edition) },
template: $tpl,
gate: Ungated,
duplicates: $duplicates,
}
};
(unsafe $attr:ident, $typ:expr, $tpl:expr, $duplicates:expr, $encode_cross_crate:expr $(,)?) => {
BuiltinAttribute {
name: sym::$attr,
encode_cross_crate: $encode_cross_crate,
type_: $typ,
safety: AttributeSafety::Unsafe,
safety: AttributeSafety::Unsafe { unsafe_since: None },
template: $tpl,
gate: Ungated,
duplicates: $duplicates,
Expand All @@ -217,7 +232,7 @@ macro_rules! gated {
name: sym::$attr,
encode_cross_crate: $encode_cross_crate,
type_: $typ,
safety: AttributeSafety::Unsafe,
safety: AttributeSafety::Unsafe { unsafe_since: None },
template: $tpl,
duplicates: $duplicates,
gate: Gated(Stability::Unstable, sym::$gate, $msg, Features::$gate),
Expand All @@ -228,7 +243,7 @@ macro_rules! gated {
name: sym::$attr,
encode_cross_crate: $encode_cross_crate,
type_: $typ,
safety: AttributeSafety::Unsafe,
safety: AttributeSafety::Unsafe { unsafe_since: None },
template: $tpl,
duplicates: $duplicates,
gate: Gated(Stability::Unstable, sym::$attr, $msg, Features::$attr),
Expand Down Expand Up @@ -423,9 +438,9 @@ pub static BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
),
ungated!(no_link, Normal, template!(Word), WarnFollowing, EncodeCrossCrate::No),
ungated!(repr, Normal, template!(List: "C"), DuplicatesOk, EncodeCrossCrate::No),
ungated!(unsafe export_name, Normal, template!(NameValueStr: "name"), FutureWarnPreceding, EncodeCrossCrate::No),
ungated!(unsafe link_section, Normal, template!(NameValueStr: "name"), FutureWarnPreceding, EncodeCrossCrate::No),
ungated!(unsafe no_mangle, Normal, template!(Word), WarnFollowing, EncodeCrossCrate::No),
ungated!(unsafe(Edition2024) export_name, Normal, template!(NameValueStr: "name"), FutureWarnPreceding, EncodeCrossCrate::No),
ungated!(unsafe(Edition2024) link_section, Normal, template!(NameValueStr: "name"), FutureWarnPreceding, EncodeCrossCrate::No),
ungated!(unsafe(Edition2024) no_mangle, Normal, template!(Word), WarnFollowing, EncodeCrossCrate::No),
ungated!(used, Normal, template!(Word, List: "compiler|linker"), WarnFollowing, EncodeCrossCrate::No),
ungated!(link_ordinal, Normal, template!(List: "ordinal"), ErrorPreceding, EncodeCrossCrate::Yes),

Expand Down
10 changes: 8 additions & 2 deletions compiler/rustc_parse/src/validate_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ fn is_attr_template_compatible(template: &AttributeTemplate, meta: &ast::MetaIte
pub fn check_attribute_safety(psess: &ParseSess, safety: AttributeSafety, attr: &Attribute) {
let attr_item = attr.get_normal_item();

if safety == AttributeSafety::Unsafe {
if let AttributeSafety::Unsafe { unsafe_since } = safety {
if let ast::Safety::Default = attr_item.unsafety {
let path_span = attr_item.path.span;

Expand All @@ -167,7 +167,13 @@ pub fn check_attribute_safety(psess: &ParseSess, safety: AttributeSafety, attr:
// square bracket respectively.
let diag_span = attr_item.span();

if attr.span.at_least_rust_2024() {
// Attributes can be safe in earlier editions, and become unsafe in later ones.
let emit_error = match unsafe_since {
None => true,
Some(unsafe_since) => attr.span.edition() >= unsafe_since,
};
Comment on lines +170 to +174
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is equivalent to Some(attr.span.edition) >= unsafe_since but that is very subtle so I wrote out the logic here. In general I don't love using Option here, but a custom enum would add a bunch of extra code and comes with its own naming problems.


if emit_error {
psess.dcx().emit_err(errors::UnsafeAttrOutsideUnsafe {
span: path_span,
suggestion: errors::UnsafeAttrOutsideUnsafeSuggestion {
Expand Down
2 changes: 1 addition & 1 deletion tests/codegen/cffi/ffi-const.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,6 @@ extern "C" {
// CHECK-LABEL: declare{{.*}}void @foo()
// CHECK-SAME: [[ATTRS:#[0-9]+]]
// CHECK-DAG: attributes [[ATTRS]] = { {{.*}}memory(none){{.*}} }
#[ffi_const]
#[unsafe(ffi_const)]
pub fn foo();
}
2 changes: 1 addition & 1 deletion tests/codegen/cffi/ffi-pure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,6 @@ extern "C" {
// CHECK-LABEL: declare{{.*}}void @foo()
// CHECK-SAME: [[ATTRS:#[0-9]+]]
// CHECK-DAG: attributes [[ATTRS]] = { {{.*}}memory(read){{.*}} }
#[ffi_pure]
#[unsafe(ffi_pure)]
pub fn foo();
}
2 changes: 1 addition & 1 deletion tests/ui/feature-gates/feature-gate-ffi_const.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![crate_type = "lib"]

extern "C" {
#[ffi_const] //~ ERROR the `#[ffi_const]` attribute is an experimental feature
#[unsafe(ffi_const)] //~ ERROR the `#[ffi_const]` attribute is an experimental feature
pub fn foo();
}
4 changes: 2 additions & 2 deletions tests/ui/feature-gates/feature-gate-ffi_const.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error[E0658]: the `#[ffi_const]` attribute is an experimental feature
--> $DIR/feature-gate-ffi_const.rs:4:5
|
LL | #[ffi_const]
| ^^^^^^^^^^^^
LL | #[unsafe(ffi_const)]
| ^^^^^^^^^^^^^^^^^^^^
|
= note: see issue #58328 <https://github.com/rust-lang/rust/issues/58328> for more information
= help: add `#![feature(ffi_const)]` to the crate attributes to enable
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/feature-gates/feature-gate-ffi_pure.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![crate_type = "lib"]

extern "C" {
#[ffi_pure] //~ ERROR the `#[ffi_pure]` attribute is an experimental feature
#[unsafe(ffi_pure)] //~ ERROR the `#[ffi_pure]` attribute is an experimental feature
pub fn foo();
}
4 changes: 2 additions & 2 deletions tests/ui/feature-gates/feature-gate-ffi_pure.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error[E0658]: the `#[ffi_pure]` attribute is an experimental feature
--> $DIR/feature-gate-ffi_pure.rs:4:5
|
LL | #[ffi_pure]
| ^^^^^^^^^^^
LL | #[unsafe(ffi_pure)]
| ^^^^^^^^^^^^^^^^^^^
|
= note: see issue #58329 <https://github.com/rust-lang/rust/issues/58329> for more information
= help: add `#![feature(ffi_pure)]` to the crate attributes to enable
Expand Down
11 changes: 7 additions & 4 deletions tests/ui/ffi-attrs/ffi_const.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
#![feature(ffi_const)]
#![crate_type = "lib"]

#[ffi_const] //~ ERROR `#[ffi_const]` may only be used on foreign functions
#[unsafe(ffi_const)] //~ ERROR `#[ffi_const]` may only be used on foreign functions
pub fn foo() {}

#[ffi_const] //~ ERROR `#[ffi_const]` may only be used on foreign functions
#[unsafe(ffi_const)] //~ ERROR `#[ffi_const]` may only be used on foreign functions
macro_rules! bar {
() => ()
() => {};
}

extern "C" {
#[ffi_const] //~ ERROR `#[ffi_const]` may only be used on foreign functions
#[unsafe(ffi_const)] //~ ERROR `#[ffi_const]` may only be used on foreign functions
static INT: i32;

#[ffi_const] //~ ERROR unsafe attribute used without unsafe
fn bar();
}
25 changes: 18 additions & 7 deletions tests/ui/ffi-attrs/ffi_const.stderr
Original file line number Diff line number Diff line change
@@ -1,21 +1,32 @@
error: unsafe attribute used without unsafe
--> $DIR/ffi_const.rs:16:7
|
LL | #[ffi_const]
| ^^^^^^^^^ usage of unsafe attribute
|
help: wrap the attribute in `unsafe(...)`
|
LL | #[unsafe(ffi_const)]
| +++++++ +

error[E0756]: `#[ffi_const]` may only be used on foreign functions
--> $DIR/ffi_const.rs:4:1
|
LL | #[ffi_const]
| ^^^^^^^^^^^^
LL | #[unsafe(ffi_const)]
| ^^^^^^^^^^^^^^^^^^^^

error[E0756]: `#[ffi_const]` may only be used on foreign functions
--> $DIR/ffi_const.rs:7:1
|
LL | #[ffi_const]
| ^^^^^^^^^^^^
LL | #[unsafe(ffi_const)]
| ^^^^^^^^^^^^^^^^^^^^

error[E0756]: `#[ffi_const]` may only be used on foreign functions
--> $DIR/ffi_const.rs:13:5
|
LL | #[ffi_const]
| ^^^^^^^^^^^^
LL | #[unsafe(ffi_const)]
| ^^^^^^^^^^^^^^^^^^^^

error: aborting due to 3 previous errors
error: aborting due to 4 previous errors

For more information about this error, try `rustc --explain E0756`.
4 changes: 2 additions & 2 deletions tests/ui/ffi-attrs/ffi_const2.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
#![feature(ffi_const, ffi_pure)]

extern "C" {
#[ffi_pure] //~ ERROR `#[ffi_const]` function cannot be `#[ffi_pure]`
#[ffi_const]
#[unsafe(ffi_pure)] //~ ERROR `#[ffi_const]` function cannot be `#[ffi_pure]`
#[unsafe(ffi_const)]
pub fn baz();
}

Expand Down
4 changes: 2 additions & 2 deletions tests/ui/ffi-attrs/ffi_const2.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error[E0757]: `#[ffi_const]` function cannot be `#[ffi_pure]`
--> $DIR/ffi_const2.rs:4:5
|
LL | #[ffi_pure]
| ^^^^^^^^^^^
LL | #[unsafe(ffi_pure)]
| ^^^^^^^^^^^^^^^^^^^

error: aborting due to 1 previous error

Expand Down
11 changes: 7 additions & 4 deletions tests/ui/ffi-attrs/ffi_pure.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
#![feature(ffi_pure)]
#![crate_type = "lib"]

#[ffi_pure] //~ ERROR `#[ffi_pure]` may only be used on foreign functions
#[unsafe(ffi_pure)] //~ ERROR `#[ffi_pure]` may only be used on foreign functions
pub fn foo() {}

#[ffi_pure] //~ ERROR `#[ffi_pure]` may only be used on foreign functions
#[unsafe(ffi_pure)] //~ ERROR `#[ffi_pure]` may only be used on foreign functions
macro_rules! bar {
() => ()
() => {};
}

extern "C" {
#[ffi_pure] //~ ERROR `#[ffi_pure]` may only be used on foreign functions
#[unsafe(ffi_pure)] //~ ERROR `#[ffi_pure]` may only be used on foreign functions
static INT: i32;

#[ffi_pure] //~ ERROR unsafe attribute used without unsafe
fn bar();
}
25 changes: 18 additions & 7 deletions tests/ui/ffi-attrs/ffi_pure.stderr
Original file line number Diff line number Diff line change
@@ -1,21 +1,32 @@
error: unsafe attribute used without unsafe
--> $DIR/ffi_pure.rs:16:7
|
LL | #[ffi_pure]
| ^^^^^^^^ usage of unsafe attribute
|
help: wrap the attribute in `unsafe(...)`
|
LL | #[unsafe(ffi_pure)]
| +++++++ +

error[E0755]: `#[ffi_pure]` may only be used on foreign functions
--> $DIR/ffi_pure.rs:4:1
|
LL | #[ffi_pure]
| ^^^^^^^^^^^
LL | #[unsafe(ffi_pure)]
| ^^^^^^^^^^^^^^^^^^^

error[E0755]: `#[ffi_pure]` may only be used on foreign functions
--> $DIR/ffi_pure.rs:7:1
|
LL | #[ffi_pure]
| ^^^^^^^^^^^
LL | #[unsafe(ffi_pure)]
| ^^^^^^^^^^^^^^^^^^^

error[E0755]: `#[ffi_pure]` may only be used on foreign functions
--> $DIR/ffi_pure.rs:13:5
|
LL | #[ffi_pure]
| ^^^^^^^^^^^
LL | #[unsafe(ffi_pure)]
| ^^^^^^^^^^^^^^^^^^^

error: aborting due to 3 previous errors
error: aborting due to 4 previous errors

For more information about this error, try `rustc --explain E0755`.
Loading