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.

Sign up for GitHub

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

Jump to bottom

Stabilize waker_getters #129919

Merged
merged 3 commits into from
Sep 5, 2024
Merged

Stabilize waker_getters #129919

merged 3 commits into from
Sep 5, 2024

Conversation

kevinmehall
Copy link
Contributor

@kevinmehall kevinmehall commented Sep 2, 2024 •
edited
Loading

Tracking issue: #96992

FCP completed on the tracking issue a while ago. It's not clear whether the libs-api team wanted the RawWaker methods moved to Waker or went back to the current API after further discussion. @Amanieu wrote "This is just waiting for someone to submit a stabilization PR." so I'm doing just that in hopes of nudging this along.

Edit: Moved the data and vtable methods from RawWaker to Waker and added Waker::new per #96992 (comment)

impl Waker {
  pub const unsafe fn new(data: *const (), vtable: &'static RawWakerVTable) -> Self;
  pub fn data(&self) -> *const ();
  pub fn vtable(&self) -> &'static RawWakerVTable;
}

Closes #96992

@rustbot
Copy link
Collaborator

rustbot commented Sep 2, 2024

r? @cuviper

rustbot has assigned @cuviper.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 2, 2024
@kevinmehall kevinmehall mentioned this pull request Sep 2, 2024
Closed
3 tasks
traviscross
traviscross requested changes Sep 2, 2024
View reviewed changes
library/core/src/task/wake.rs Outdated
Comment on lines 66 to 77
#[stable(feature = "waker_getters", since = "CURRENT_RUSTC_VERSION")]
pub fn data(&self) -> *const () {
self.data
}

/// Gets the `vtable` pointer used to create this `RawWaker`.
#[inline]
#[must_use]
#[unstable(feature = "waker_getters", issue = "96992")]
#[stable(feature = "waker_getters", since = "CURRENT_RUSTC_VERSION")]
pub fn vtable(&self) -> &'static RawWakerVTable {
self.vtable
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The API still needs to be updated to what was decided via FCP here:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

Should the new pub const unsafe fn new(data: *const (), vtable: &'static RawWakerVTable) -> Self; constructor from that comment be made immediately stable in this PR? Or should that be done separately under a new feature since it's not really part of waker_getters? The plan to deprecate RawWaker doesn't work because it's in the RawWakerVTable signature as discussed in later comments on the tracking issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

cc @dtolnay

Copy link
Member

Choose a reason for hiding this comment

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

Waker::new is an obvious function with an obvious name, and is the composition of 2 already-stable APIs (RawWaker::new + Waker::from_raw). It was discussed by most of the team in a library API team meeting (#96992 (comment)) and approved by the team's consensus process in #96992 (comment) with no objections. I think the circumstances are fine to stabilize this immediately.

@dtolnay dtolnay assigned dtolnay and unassigned cuviper Sep 4, 2024
dtolnay
dtolnay approved these changes Sep 4, 2024
View reviewed changes
Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Thank you!

@dtolnay
Copy link
Member

dtolnay commented Sep 4, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Sep 4, 2024

📌 Commit 22bd319 has been approved by dtolnay

It is now in the queue for this repository.

@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 Sep 4, 2024
@matthiaskrgr matthiaskrgr mentioned this pull request Sep 5, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 5, 2024
@bors
Auto merge of rust-lang#129978 - matthiaskrgr:rollup-a7ryoox, r=matth…
18237ce 
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#101339 (enable -Zrandomize-layout in debug CI builds )
 - rust-lang#120736 (rustdoc: add header map to the table of contents)
 - rust-lang#127021 (Add target support for RTEMS Arm)
 - rust-lang#128928 (CI: rfl: add more tools and steps)
 - rust-lang#129584 (warn the user if the upstream master branch is old)
 - rust-lang#129664 (Arbitrary self types v2: pointers feature gate.)
 - rust-lang#129752 (Make supertrait and implied predicates queries defaulted)
 - rust-lang#129918 (Update docs of `missing_abi` lint)
 - rust-lang#129919 (Stabilize `waker_getters`)
 - rust-lang#129925 (remove deprecated option `rust.split-debuginfo`)

Failed merges:

 - rust-lang#129789 (rustdoc: use strategic boxing to shrink `clean::Item`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 5, 2024
@bors
Auto merge of rust-lang#129978 - matthiaskrgr:rollup-a7ryoox, r=matth…
eb33b43 
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#101339 (enable -Zrandomize-layout in debug CI builds )
 - rust-lang#120736 (rustdoc: add header map to the table of contents)
 - rust-lang#127021 (Add target support for RTEMS Arm)
 - rust-lang#128928 (CI: rfl: add more tools and steps)
 - rust-lang#129584 (warn the user if the upstream master branch is old)
 - rust-lang#129664 (Arbitrary self types v2: pointers feature gate.)
 - rust-lang#129752 (Make supertrait and implied predicates queries defaulted)
 - rust-lang#129918 (Update docs of `missing_abi` lint)
 - rust-lang#129919 (Stabilize `waker_getters`)
 - rust-lang#129925 (remove deprecated option `rust.split-debuginfo`)

Failed merges:

 - rust-lang#129789 (rustdoc: use strategic boxing to shrink `clean::Item`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit c7c3ada into rust-lang:master Sep 5, 2024
6 checks passed
@rustbot rustbot added this to the 1.83.0 milestone Sep 5, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Sep 5, 2024
@rust-timer
Unrolled build for rust-lang#129919
b385528 
Rollup merge of rust-lang#129919 - kevinmehall:waker-getters, r=dtolnay

Stabilize `waker_getters`

Tracking issue: rust-lang#96992

FCP completed on the tracking issue a while ago. It's not clear whether the libs-api team wanted the `RawWaker` methods moved to `Waker` or went back to the current API after further discussion. `@Amanieu` [wrote "This is just waiting for someone to submit a stabilization PR."](rust-lang#96992 (comment)) so I'm doing just that in hopes of nudging this along.

Edit: Moved the `data` and `vtable` methods from `RawWaker` to `Waker` and added `Waker::new` per rust-lang#96992 (comment)

```rs
impl Waker {
  pub const unsafe fn new(data: *const (), vtable: &'static RawWakerVTable) -> Self;
  pub fn data(&self) -> *const ();
  pub fn vtable(&self) -> &'static RawWakerVTable;
}
```

Closes rust-lang#96992
@Dirbaio Dirbaio mentioned this pull request Oct 8, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@traviscross traviscross traviscross requested changes

@dtolnay dtolnay dtolnay approved these changes

Assignees

@dtolnay dtolnay

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.83.0
Development

Successfully merging this pull request may close these issues.

Tracking Issue for waker_getters
6 participants
@kevinmehall @rustbot @dtolnay @bors @traviscross @cuviper