-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Let Option derive #[must_use]
#3906
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
base: master
Are you sure you want to change the base?
Conversation
|
Is there some reason #[must_use] should not be a marker trait? |
|
I would say that actually must_use is more a property of a particular function, not really of types, and sticking it on types is a shorthanded way to put it on a lot of functions "for free" cognitively. I'd say it's best if we, as an ecosystem, stick to having must_use on functions/methods rather than types. |
So #[must_use] could be a marker trait on the function's type where it is applied (each function has a unique type) and on each type it is applied. Then you add a blanked impl on functions whenever their output type impls the trait. (it could be two traits, one for types you apply #[must_use] and another for functions, so that returning the function doesn't trigger it) |
Yes, but
Perhaps, but that would be quite a significant revision. Definitely out-of-scope for this RFC. |
|
Agreed! For this RFC a trait is not necessary. If extending this feature to custom types is desired, a trait would be useful |
|
Note that it is already possible to mark a trait as |
| # Guide-level explanation | ||
| [guide-level-explanation]: #guide-level-explanation | ||
|
|
||
| The `Option` and `Box` types will "magically" have the `#[must_use]` attribute if and only if their generic parameter `T` does. |
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.
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.
impl<T: MustUse> MustUse for Option {}
#[must_use] is not currently a trait. If you want to make an RFC for that, please go ahead. It may be a good idea, but it is an idea for another RFC, not this one.
Since #[must_use] is not currently a trait, we lack the language to describe derive behaviour, hence use of "magically" here.
Option<#[must_use] T>
Use of #[must_use] on parameters is novel syntax that I don't particularly like.
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.
sorry, i thought i was clear in the original comment that "the MustUse trait" was mentioned purely as an analogy, and not as a normative suggestion, but perhaps not. fwiw i think a MustUse trait would be bad since
- trait solver is slow and heavy compared to just looking at some simple hard-coded rules based on named types/traits.
- it gives perhaps too much flexibility to the user. do we really want people to be able to write
impl<T: Copy> MustUse for Thing<T>, for instance.
i think users should have access to more fine-grained #[must_use] annotations, but that doesn't per-se have to be part of this change since Option and Box alone would be clear usability improvements in my book.
i don't want to overlitigate, but i still reckon this RFC doesn't really justify the "magic" design as written: it's clearly conceptually easier to special case these two generic types, but i don't see it as better from a maintenance perspective if we do want to rip it out for something holistic. for instance is_ty_must_use is already more complicated than ideally it should be (sidebar: and, seemingly, incomplete; see this playground). should we keep adding additional branches ad infinitum?
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.
Use of
#[must_use]on parameters is novel syntax that I don't particularly like.
Attribute on type parameter is already in use since #3621 (ref), it is no longer a novel syntax.
#[derive(CoercePointee)]
#[repr(transparent)]
struct MySmartPointer<#[pointee] T: ?Sized, U> {
ptr: Box<T>,
_phantom: PhantomData<U>,
}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.
Option<#[must_use] T>
Something like this would be clearer (though I do find derive's "magic" handling of generic parameters unfortunate relative to making this explicit, as with autoimpl):
#[derive(must_use)]
enum Option<T> { /* ... */ }That may be besides the point though if this RFC adds more complexity than value. (I had no idea that Result<T, Infallible> would not trigger a "must use" warning.)
|
Since there have been a number of suggestions about how to generalise this RFC, let me be clear: this RFC is extremely specific on purpose:
|
|
It seems strange to me for it to magically apply to these two types, without a more general mechanism. And why not Result? Or Pin? |
|
Afaik, no pointer types are |
Let
OptionandBoxderive#[must_use]from their generic parameterT.Rendered