Skip to content

Stabilize Poll::is_ready and is_pending as const #76227

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 3 commits into from
Nov 8, 2020

Conversation

CDirkx
Copy link
Contributor

@CDirkx CDirkx commented Sep 2, 2020

Insta-stabilize the methods is_ready and is_pending of std::task::Poll as const, in the same way as PR#76198.

Possible because of the recent stabilization of const control flow.

Part of #76225.

Insta-stabilize the methods `is_ready` and `is_pending` of `Poll`.

Possible because of the recent stabilization of const control flow.

Also adds a test for these methods in a const context.
@rust-highfive
Copy link
Contributor

r? @shepmaster

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 2, 2020

const IS_PENDING : bool = POLL.is_pending();
assert!(IS_PENDING);
}
Copy link
Member

Choose a reason for hiding this comment

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

Hm, why are we using a UI test for this, especially a run-pass one? I would think that these integrated tests are much costlier than standard #[test] tests, as well as much harder to debug. Linking a whole separate executable to check two const methods of stdlib is an overkill.

I would suggest testing similar things by unit or integrated #[test] tests in core or std itself.

It might also be a good idea to sift through existing test suite for stdlib tests which can be moved to unit tests.

Copy link
Member

Choose a reason for hiding this comment

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

Hm, yeah, we already have a bunch of tests like this...

I've opened https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/UI.20tests.20for.20stdlib for discussion, it might be that I am missing some context here of course.

Copy link
Member

Choose a reason for hiding this comment

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

The conclusion is that we want to move such tests to library/: #76268

@CDirkx if you could move some of the already existing const tests in a similar way, that would be awesome :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, I'll have a look later today

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tests are moved 👍

@shepmaster
Copy link
Member

r? @Amanieu

Same deal as #76198 (comment) ?

@rust-highfive rust-highfive assigned Amanieu and unassigned shepmaster Sep 2, 2020
@Amanieu Amanieu added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Sep 3, 2020
@jonas-schievink jonas-schievink added the relnotes Marks issues that should be documented in the release notes of the next release. label Sep 13, 2020
@Dylan-DPC-zz
Copy link

r? @KodrAus

@rust-highfive rust-highfive assigned KodrAus and unassigned Amanieu Sep 29, 2020
@KodrAus
Copy link
Contributor

KodrAus commented Sep 30, 2020

This seems reasonable to me. Thanks @CDirkx!

@rfcbot fcp merge

This stabilizes the following boolean-style match methods on Poll as const:

impl<T> Poll<T> {
    pub const fn is_ready(&self) -> bool;
    pub const fn is_pending(&self) -> bool;
}

@rfcbot
Copy link
Collaborator

rfcbot commented Sep 30, 2020

Team member @KodrAus 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. labels Sep 30, 2020
@camelid camelid added S-waiting-on-fcp Status: PR is in FCP and is awaiting for FCP to complete. S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. S-waiting-on-fcp Status: PR is in FCP and is awaiting for FCP to complete. labels Oct 16, 2020
@rfcbot
Copy link
Collaborator

rfcbot commented Oct 16, 2020

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

@rfcbot rfcbot added 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 Oct 16, 2020
@dtolnay
Copy link
Member

dtolnay commented Oct 16, 2020

@bors r+

@camelid camelid removed the S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). label Oct 16, 2020
@rfcbot rfcbot added 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 Oct 26, 2020
@rfcbot
Copy link
Collaborator

rfcbot commented Oct 26, 2020

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.

The RFC will be merged soon.

@spastorino spastorino removed the to-announce Announce this issue on triage meeting label Oct 29, 2020
@CDirkx
Copy link
Contributor Author

CDirkx commented Nov 7, 2020

Did this get added to the queue correctly?

@Dylan-DPC-zz
Copy link

@bors r=KodrAus

@bors
Copy link
Collaborator

bors commented Nov 7, 2020

📌 Commit 5e80c65 has been approved by KodrAus

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Nov 7, 2020
m-ou-se added a commit to m-ou-se/rust that referenced this pull request Nov 7, 2020
Stabilize `Poll::is_ready` and `is_pending` as const

Insta-stabilize the methods `is_ready` and `is_pending` of `std::task::Poll` as const, in the same way as [PR#76198](rust-lang#76198).

Possible because of the recent stabilization of const control flow.

Part of rust-lang#76225.
m-ou-se added a commit to m-ou-se/rust that referenced this pull request Nov 7, 2020
Stabilize `Poll::is_ready` and `is_pending` as const

Insta-stabilize the methods `is_ready` and `is_pending` of `std::task::Poll` as const, in the same way as [PR#76198](rust-lang#76198).

Possible because of the recent stabilization of const control flow.

Part of rust-lang#76225.
m-ou-se added a commit to m-ou-se/rust that referenced this pull request Nov 7, 2020
Stabilize `Poll::is_ready` and `is_pending` as const

Insta-stabilize the methods `is_ready` and `is_pending` of `std::task::Poll` as const, in the same way as [PR#76198](rust-lang#76198).

Possible because of the recent stabilization of const control flow.

Part of rust-lang#76225.
m-ou-se added a commit to m-ou-se/rust that referenced this pull request Nov 7, 2020
Stabilize `Poll::is_ready` and `is_pending` as const

Insta-stabilize the methods `is_ready` and `is_pending` of `std::task::Poll` as const, in the same way as [PR#76198](rust-lang#76198).

Possible because of the recent stabilization of const control flow.

Part of rust-lang#76225.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Nov 8, 2020
Stabilize `Poll::is_ready` and `is_pending` as const

Insta-stabilize the methods `is_ready` and `is_pending` of `std::task::Poll` as const, in the same way as [PR#76198](rust-lang#76198).

Possible because of the recent stabilization of const control flow.

Part of rust-lang#76225.
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 8, 2020
Rollup of 19 pull requests

Successful merges:

 - rust-lang#76097 (Stabilize hint::spin_loop)
 - rust-lang#76227 (Stabilize `Poll::is_ready` and `is_pending` as const)
 - rust-lang#78065 (make concurrency helper more pleasant to read)
 - rust-lang#78570 (Remove FIXME comment in print_type_sizes ui test suite)
 - rust-lang#78572 (Use SOCK_CLOEXEC and accept4() on more platforms.)
 - rust-lang#78658 (Add a tool to run `x.py` from any subdirectory)
 - rust-lang#78706 (Fix run-make tests running when LLVM is disabled)
 - rust-lang#78728 (Constantify `UnsafeCell::into_inner` and related)
 - rust-lang#78775 (Bump Rustfmt and RLS)
 - rust-lang#78788 (Correct unsigned equivalent of isize to be usize)
 - rust-lang#78811 (Make some std::io functions `const`)
 - rust-lang#78828 (use single char patterns for split() (clippy::single_char_pattern))
 - rust-lang#78841 (Small cleanup in `TypeFoldable` derive macro)
 - rust-lang#78842 (Honor the rustfmt setting in config.toml)
 - rust-lang#78843 (Less verbose debug logging from inlining integrator)
 - rust-lang#78852 (Convert a bunch of intra-doc links)
 - rust-lang#78860 (rustc_resolve: Use `#![feature(format_args_capture)]`)
 - rust-lang#78861 (typo and formatting)
 - rust-lang#78865 (Don't fire `CONST_ITEM_MUTATION` lint when borrowing a deref)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit bdeace9 into rust-lang:master Nov 8, 2020
@rustbot rustbot added this to the 1.49.0 milestone Nov 8, 2020
@CDirkx CDirkx deleted the const-poll branch November 8, 2020 17:00
@dtolnay dtolnay self-assigned this Mar 24, 2024
@RalfJung RalfJung added the A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) label Dec 1, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) 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. relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. S-waiting-on-fcp Status: PR is in FCP and is awaiting for FCP to complete. 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.