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

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 Area: Async & Await A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. Libs-Small Libs issues that are considered "small" or self-contained Libs-Tracked Libs issues that are tracked on the team's project board. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@yoshuawuyts
Copy link
Member

yoshuawuyts commented Apr 8, 2020 •
edited by joshtriplett
Loading

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

Steps / History

Unresolved Questions
@yoshuawuyts yoshuawuyts added the C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. label Apr 8, 2020
@jonas-schievink jonas-schievink added A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) B-unstable Blocker: Implemented in the nightly compiler and unstable. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Apr 8, 2020
@yoshuawuyts yoshuawuyts changed the title Tracking Issue for ready! macro Tracking Issue for core::task::ready! macro Jul 15, 2020
@yoshuawuyts yoshuawuyts mentioned this issue Jul 15, 2020
Merged
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 18, 2020
@Manishearth
Rollup merge of rust-lang#70817 - yoshuawuyts:task-ready, r=dtolnay
479c8ad 
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 Area: Async & Await Libs-Small Libs issues that are considered "small" or self-contained Libs-Tracked Libs issues that are tracked on the team's project board. labels Jul 29, 2020
@tmandry tmandry added the AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. label Aug 4, 2020
@yoshuawuyts
Copy link
Member Author

#77862 seems like it may resolve the rustdoc issue

@RalfJung

This comment has been minimized.

Sign in to view
@yoshuawuyts

This comment has been minimized.

Sign in to view
@yoshuawuyts yoshuawuyts mentioned this issue Jan 15, 2021
Merged
@yoshuawuyts
Copy link
Member Author

Closed via #81050

@dtolnay dtolnay mentioned this issue Oct 11, 2021
Closed
@dtolnay
Copy link
Member

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 commented Jul 6, 2022 •
edited
Loading

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 commented Jul 6, 2022 •
edited by joshtriplett
Loading

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 Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Jul 6, 2022
@rfcbot
Copy link

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 The final comment period is finished for this PR / Issue. to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Jul 16, 2022
@rfcbot
Copy link

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.

@yoshuawuyts yoshuawuyts mentioned this issue Jul 18, 2022
Merged
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jul 18, 2022
@matthiaskrgr
Rollup merge of rust-lang#99419 - yoshuawuyts:stabilize-task-ready, r…
f3816dd 
…=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
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Sep 8, 2022
workingjubilee pushed a commit to tcdi/postgrestd that referenced this issue Sep 15, 2022
@matthiaskrgr
Rollup merge of #99419 - yoshuawuyts:stabilize-task-ready, r=Mark-Sim…
2c40aa4 
…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``
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-async-await Area: Async & Await A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. Libs-Small Libs issues that are considered "small" or self-contained Libs-Tracked Libs issues that are tracked on the team's project board. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Stabilize core::task::ready!
9 participants
@RalfJung @m-ou-se @jonas-schievink @dtolnay @tmandry @yoshuawuyts @apiraino @KodrAus @rfcbot