-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Make LocalWaker::new a const fn #53456
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
Conversation
@@ -119,7 +119,7 @@ impl LocalWaker { | |||
/// For this function to be used safely, it must be sound to call `inner.wake_local()` | |||
/// on the current thread. | |||
#[inline] | |||
pub unsafe fn new(inner: NonNull<dyn UnsafeWake>) -> Self { | |||
pub const unsafe fn new(inner: NonNull<dyn UnsafeWake>) -> Self { |
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.
Please add the rustc_const_unstable
attribute and a test enabling the feature and using it.
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.
See #47562 for an example.
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.
Yep, I managed to get an example going. Unfortunately writing that example made me realise that this isn't usable for what I want anyway since it's !Send + !Sync
so can't go in a static, as I don't know of any other usecases for this to be const
I'm gonna close this and try to find an alternative.
One thing I noticed while adding the example, rustc_const_unstable
requires a stable
/unstable
directly on the item being annotated, LocalWaker
is in an unstable
module so doesn't have a direct annotation, is there some reason the attribute needs to be directly on the item instead of inheriting from its parent scope?
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.
Is there some reason the attribute needs to be directly on the item instead of inheriting from its parent scope?
Does implementation lazyness count as a reason?
Most newly added const fns already had a stability attribute, so it was less hazzle to add an attribute to those that didn't have it than to correctly implement the attribute
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.
Implementation lazyness is the best reason.
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.
<Joke about Haskell has been elided due to laziness>
This is mostly not that useful, except for
futures-test
where we want to have statics available for the more trivial test wakers for better ergonomics.r? @cramertj