Implements the changes for Wakers that are currently discussed in the tracking issue for stabilization #1340
Conversation
This depends on changes to libcore and libstd to work.
|
I already upload it here, so that people can take a look on the impact of the API changes. However I'm not sure why currently |
Matthias247
changed the title
There was a problem hiding this comment.
Assorted places I think unsafe is unneeded. While skimming this PR I noticed that there are places where unsafe { ... } is used; I think generally, uses of unsafe { ... } should come with a comment containing an informal proof of safety. This is especially true for bits and pieces that are going to be included in the standard library.
| pub fn new() -> Self { | ||
| Self { _reserved: () } | ||
| } | ||
| unsafe fn noop(_data: *const()) { |
There was a problem hiding this comment.
| unsafe fn noop(_data: *const()) { | |
| fn noop(_data: *const()) { |
You should be able to remove unsafe here because of this coercion between fn and unsafe fn:
fn noop(_data: *const()) {}
const PTR: unsafe fn(*const ()) = noop;
There was a problem hiding this comment.
Interesting, so there’s a coercion from fn to unsafe fn and from environment-less-closure to fn, but not the combined environment-less-closure to unsafe fn: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=eaec9c9f080c999a55a08dd2184d8669. Has there been any discussion of adding the latter?
There was a problem hiding this comment.
I think that's just due to the lack of transitive coercions in general. It is not particular to this case AFAIK.
There was a problem hiding this comment.
We generally accept PRs for these without FCP since the original RFC specified that we should include all transitive coercions, but there isn't a mechanism in the compiler for that today.
| #[derive(Debug)] | ||
| pub struct PanicWake { | ||
| _reserved: (), | ||
| unsafe fn noop_clone(_data: *const()) -> RawWaker { |
There was a problem hiding this comment.
| unsafe fn noop_clone(_data: *const()) -> RawWaker { | |
| fn noop_clone(_data: *const()) -> RawWaker { |
ditto
| fn default() -> Self { | ||
| Self::new() | ||
| } | ||
| unsafe fn wake_panic(_data: *const()) { |
There was a problem hiding this comment.
| unsafe fn wake_panic(_data: *const()) { | |
| fn wake_panic(_data: *const()) { |
ditto
| fn wake(_arc_self: &Arc<Self>) { | ||
| panic!("should not be woken") | ||
| } | ||
| unsafe fn noop_into_waker(_data: *const()) -> Option<RawWaker> { |
There was a problem hiding this comment.
| unsafe fn noop_into_waker(_data: *const()) -> Option<RawWaker> { | |
| fn noop_into_waker(_data: *const()) -> Option<RawWaker> { |
ditto
| LocalWakerRef::new(local_waker) | ||
| } | ||
|
|
||
| unsafe fn noop(_data: *const()) { |
There was a problem hiding this comment.
| unsafe fn noop(_data: *const()) { | |
| fn noop(_data: *const()) { |
ditto
| #[derive(Debug)] | ||
| struct NoopWake { | ||
| _reserved: (), | ||
| unsafe fn noop_clone(_data: *const()) -> RawWaker { |
There was a problem hiding this comment.
| unsafe fn noop_clone(_data: *const()) -> RawWaker { | |
| fn noop_clone(_data: *const()) -> RawWaker { |
ditto
| unsafe fn drop_raw(&self) {} | ||
|
|
||
| unsafe fn wake(&self) {} | ||
| unsafe fn noop(_data: *const()) { |
There was a problem hiding this comment.
| unsafe fn noop(_data: *const()) { | |
| fn noop(_data: *const()) { |
ditto
| fn noop_unsafe_wake() -> NonNull<dyn UnsafeWake> { | ||
| static mut INSTANCE: NoopWake = NoopWake { _reserved: () }; | ||
| unsafe { NonNull::new_unchecked(&mut INSTANCE as *mut dyn UnsafeWake) } | ||
| unsafe fn noop_into_waker(_data: *const()) -> Option<RawWaker> { |
There was a problem hiding this comment.
| unsafe fn noop_into_waker(_data: *const()) -> Option<RawWaker> { | |
| fn noop_into_waker(_data: *const()) -> Option<RawWaker> { |
|
@Matthias247 can be closed now? |
|
Merged in a different CR |
This depends on changes to libcore and libstd to work.
See discussion in rust-lang/rfcs#2592 and proposed update in
aturon/rfcs#15