-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Unsafe derives and attributes #3715
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
Open
joshtriplett
wants to merge
21
commits into
rust-lang:master
Choose a base branch
from
joshtriplett:unsafe-derives
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+188
−0
Open
Changes from all commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
0530e44
Unsafe derives and attributes
joshtriplett aaeab26
RFC 3715
joshtriplett 41b758b
Mention alternative syntax and explain why we don't use it
joshtriplett 6e054cc
Unresolved question for unsafe placement
joshtriplett 7a3e7ab
Resolve the unresolved question about unsafe placement, and document …
joshtriplett 09fd6b2
Fix SAFETY comments to be non-doc comments
joshtriplett 82c2ad5
Add another rationale for `derive(unsafe(UnsafeTrait))`, from TC
joshtriplett 962e17c
Switch language to reference derive macros rather than traits
joshtriplett 2ecc61d
More wording updates from "trait" to "derive macro"
joshtriplett 0c7684f
Add alternative for restricting to one derive macro per `unsafe(deriv…
joshtriplett dfad26f
Flag order-dependent derives as an unusual case
joshtriplett 309cb97
Add alternative for declaring with `proc_macro_derive(unsafe(Dangerou…
joshtriplett bbed33f
Rewrite descriptions of SAFETY comments
joshtriplett 6b5499c
Add note about rustfmt
joshtriplett 04caa41
Mention and discuss alternative of unsafe supertrait
joshtriplett 2b4e22b
Change syntax to `proc_macro_derive(unsafe(DangerousDeriveMacro))`
joshtriplett dd1c9f0
Expand motivation
joshtriplett 102223a
Reorder attributes before derives
joshtriplett fcc7e78
Add unsafe helper attributes
joshtriplett 0549114
Fix typo
joshtriplett 0c8a3cb
RFC 3715: Expand motivation
joshtriplett File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,188 @@ | ||
- Feature Name: `unsafe_derives_and_attrs` | ||
- Start Date: 2024-10-22 | ||
- RFC PR: [rust-lang/rfcs#3715](https://github.com/rust-lang/rfcs/pull/3715) | ||
- Rust Issue: [rust-lang/rust#0000](https://github.com/rust-lang/rust/issues/0000) | ||
|
||
# Summary | ||
[summary]: #summary | ||
|
||
Allow declaring proc macro attributes and derive macros as unsafe, and | ||
requiring `unsafe` to invoke them. | ||
|
||
# Motivation | ||
[motivation]: #motivation | ||
|
||
Some traits place requirements on implementations that the Rust compiler cannot | ||
verify. Those traits can mark themselves as unsafe, requiring `unsafe impl` | ||
syntax to implement. However, trait `derive` macros cannot currently require | ||
`unsafe`. This RFC defines a syntax for declaring and using unsafe `derive` | ||
macros. | ||
|
||
This provides value for any derive of an `unsafe trait`, ranging from standard | ||
library unsafe traits such as `Send` and `Sync` to more complex unsafe traits | ||
in the ecosystem. With this mechanism available, an `unsafe trait` has the | ||
option of providing two different kinds of `derive` macros: a safe `derive` | ||
macro that implements the `unsafe` trait in a fashion that's always safe (or | ||
that fails if some obligation is not met), or an `unsafe` `derive` macro that | ||
puts the safety obligation on the invoker of the `derive`. | ||
|
||
Some examples of potential unsafe derives for `unsafe trait`s: | ||
|
||
- `TrustedLen` and `DerefPure` in the standard library. (Currently unstable, | ||
but such a derive would be useful when they're stable, serving the function | ||
of an `unsafe impl`.) | ||
- `pyo3::marker::Ungil` in `pyo3`, in place of the current handling of a | ||
blanket impl for any `Send` type. | ||
- A hypothetical derive for `bytes::Buf` or similar. (For cases where a type | ||
has fields or trivial expressions corresponding to the current slice and | ||
offset.) | ||
|
||
This RFC also defines a syntax for declaring proc macro attributes as unsafe. | ||
|
||
# Guide-level explanation | ||
[guide-level-explanation]: #guide-level-explanation | ||
|
||
## Attributes | ||
|
||
When declaring a proc macro attribute, you can add the `unsafe` parameter to | ||
the `proc_macro_attribute` attribute to indicate that the attribute requires | ||
`unsafe`: | ||
|
||
```rust | ||
#[proc_macro_attribute(unsafe)] | ||
pub fn dangerous(_attr: TokenStream, item: TokenStream) -> TokenStream { | ||
item | ||
} | ||
``` | ||
|
||
Invoking an unsafe attribute requires the unsafe attribute syntax: | ||
`#[unsafe(dangerous)]`. | ||
|
||
When writing a `SAFETY` comment for each `unsafe`, you can place the `SAFETY` | ||
comment immediately prior to the attribute: | ||
|
||
```rust | ||
// SAFETY: ... | ||
#[unsafe(dangerous)] | ||
``` | ||
|
||
## Derives | ||
|
||
When declaring a proc macro `derive`, you can use the following syntax to | ||
indicate that the derive requires `unsafe`: | ||
|
||
```rust | ||
#[proc_macro_derive(unsafe(DangerousDeriveMacro)] | ||
pub fn derive_dangerous_derive_macro(_item: TokenStream) -> TokenStream { | ||
TokenStream::new() | ||
} | ||
``` | ||
|
||
Invoking this derive macro requires writing | ||
`#[derive(unsafe(DangerousDeriveMacro))]`. Invoking an unsafe derive macro | ||
without the unsafe derive syntax will produce a compiler error. Using the | ||
unsafe derive syntax without an unsafe derive macro will trigger an "unused | ||
unsafe" lint. | ||
|
||
A `proc_macro_derive` attribute can include both `attributes` for helper | ||
attributes and `unsafe` to declare the derive unsafe, in any order. | ||
|
||
When writing a `SAFETY` comment for each `unsafe`, you can place the `SAFETY` | ||
comment either prior to the derive (for a single unsafe derive) or prior to the | ||
specific `unsafe(DangerousDeriveMacro)` in a list of derives: | ||
|
||
```rust | ||
// SAFETY: ... | ||
#[derive(unsafe(DangerousDeriveMacro))] | ||
struct SomeStruct { ... } | ||
|
||
#[derive( | ||
// SAFETY: ... | ||
unsafe(DangerousDeriveMacro), | ||
// SAFETY: ... | ||
unsafe(AnotherDangerousDeriveMacro), | ||
)] | ||
struct AnotherStruct { ... } | ||
``` | ||
|
||
(Note that current rustfmt will place every derive on a line of its own if any | ||
have a comment. That could be changed in a future style edition, but this RFC | ||
is not making or advocating any style proposals.) | ||
|
||
### Helper attributes | ||
|
||
A `derive` macro can have helper attributes. You can use the following syntax | ||
to declare a helper attribute as `unsafe`: | ||
|
||
```rust | ||
#[proc_macro_derive(MyDeriveMacro, attributes(unsafe(dangerous_helper_attr))] | ||
pub fn derive_my_derive_macro(_item: TokenStream) -> TokenStream { | ||
TokenStream::new() | ||
} | ||
``` | ||
|
||
Invoking this helper attribute requires the unsafe attribute syntax: | ||
`#[unsafe(dangerous_helper_attr)]`. | ||
|
||
Any combination of safe and unsafe attributes are allowed in both safe and | ||
unsafe derive macros. | ||
|
||
# Rationale and alternatives | ||
[rationale-and-alternatives]: #rationale-and-alternatives | ||
|
||
This RFC proposes the synax `#[derive(unsafe(DangerousDeriveMacro))]`. We | ||
could, instead, put the `unsafe` on the outside: | ||
`#[unsafe(derive(DangerousDeriveMacro))]`. | ||
|
||
Some rationale for putting it on the inside: | ||
- This encourages minimizing the scope of the `unsafe`, isolating it to a | ||
single derive macro. | ||
- This allows writing all derive macros to invoke within a single | ||
`#[derive(...)]`, if desired. Putting the `unsafe` on the outside requires | ||
separate `derive`s for safe and unsafe derives, and potentially multiple in | ||
the (unusual) case where the derives care about ordering. | ||
- This makes it easy to attach `SAFETY` comments to each individual derive | ||
macro. | ||
joshtriplett marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- One way to think of `derive(Macro)` is that `derive(..)` enters a context in | ||
which one or more derive macros can be invoked, and naming `Macro` is how we | ||
actually invoke the derive macro. When invoking unsafe derive macros, we have | ||
to wrap those with `unsafe(..)` as in `derive(unsafe(DangerousDeriveMacro))`. | ||
|
||
We could, *if* we used the `unsafe(derive(...))` syntax, additionally restrict | ||
such derives to only contain a single trait, forcing the developer to only | ||
invoke a single derive macro per `unsafe(derive(...))`. However, this syntax | ||
naturally leads people to assume this would work, only to encounter an error | ||
when they do the natural thing that seems like it should work. | ||
|
||
We could use a different syntax for invoking unsafe derives, such as | ||
`derive(unsafe DangerousDeriveMacro)`. However, that would be inconsistent with | ||
unsafe attributes (which use parentheses), *and* it has the potential to look | ||
like a modifier to `DangerousDeriveMacro` (e.g. an unsafe version of | ||
`DangerousDeriveMacro`), particularly in the common case where | ||
`DangerousDeriveMacro` has the same name as a trait. | ||
|
||
We could use a different syntax for declaring unsafe derives, such as | ||
`proc_macro_derive(DangerousDeriveMacro, unsafe)`. This would have the | ||
advantage of not looking like the definition incurs an unsafe obligation, but | ||
the disadvantage of using a different syntax for definition and use. | ||
|
||
If we didn't have this feature, a workaround trait authors could use for the | ||
specific case of a derive macro implementing a trait is to have a separate | ||
marker trait as a supertrait of the unsafe trait. Then, | ||
`derive(DangerousTrait)` could require separately doing `unsafe impl | ||
PrerequisiteForDangerousTrait`. This would achieve the goal of requiring | ||
`unsafe` to appear somewhere when deriving the trait, but would not tie the two | ||
together as directly or clearly. | ||
|
||
# Prior art | ||
[prior-art]: #prior-art | ||
|
||
RFC 3325 defined unsafe attributes. This RFC provides a natural extension of | ||
that mechanism to derives. | ||
|
||
# Future possibilities | ||
[future-possibilities]: #future-possibilities | ||
|
||
When we add support for `macro_rules!`-based attributes and derives, we should | ||
provide a means for such attributes and derives to declare themselves unsafe as | ||
well. |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If these are the only motivation examples, I don't get why these marker traits
TrustedLen
,TrustedStep
,DerefPure
,Ungil
require using the#[derive]
mechanism over manualunsafe impl
.For the first three the advantages are not having to explicitly write the
where
bounds, but they have theIterator
orDeref
supertraits which cannot be derived so you gotta repeat those bounds anyway.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Literally any
unsafe trait
that one might wish toderive
is a potential example. These were a few samples. If you have other examples ofunsafe trait
that you'd prefer to have cited instead of or in addition to these, I'd be happy to add them.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Normally if we wish to
derive
something it is because we know that it is possible to fill out theimpl
programmatically with the type's definition alone, thus saving effort to produce the boilerplate impl.If all the derive doing is just implementing a marker trait without anything extra there's certainly no point from the supply-side anyone wish to write a proc-macro for it. At least it should be derivable together with its base trait like
#[derive(Clone, Copy)]
i.e.#[derive(Iterator, unsafe(TrustedLen))]
, but you can't & can't derive an Iterator.Another reason we wish to
derive
a marker trait is to insert some compile-time checks, such aszerocopy
's macros. Butzerocopy
's derive-macro checks at the same proved the conditions required by theunsafe trait
in the first place, making those traits safe to derive even if unsafe to manually implement.For
libstd
the public, documented, non-sealed unsafe traits are:Perhaps
CloneToUninit
is the one most compatible with#[derive]
:but the actual impl can satisfy
impl CloneToUninit
's safety condition without user's intervention, meaning#[derive(CloneToUninit)]
itself is actually safe.So no I don't have any other examples to support this RFC, i.e. I'm still on the side of the concern "needs fleshed out use case".