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

Tracking Issue for core::task::ready! macro #70922

Closed
7 of 8 tasks
yoshuawuyts opened this issue Apr 8, 2020 · 9 comments · Fixed by #99419
Closed
7 of 8 tasks

Tracking Issue for core::task::ready! macro #70922

yoshuawuyts opened this issue Apr 8, 2020 · 9 comments · Fixed by #99419
Labels
A-async-await A-macros AsyncAwait-Triaged B-unstable C-tracking-issue disposition-merge finished-final-comment-period Libs-Small Libs-Tracked T-libs-api to-announce

Comments

@yoshuawuyts
Copy link
Member

@yoshuawuyts yoshuawuyts commented Apr 8, 2020

This is a tracking issue for the ready! macro.
The feature gate for the issue is #![feature(ready_macro)].

Steps / History

  • Initial implementation in #70817
  • FCP
  • Stabilization PR in #81050
  • Reverting stabilization: #89651
  • FCP, second attempt
  • Stabilization, second attempt

Unresolved Questions

  • bug: pub macro defined in submodule is shown at the wrong path #74355
  • Use .ready() method instead? See #89780
@yoshuawuyts yoshuawuyts added the C-tracking-issue label Apr 8, 2020
@jonas-schievink jonas-schievink added A-macros B-unstable T-libs-api labels Apr 8, 2020
@yoshuawuyts yoshuawuyts changed the title Tracking Issue for ready! macro Tracking Issue for core::task::ready! macro Jul 15, 2020
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 18, 2020
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)
@KodrAus KodrAus added A-async-await Libs-Small Libs-Tracked labels Jul 29, 2020
@tmandry tmandry added the AsyncAwait-Triaged label Aug 4, 2020
@yoshuawuyts
Copy link
Member Author

@yoshuawuyts yoshuawuyts commented Nov 19, 2020

#77862 seems like it may resolve the rustdoc issue

@RalfJung

This comment has been minimized.

@yoshuawuyts

This comment has been minimized.

@yoshuawuyts
Copy link
Member Author

@yoshuawuyts yoshuawuyts commented Jul 30, 2021

Closed via #81050

@dtolnay
Copy link
Member

@dtolnay dtolnay commented Oct 12, 2021

Reopening on account of the revert in #89651.

@dtolnay dtolnay reopened this Oct 12, 2021
@m-ou-se
Copy link
Member

@m-ou-se m-ou-se commented Jul 6, 2022

After all the discussion on #89780 and Zulip, I think there might be a consensus on continuing with stabilizing the std::task::ready!() macro. (Regardless of whether we want to continue investigating the .ready()? route or not.)

@rfcbot merge

@rfcbot
Copy link

@rfcbot rfcbot commented Jul 6, 2022

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.

@rfcbot rfcbot added proposed-final-comment-period disposition-merge final-comment-period and removed proposed-final-comment-period labels Jul 6, 2022
@rfcbot
Copy link

@rfcbot rfcbot commented Jul 6, 2022

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added finished-final-comment-period to-announce and removed final-comment-period labels Jul 16, 2022
@rfcbot
Copy link

@rfcbot rfcbot commented Jul 16, 2022

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.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jul 18, 2022
…=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`
@bors bors closed this as completed in 7d75497 Jul 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-async-await A-macros AsyncAwait-Triaged B-unstable C-tracking-issue disposition-merge finished-final-comment-period Libs-Small Libs-Tracked T-libs-api to-announce
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants