search

rust-lang / rfcs

RFCs for changes to Rust
https://rust-lang.github.io/rfcs/
Apache License 2.0
5.97k stars 1.57k forks source link

Unsafe derives and attributes #3715

Open joshtriplett opened 1 month ago

joshtriplett commented 1 month ago

Allow declaring proc macro attributes and derive macros as unsafe, and requiring unsafe to invoke them.

Rendered

clarfonthey commented 1 month ago

Just my 2¢, but I think that the shorthand for derive(..., unsafe(Trait), ...) is a bit unprecedented.

You have to separate out derive traits any time there's some different requirement, e.g. cfg_attr(feature = "serde", derive(Serialize, Deserialize)), and by keeping unsafe at the top level you can easily grep for #!?\[unsafe\( whereas unsafe( will catch any call to method_unsafe(.

Sure, it's likely it won't make a difference, but I think that only having to check attributes at the top level for unsafe to verify safety is best.

carbotaniuman commented 1 month ago

The greppability is already broken by things like #[cfg_attr(all(), unsafe(no_mangle)], and can be restored by just grepping unsafe( I think putting the unsafe with the trait makes syntactic sense as it discharges the safety obligation of each trait derive.

GnomedDev commented 1 month ago

Any reason the syntax for defining an unsafe derive/attribute isn't unsafe fn?

#[proc_macro_attribute]
pub unsafe fn dangerous(_attr: TokenStream, item: TokenStream) -> TokenStream {
    item
}
Noratrieb commented 1 month ago

because that makes no sense, the unsafety is at a meta-level and there are no safety requirements on the macro function itself

GnomedDev commented 1 month ago

Is there not? I thought the point of an unsafe derive/attribute was that it was unsafe to trigger and had safety requirements?

clarfonthey commented 1 month ago

The greppability is already broken by things like #[cfg_attr(all(), unsafe(no_mangle)], and can be restored by just grepping unsafe( I think putting the unsafe with the trait makes syntactic sense as it discharges the safety obligation of each trait derive.

I feel kind of silly for literally alluding to this point in my post and missing it somehow. You're right and I retract my original claim.

joshtriplett commented 5 days ago

We had a @rust-lang/lang design meeting today on the set of macro RFCs. I've updated the RFC to incorporate the feedback from that design meeting.

Per the feedback in that meeting, I'm starting an FCP to start letting people register consensus for this RFC.

@rfcbot merge

rfcbot commented 5 days ago

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns. See this document for info about what commands tagged team members can give me.