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

Stabilize noop_waker #133089

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Stabilize noop_waker #133089

wants to merge 2 commits into from

Conversation

eholk
Copy link
Contributor

@eholk eholk commented Nov 16, 2024

Tracking Issue: #98286

This is a handy feature that's been used widely in tests and example async code and it'd be nice to make it available to users.

cc @rust-lang/wg-async

@eholk eholk added the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Nov 16, 2024
@rustbot
Copy link
Collaborator

rustbot commented Nov 16, 2024

r? @TaKO8Ki

rustbot has assigned @TaKO8Ki.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added PG-exploit-mitigations Project group: Exploit mitigations S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Nov 16, 2024
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

Copy link
Member

@yoshuawuyts yoshuawuyts left a comment

Choose a reason for hiding this comment

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

Stabilizing this seems reasonable to me! We spent an extended period discussing this in @rust-lang/wg-async triage earlier this year, and I believe we were generally in favor of moving forward with this API. Thank you for putting this up!

@Amanieu Amanieu added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Nov 19, 2024
@Amanieu
Copy link
Member

Amanieu commented Nov 19, 2024

@rfcbot fcp merge

@rfcbot
Copy link

rfcbot commented Nov 19, 2024

Team member @Amanieu 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 the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Nov 19, 2024
@Amanieu Amanieu removed the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Nov 19, 2024
@rfcbot rfcbot added 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 Nov 19, 2024
@rfcbot
Copy link

rfcbot commented Nov 19, 2024

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

@eholk eholk marked this pull request as ready for review November 19, 2024 18:01
@rustbot
Copy link
Collaborator

rustbot commented Nov 19, 2024

Some changes occurred in tests/ui/sanitizer

cc @rust-lang/project-exploit-mitigations, @rcvalle

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred in coverage tests.

cc @Zalathar

@compiler-errors compiler-errors added S-waiting-on-fcp Status: PR is in FCP and is awaiting for FCP to complete. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 21, 2024
@bors
Copy link
Contributor

bors commented Nov 23, 2024

☔ The latest upstream changes (presumably #133349) made this pull request unmergeable. Please resolve the merge conflicts.

@matthiaskrgr
Copy link
Member

@bors r-
#133783 (comment)

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 3, 2024
@eholk
Copy link
Contributor Author

eholk commented Dec 3, 2024

@bors r=dtolnay rollup=never

I somehow managed not to include the change to await_ready.rs that removed the noop_waker feature and only included the change to the .coverage file. This is fixed now.

@bors
Copy link
Contributor

bors commented Dec 3, 2024

📌 Commit 279d3d2 has been approved by dtolnay

It is now in the queue for this repository.

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 3, 2024
@bors
Copy link
Contributor

bors commented Dec 3, 2024

🔒 Merge conflict

This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again.

How do I rebase?

Assuming self is your fork and upstream is this repository, you can resolve the conflict following these steps:

  1. git checkout stabilize-noop-waker (switch to your branch)
  2. git fetch upstream master (retrieve the latest master)
  3. git rebase upstream/master -p (rebase on top of it)
  4. Follow the on-screen instruction to resolve conflicts (check git status if you got lost).
  5. git push self stabilize-noop-waker --force-with-lease (update this PR)

You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial.

Please avoid the "Resolve conflicts" button on GitHub. It uses git merge instead of git rebase which makes the PR commit history more difficult to read.

Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Cargo.lock conflict is handled during merge and rebase. This is normal, and you should still perform step 5 to update this PR.

Error message
Auto-merging tests/ui/async-await/async-closures/mangle.rs
Auto-merging tests/ui/async-await/async-closures/inline-body.rs
Auto-merging tests/crashes/124020.rs
CONFLICT (content): Merge conflict in tests/crashes/124020.rs
Auto-merging src/tools/miri/tests/pass/async-closure-drop.rs
CONFLICT (content): Merge conflict in src/tools/miri/tests/pass/async-closure-drop.rs
Auto-merging src/tools/miri/tests/pass/async-closure-captures.rs
CONFLICT (content): Merge conflict in src/tools/miri/tests/pass/async-closure-captures.rs
Auto-merging library/core/tests/lib.rs
Automatic merge failed; fix conflicts and then commit the result.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 3, 2024
@eholk
Copy link
Contributor Author

eholk commented Dec 3, 2024

@bors r=dtolnay

Rebased after conflicting upstream changes.

@bors
Copy link
Contributor

bors commented Dec 3, 2024

📌 Commit 31689e9 has been approved by dtolnay

It is now in the queue for this repository.

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 3, 2024
@Zalathar
Copy link
Contributor

Zalathar commented Dec 4, 2024

Be aware that #133849 is going to churn all of the coverage-test snapshot files, so if that lands you'll need another rebase and bless to fix the conflicts. But the test .rs files aren't modified, so you shouldn't need to resolve anything manually.

Also, it looks like this PR is not actually in the bors queue for some reason, despite being approved.

@oli-obk
Copy link
Contributor

oli-obk commented Dec 4, 2024

@bors r-
@bors r=dtolnay

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 4, 2024
@bors
Copy link
Contributor

bors commented Dec 4, 2024

📌 Commit 279d3d2 has been approved by dtolnay

It is now in the queue for this repository.

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 4, 2024
@bors
Copy link
Contributor

bors commented Dec 4, 2024

☔ The latest upstream changes (presumably #133865) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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. PG-exploit-mitigations Project group: Exploit mitigations S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. to-announce Announce this issue on triage meeting
Projects
None yet
Development

Successfully merging this pull request may close these issues.