Skip to content
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

Implement RawWaker and Waker getters for underlying pointers #91828

Merged
merged 1 commit into from
Feb 1, 2022

Conversation

oxalica
Copy link
Contributor

@oxalica oxalica commented Dec 12, 2021

implement #87021

New APIs:

  • RawWaker::data(&self) -> *const ()
  • RawWaker::vtable(&self) -> &'static RawWakerVTable
  • Waker::as_raw_waker(&self) -> &RawWaker Waker::as_raw(&self) -> &RawWaker

This third one is an auxiliary function to make the two APIs above more useful. Since we can only get &Waker in Future::poll, without this, we need to transmute it into &RawWaker (relying on repr(transparent)) in order to access its data/vtable pointers.

Not sure if it should be named as_raw or as_raw_waker. Seems we always use as_<something-raw> instead of just as_raw. But as_raw_waker seems not quite consistent with Waker::from_raw. As suggested in #91828 (comment), use as_raw.

@rust-highfive
Copy link
Collaborator

r? @dtolnay

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 12, 2021
@oxalica
Copy link
Contributor Author

oxalica commented Dec 12, 2021

r? @joshtriplett

@apiraino apiraino added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Dec 14, 2021
Copy link
Member

@nrc nrc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Looks good to me with the two minor comments addressed.

library/core/src/task/wake.rs Outdated Show resolved Hide resolved
library/core/src/task/wake.rs Outdated Show resolved Hide resolved
Copy link
Member

@nrc nrc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes!

@nrc
Copy link
Member

nrc commented Dec 17, 2021

@joshtriplett @tmandry or @cuviper y'all approved of this plan in #87021, this PR lgtm, could one of you take a look and give it the official mark of approval please?

@dtolnay dtolnay added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 20, 2021
@tmandry
Copy link
Member

tmandry commented Jan 11, 2022

LGTM. Seems like what was approved in #87021 (comment) plus the Waker::as_raw() method. Does someone from @rust-lang/libs-api want to r+?

@dtolnay
Copy link
Member

dtolnay commented Jan 31, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Jan 31, 2022

📌 Commit bae0da8 has been approved by dtolnay

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 31, 2022
@dtolnay dtolnay assigned dtolnay and unassigned joshtriplett Jan 31, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 31, 2022
Implement `RawWaker` and `Waker` getters for underlying pointers

implement rust-lang#87021

New APIs:
- `RawWaker::data(&self) -> *const ()`
- `RawWaker::vtable(&self) -> &'static RawWakerVTable`
- ~`Waker::as_raw_waker(&self) -> &RawWaker`~ `Waker::as_raw(&self) -> &RawWaker`

This third one is an auxiliary function to make the two APIs above more useful. Since we can only get `&Waker` in `Future::poll`, without this, we need to `transmute` it into `&RawWaker` (relying on `repr(transparent)`) in order to access its data/vtable pointers.

~Not sure if it should be named `as_raw` or `as_raw_waker`. Seems we always use `as_<something-raw>` instead of just `as_raw`. But `as_raw_waker` seems not quite consistent with `Waker::from_raw`.~ As suggested in rust-lang#91828 (comment), use `as_raw`.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 31, 2022
Implement `RawWaker` and `Waker` getters for underlying pointers

implement rust-lang#87021

New APIs:
- `RawWaker::data(&self) -> *const ()`
- `RawWaker::vtable(&self) -> &'static RawWakerVTable`
- ~`Waker::as_raw_waker(&self) -> &RawWaker`~ `Waker::as_raw(&self) -> &RawWaker`

This third one is an auxiliary function to make the two APIs above more useful. Since we can only get `&Waker` in `Future::poll`, without this, we need to `transmute` it into `&RawWaker` (relying on `repr(transparent)`) in order to access its data/vtable pointers.

~Not sure if it should be named `as_raw` or `as_raw_waker`. Seems we always use `as_<something-raw>` instead of just `as_raw`. But `as_raw_waker` seems not quite consistent with `Waker::from_raw`.~ As suggested in rust-lang#91828 (comment), use `as_raw`.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 1, 2022
Implement `RawWaker` and `Waker` getters for underlying pointers

implement rust-lang#87021

New APIs:
- `RawWaker::data(&self) -> *const ()`
- `RawWaker::vtable(&self) -> &'static RawWakerVTable`
- ~`Waker::as_raw_waker(&self) -> &RawWaker`~ `Waker::as_raw(&self) -> &RawWaker`

This third one is an auxiliary function to make the two APIs above more useful. Since we can only get `&Waker` in `Future::poll`, without this, we need to `transmute` it into `&RawWaker` (relying on `repr(transparent)`) in order to access its data/vtable pointers.

~Not sure if it should be named `as_raw` or `as_raw_waker`. Seems we always use `as_<something-raw>` instead of just `as_raw`. But `as_raw_waker` seems not quite consistent with `Waker::from_raw`.~ As suggested in rust-lang#91828 (comment), use `as_raw`.
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 1, 2022
…askrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#86374 (Enable combining `+crt-static` and `relocation-model=pic` on `x86_64-unknown-linux-gnu`)
 - rust-lang#91828 (Implement `RawWaker` and `Waker` getters for underlying pointers)
 - rust-lang#92021 (Eliminate duplicate codes of is_single_fp_element)
 - rust-lang#92584 (add rustc lint, warning when iterating over hashmaps 2)
 - rust-lang#93267 (implement a lint for suspicious auto trait impls)
 - rust-lang#93290 (remove `TyS::same_type`)
 - rust-lang#93436 (Update compiler_builtins to fix duplicate symbols in `armv7-linux-androideabi` rlib)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a643e59 into rust-lang:master Feb 1, 2022
@rustbot rustbot added this to the 1.60.0 milestone Feb 1, 2022
@oxalica oxalica deleted the feat/waker-getters branch February 2, 2022 13:36
@oxalica oxalica mentioned this pull request May 12, 2022
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants