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

Add Poll::ready and revert stabilization of task::ready! #89651

Merged
merged 4 commits into from
Oct 12, 2021

Conversation

ibraheemdev
Copy link
Member

@ibraheemdev ibraheemdev commented Oct 7, 2021

This PR adds an inherent ready method to Poll that can be used with the ? operator as an alternative to the task::ready! macro:

let val = ready!(fut.poll(cx));
let val = fut.poll(cx).ready()?;

I think this form is a nice, non-breaking middle ground between changing the impl Try for Poll, and adding a separate macro. It looks better than ready! in my opinion, and it composes well:

let elem = ready!(fut.poll(cx)).pop().unwrap();
let elem = fut.poll(cx).ready()?.pop().unwrap();

The planned stabilization of ready! in 1.56 has been reverted because I think this alternate approach is worth considering.

r? rust-lang/libs

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 7, 2021
@ibraheemdev ibraheemdev changed the title Add Poll::ready Add Poll::ready and revert stabilization of task::ready! Oct 7, 2021
@rust-log-analyzer

This comment has been minimized.

@tmandry
Copy link
Member

tmandry commented Oct 8, 2021

This is quite nice. Worth considering at least.

@camelid
Copy link
Member

camelid commented Oct 8, 2021

(Just noting that 1.56 release notes would likely need to be updated.)

@Mark-Simulacrum
Copy link
Member

Please note that if you choose to revert things landing in 1.56, the timeline on that is quite short -- ideally PRs for that are merged into master and tagged with beta-nominated + beta-accepted by mid-next week -- we have just 2 weeks before the release must ship.

@dtolnay dtolnay added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Oct 8, 2021
@dtolnay
Copy link
Member

dtolnay commented Oct 8, 2021

@rfcbot poll libs-api Retract ready! stabilization from 1.56.0?

@rfcbot
Copy link

rfcbot commented Oct 8, 2021

Team member @dtolnay has asked teams: T-libs-api, for consensus on:

Retract ready! stabilization from 1.56.0?

@joshtriplett
Copy link
Member

While I don't see much harm in having both, this does seem better in every way.

@yaahc
Copy link
Member

yaahc commented Oct 11, 2021

While I don't see much harm in having both, this does seem better in every way.

I think there's a distinct disadvantage to having both, insofar as I think it's generally more confusing and difficult to teach features or functionality that can be invoked in multiple different ways. Not to mention the needless style discussions it ends up causing downstream where users often end up wanting to standardize on one option or the other.

@dtolnay
Copy link
Member

dtolnay commented Oct 11, 2021

@bors r+
@rustbot label +beta-nominated

Nominating only the 2nd and 3rd commit (5f7e7d2 + 7a7dfa8) to be backported.

@bors
Copy link
Contributor

bors commented Oct 11, 2021

📌 Commit a1e03fc has been approved by dtolnay

@rustbot rustbot added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Oct 11, 2021
@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 Oct 11, 2021
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 11, 2021
Add `Poll::ready` and revert stabilization of `task::ready!`

This PR adds an inherent `ready` method to `Poll` that can be used with the `?` operator as an alternative to the `task::ready!` macro:
```rust
let val = ready!(fut.poll(cx));
let val = fut.poll(cx).ready()?;
```

I think this form is a nice, non-breaking middle ground between changing the `impl Try for Poll`, and adding a separate macro. It looks better than `ready!` in my opinion, and it composes well:

```rust
let elem = ready!(fut.poll(cx)).pop().unwrap();
let elem = fut.poll(cx).ready()?.pop().unwrap();
```

The planned stabilization of `ready!` in 1.56 has been reverted because I think this alternate approach is worth considering.

r? rust-lang/libs
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 12, 2021
…askrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#89471 (Use Ancestory to check default fn in const impl instead of comparing idents)
 - rust-lang#89643 (Fix inherent impl overlap check.)
 - rust-lang#89651 (Add `Poll::ready` and revert stabilization of `task::ready!`)
 - rust-lang#89675 (Re-use TypeChecker instead of passing around some of its fields )
 - rust-lang#89710 (Add long explanation for error E0482)
 - rust-lang#89756 (Greatly reduce amount of debuginfo compiled for bootstrap itself)
 - rust-lang#89760 (Remove hack ignoring unused attributes for stage 0 std)
 - rust-lang#89772 (Fix function-names test for GDB 10.1)
 - rust-lang#89785 (Fix ICE when compiling nightly std/rustc on beta compiler)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit d3984e1 into rust-lang:master Oct 12, 2021
@rustbot rustbot added this to the 1.57.0 milestone Oct 12, 2021
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 12, 2021
…htriplett

fix minor spelling error in Poll::ready docs

Fixes minor spelling error in the proposed `Poll::ready` docs. Not that my opinion matters, but +1 on the original PR (rust-lang#89651), it reads much nicer to me than the `ready!` macro.
@m-ou-se m-ou-se added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Oct 12, 2021
@m-ou-se
Copy link
Member

m-ou-se commented Oct 12, 2021

Beta-accepted based on #89651 (comment)

Note for whoever end up backporting this: Only commits 5f7e7d2 and 7a7dfa8 should be backported, as David noted above.

the8472 added a commit to the8472/rust that referenced this pull request Oct 12, 2021
…htriplett

fix minor spelling error in Poll::ready docs

Fixes minor spelling error in the proposed `Poll::ready` docs. Not that my opinion matters, but +1 on the original PR (rust-lang#89651), it reads much nicer to me than the `ready!` macro.
the8472 added a commit to the8472/rust that referenced this pull request Oct 12, 2021
…htriplett

fix minor spelling error in Poll::ready docs

Fixes minor spelling error in the proposed `Poll::ready` docs. Not that my opinion matters, but +1 on the original PR (rust-lang#89651), it reads much nicer to me than the `ready!` macro.
@cuviper cuviper mentioned this pull request Oct 13, 2021
@cuviper cuviper removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Oct 13, 2021
@cuviper cuviper modified the milestones: 1.57.0, 1.56.0 Oct 13, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 14, 2021
[beta] backports

- 2229: Consume IfLet expr rust-lang#89282
- Wrapper for -Z gcc-ld=lld to invoke rust-lld with the correct flavor rust-lang#89288
- Fix unsound optimization with explicit variant discriminants rust-lang#89489
- Fix stabilization version for bindings_after_at rust-lang#89605
- Turn vtable_allocation() into a query rust-lang#89619
- Revert "Stabilize Iterator::intersperse()" rust-lang#89638
- Ignore type of projections for upvar capturing rust-lang#89648
- ~~Add Poll::ready and~~ revert stabilization of task::ready! rust-lang#89651
- CI: Use mirror for libisl downloads for more docker dist builds rust-lang#89661
-  Use correct edition for panic in [debug_]assert!(). rust-lang#89622
-  Switch to our own mirror of libisl plus ct-ng oldconfig fixes rust-lang#89599
-  Emit item no type error even if type inference fails rust-lang#89585
-  Revert enum discriminants rust-lang#89884
@qm3ster

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. 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.