-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Tracking Issue for core::task::ready! macro #70922
Comments
Add core::task::ready! macro This PR adds `ready!` as a top-level macro to `libcore` following the implementation of `futures_core::ready`, tracking issue rust-lang#70922. This macro is commonly used when implementing `Future`, `AsyncRead`, `AsyncWrite` and `Stream`. And being only 5 lines, it seems like a useful and straight forward addition to std. ## Example ```rust use core::task::{Context, Poll}; use core::future::Future; use core::pin::Pin; async fn get_num() -> usize { 42 } pub fn do_poll(cx: &mut Context<'_>) -> Poll<()> { let mut f = get_num(); let f = unsafe { Pin::new_unchecked(&mut f) }; let num = ready!(f.poll(cx)); // ... use num Poll::Ready(()) } ``` ## Naming In `async-std` we chose to nest the macro under the `task` module instead of having the macro at the top-level. This is a pattern that currently does not occur in std, mostly due to this not being possible prior to Rust 2018. This PR proposes to add the `ready` macro as `core::ready`. But another option would be to introduce it as `core::task::ready` since it's really only useful when used in conjunction with `task::{Context, Poll}`. ## Implementation questions I tried rendering the documentation locally but the macro didn't show up under `core`. I'm not sure if I quite got this right. I used the [`todo!` macro PR](https://github.com/rust-lang/rust/pull/56348/files) as a reference, and our approaches look similar. ## References - [`futures::ready`](https://docs.rs/futures/0.3.4/futures/macro.ready.html) - [`async_std::task::ready`](https://docs.rs/async-std/1.5.0/async_std/task/index.html) - [`futures_core::ready`](https://docs.rs/futures-core/0.3.4/futures_core/macro.ready.html)
#77862 seems like it may resolve the rustdoc issue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Closed via #81050 |
Reopening on account of the revert in #89651. |
Team member @m-ou-se has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
The final comment period, with a disposition to merge, as per the review above, is now complete. As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed. This will be merged soon. |
…=Mark-Simulacrum Stabilize `core::task::ready!` This stabilizes `core::task::ready!` for Rust 1.64. The FCP for stabilization was just completed here rust-lang#70922 (comment). Thanks! Closes rust-lang#70922 cc/ `@rust-lang/libs-api`
…ulacrum Stabilize `core::task::ready!` This stabilizes `core::task::ready!` for Rust 1.64. The FCP for stabilization was just completed here rust-lang/rust#70922 (comment). Thanks! Closes #70922 cc/ ``@rust-lang/libs-api``
This is a tracking issue for the
ready!
macro.The feature gate for the issue is
#![feature(ready_macro)]
.Steps / History
Poll::ready
and revert stabilization oftask::ready!
#89651Unresolved Questions
pub macro
defined in submodule is shown at the wrong pathpub macro
defined in submodule is shown at the wrong path #74355.ready()
method instead? See Tracking Issue for poll.ready()? #89780The text was updated successfully, but these errors were encountered: