Skip to content
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

Tracking Issue for RFC 3325: unsafe attributes #123757

Closed
8 tasks done
traviscross opened this issue Apr 10, 2024 · 12 comments
Closed
8 tasks done

Tracking Issue for RFC 3325: unsafe attributes #123757

traviscross opened this issue Apr 10, 2024 · 12 comments
Assignees
Labels
A-edition-2024 Area: The 2024 edition B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. F-unsafe_attributes `#![feature(unsafe_attributes)]` L-unsafe_attr_outside_unsafe Lint: unsafe_attr_outside_unsafe S-tracking-ready-for-edition Status: This issue is ready for inclusion in the edition. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@traviscross
Copy link
Contributor

traviscross commented Apr 10, 2024

This is a tracking issue for RFC 3325: unsafe attributes

RFC: rust-lang/rfcs#3325
Issues: F-unsafe_attributes `#![feature(unsafe_attributes)]`

The feature gate for the issue is #![feature(unsafe_attributes)].

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

About tracking issues

Tracking issues are used to record the overall progress of implementation. They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions. A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature. Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

Steps

Unresolved Questions

  • Different lint staging. The lint on using existing unsafe attributes like no_mangle outside unsafe(...) could be staged in various ways: it could be warn-by-default to start or we wait a while before to do that, it could be edition-dependent, it might eventually be deny-by-default or even a hard error on some editions -- there are lots of details here, which can be determined later during the process.

Implementation history

cc @RalfJung @carbotaniuman @rust-lang/style

This issue has been assigned to @carbotaniuman via this comment.

@traviscross traviscross added T-lang Relevant to the language team, which will review and decide on the PR/issue. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. A-edition-2024 Area: The 2024 edition B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. labels Apr 10, 2024
@traviscross
Copy link
Contributor Author

@rustbot assign @carbotaniuman

@carbotaniuman has volunteered to own this item to get it over the line for the edition. Huge thanks for that.

@rustbot rustbot self-assigned this Apr 21, 2024
bors added a commit to rust-lang-ci/rust that referenced this issue May 8, 2024
…<try>

Parse unsafe attributes

Initial parse implementation for rust-lang#123757

This is the initial work to parse unsafe attributes, which is represented as an extra `unsafety` field in `MetaItem` and `AttrItem`. There's two areas in the code where it appears that parsing is done manually and not using the parser stuff, and I'm not sure how I'm supposed to thread the change there.
@traviscross
Copy link
Contributor Author

@rustbot labels +I-style-nominated

There's probably not a lot for T-style to do here, but strictly speaking, this is a kind of new syntax, so it may be worth at least a short discussion for visibility.

@rustbot rustbot added the I-style-nominated Nominated for discussion during a style team meeting. label May 14, 2024
@traviscross traviscross added S-tracking-impl-incomplete Status: The implementation is incomplete. S-tracking-needs-migration-lint Status: This item needs a migration lint. S-tracking-needs-documentation Status: Needs documentation. labels May 21, 2024
@traviscross
Copy link
Contributor Author

@rustbot labels -I-style-nominated

We discussed this in the style team meeting today and confirmed that this will not require any special handling or changes to the style guide.

@rustbot rustbot removed the I-style-nominated Nominated for discussion during a style team meeting. label Jun 5, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jun 7, 2024
…r=michaelwoerister

Parse unsafe attributes

Initial parse implementation for rust-lang#123757

This is the initial work to parse unsafe attributes, which is represented as an extra `unsafety` field in `MetaItem` and `AttrItem`. There's two areas in the code where it appears that parsing is done manually and not using the parser stuff, and I'm not sure how I'm supposed to thread the change there.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jun 7, 2024
…r=michaelwoerister

Parse unsafe attributes

Initial parse implementation for rust-lang#123757

This is the initial work to parse unsafe attributes, which is represented as an extra `unsafety` field in `MetaItem` and `AttrItem`. There's two areas in the code where it appears that parsing is done manually and not using the parser stuff, and I'm not sure how I'm supposed to thread the change there.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jun 7, 2024
…r=michaelwoerister

Parse unsafe attributes

Initial parse implementation for rust-lang#123757

This is the initial work to parse unsafe attributes, which is represented as an extra `unsafety` field in `MetaItem` and `AttrItem`. There's two areas in the code where it appears that parsing is done manually and not using the parser stuff, and I'm not sure how I'm supposed to thread the change there.
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jun 7, 2024
Rollup merge of rust-lang#124214 - carbotaniuman:parse_unsafe_attrs, r=michaelwoerister

Parse unsafe attributes

Initial parse implementation for rust-lang#123757

This is the initial work to parse unsafe attributes, which is represented as an extra `unsafety` field in `MetaItem` and `AttrItem`. There's two areas in the code where it appears that parsing is done manually and not using the parser stuff, and I'm not sure how I'm supposed to thread the change there.
@traviscross traviscross removed the S-tracking-needs-migration-lint Status: This item needs a migration lint. label Jun 8, 2024
@jieyouxu jieyouxu added L-unsafe_attr_outside_unsafe Lint: unsafe_attr_outside_unsafe F-unsafe_attributes `#![feature(unsafe_attributes)]` labels Jun 9, 2024
@ehuss
Copy link
Contributor

ehuss commented Jun 20, 2024

Edition docs are up at rust-lang/edition-guide#308.

workingjubilee added a commit to workingjubilee/rustc that referenced this issue Jun 24, 2024
…r=jieyouxu

Add hard error and migration lint for unsafe attrs

More implementation work for rust-lang#123757

This adds the migration lint for unsafe attributes, as well as making it a hard error in Rust 2024.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jun 24, 2024
…r=jieyouxu

Add hard error and migration lint for unsafe attrs

More implementation work for rust-lang#123757

This adds the migration lint for unsafe attributes, as well as making it a hard error in Rust 2024.
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jun 24, 2024
Rollup merge of rust-lang#126177 - carbotaniuman:unsafe_attr_errors, r=jieyouxu

Add hard error and migration lint for unsafe attrs

More implementation work for rust-lang#123757

This adds the migration lint for unsafe attributes, as well as making it a hard error in Rust 2024.
@traviscross traviscross added S-tracking-ready-to-stabilize Status: This is ready to stabilize; it may need a stabilization report and a PR and removed S-tracking-impl-incomplete Status: The implementation is incomplete. labels Jun 25, 2024
@traviscross traviscross removed the S-tracking-needs-documentation Status: Needs documentation. label Jul 22, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jul 30, 2024
…ification, 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:

- rust-lang#123757
bors added a commit to rust-lang-ci/rust that referenced this issue Aug 1, 2024
…ication, 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:

- rust-lang#123757
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Aug 2, 2024
…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:

- rust-lang/rust#123757
lnicola pushed a commit to lnicola/rust-analyzer that referenced this issue Aug 13, 2024
…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:

- rust-lang/rust#123757
tgross35 added a commit to tgross35/rust that referenced this issue Aug 17, 2024
…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`.
bors added a commit to rust-lang-ci/rust that referenced this issue Aug 17, 2024
… 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`.
@traviscross traviscross added S-tracking-ready-for-edition Status: This issue is ready for inclusion in the edition. and removed S-tracking-ready-to-stabilize Status: This is ready to stabilize; it may need a stabilization report and a PR labels Aug 20, 2024
@traviscross
Copy link
Contributor Author

traviscross commented Aug 20, 2024

@rustbot labels +S-tracking-ready-for-edition

We reviewed this in the edition call today. This item is ready for Rust 2024.

Thanks to @carbotaniuman for pushing forward this item. Thanks to @RalfJung for writing up the RFC. And thanks to the many reviewers.

@RalfJung
Copy link
Member

RalfJung commented Aug 28, 2024

Let's reopen this to track documentation: somewhere around here, it should document what the safety requirements are for these attributes:

  • no_mangle
  • link_section
  • export_name

@RalfJung RalfJung reopened this Aug 28, 2024
@ehuss
Copy link
Contributor

ehuss commented Aug 28, 2024

@RalfJung That documentation has already been updated by rust-lang/reference#1539. Can you say if that covers what you are expecting?

@geofft
Copy link
Contributor

geofft commented Aug 28, 2024

I think we could add a sentence like "It may also lead to addresses being accessed with the wrong address space, for instance, on Harvard architectures" to address #76507 specifically, but I think "place data and code into sections
of memory not expecting them" sort of covers that already.

@RalfJung
Copy link
Member

Ah, fair, yes that should suffice for now. :)

@arthuro555

This comment was marked as off-topic.

@RalfJung

This comment was marked as resolved.

compiler-errors pushed a commit to compiler-errors/rust that referenced this issue Sep 16, 2024
… 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`.
@RalfJung
Copy link
Member

Turns out there's some follow-up cleanup we can do, now that we have a proper notion of "unsafe attributes": #131801

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-edition-2024 Area: The 2024 edition B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. F-unsafe_attributes `#![feature(unsafe_attributes)]` L-unsafe_attr_outside_unsafe Lint: unsafe_attr_outside_unsafe S-tracking-ready-for-edition Status: This issue is ready for inclusion in the edition. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants