Skip to content

switch to std::task::ready!() where possible#11130

Merged
bors merged 1 commit intorust-lang:masterfrom
Muscraft:use-ready-macro
Sep 22, 2022
Merged

switch to std::task::ready!() where possible#11130
bors merged 1 commit intorust-lang:masterfrom
Muscraft:use-ready-macro

Conversation

@Muscraft
Copy link
Member

@Muscraft Muscraft commented Sep 22, 2022

In Rust 1.64.0, std::task::ready was stabilized. Using ready!() can make what is happening clearer in the code as it expands to roughly:

let num = match fut.poll(cx) {
    Poll::Ready(t) => t,
    Poll::Pending => return Poll::Pending,
};

This PR replaces places using Poll::Pending => return Poll::Pending with ready!() where appropriate.

I was unsure about how useful the new macro would be in one place in cargo/registry/index.rs, so I left it out and will add it in if needed.

Close #11128

r? @Eh2406

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 22, 2022
@Eh2406
Copy link
Contributor

Eh2406 commented Sep 22, 2022

These all look like good improvements.
I agree with you that cargo/registry/index.rs, is significantly clearer as a match.

What do you think about changing cargo/sources/registry/http_remote.rs and cargo/sources/registry/remote.rs?

@Muscraft
Copy link
Member Author

Muscraft commented Sep 22, 2022 •
edited
Loading

I think it makes sense to use ready!() in those two places, it makes them so you are matching once it is ready. I think theoretically cargo/registry/index.rs could probably have a similar structure:

match ready!(self.query_inner_with_online(dep, load, yanked_whitelist, f, false)?) {
    0 => {}
    _ => return Poll::Ready(Ok(())),
}

Which would make the whole thing:

if self.config.offline() {
    match ready!(self.query_inner_with_online(dep, load, yanked_whitelist, f, false)?) {
        0 => {}
        _ => return Poll::Ready(Ok(())),
    }
    // If offline, and there are no matches, try again with online.
    // This is necessary for dependencies that are not used (such as
    // target-cfg or optional), but are not downloaded. Normally the
    // build should succeed if they are not downloaded and not used,
    // but they still need to resolve. If they are actually needed
    // then cargo will fail to download and an error message
    // indicating that the required dependency is unavailable while
    // offline will be displayed.
}
self.query_inner_with_online(dep, load, yanked_whitelist, f, true)
    .map_ok(|_| ())

I'm not sold on this but I think it could be fine. I'll leave it up to you.

@Eh2406
Copy link
Contributor

Eh2406 commented Sep 22, 2022 •
edited
Loading

Maybe something like:

    if ready!(self.query_inner_with_online(dep, load, yanked_whitelist, f, false)?) > 0 {
        return Poll::Ready(Ok(())),
    }

@Eh2406
Copy link
Contributor

Eh2406 commented Sep 22, 2022

@bors r+

And thank you!

@bors
Copy link
Contributor

bors commented Sep 22, 2022

📌 Commit 0c2e0fe has been approved by Eh2406

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-review Status: Awaiting review from the assignee but also interested parties. labels Sep 22, 2022
@bors
Copy link
Contributor

bors commented Sep 22, 2022

⌛ Testing commit 0c2e0fe with merge dbd1c18...

@bors
Copy link
Contributor

bors commented Sep 22, 2022

☀️ Test successful - checks-actions
Approved by: Eh2406
Pushing dbd1c18 to master...

@bors bors merged commit dbd1c18 into rust-lang:master Sep 22, 2022
@Muscraft Muscraft deleted the use-ready-macro branch September 22, 2022 20:10
@weihanglo weihanglo mentioned this pull request Sep 27, 2022
Merged
bors pushed a commit to rust-lang-ci/rust that referenced this pull request Sep 27, 2022
@weihanglo
Update cargo
5095f54 
22 commits in 73ba3f35e0205844418260722c11602113179c4a..f5fed93ba24607980647962c59863bbabb03ce14
2022-09-18 06:38:16 +0000 to 2022-09-27 12:03:57 +0000

- build-scripts.md: Use em dash consistently. (rust-lang/cargo#11150)
- Indicate how Cargo locates the manifest (rust-lang/cargo#10770)
- Reduce references to `[project]` within cargo (rust-lang/cargo#11135)
- Iteratively construct target cfg (rust-lang/cargo#11114)
- update comment about `CARGO_BIN_EXE_` (rust-lang/cargo#11146)
- Call out that not all config values can be set via env vars (rust-lang/cargo#11139)
- Bump to 0.67.0, update changelog (rust-lang/cargo#11137)
- ci: update toolchain for building api doc (rust-lang/cargo#11134)
- Http publish not noop (rust-lang/cargo#11111)
- Improve errors for TOML fields that support workspace inheritance (rust-lang/cargo#11113)
- switch to `std::task::ready!()` where possible (rust-lang/cargo#11130)
- Report cmd aliasing failure with more contexts (rust-lang/cargo#11087)
- minor: remove unused mut (rust-lang/cargo#11127)
- fix(cli): Forward non-UTF8 arguments to external subcommands (rust-lang/cargo#11118)
- This change adds an example to the authors attribute in the manifest. (rust-lang/cargo#10938)
- Add support for relative git submodule paths (rust-lang/cargo#11106)
- make unknown features on `cargo add` more discoverable (rust-lang/cargo#11098)
- Unlink old final artifacts before compilation (rust-lang/cargo#11122)
- refactor(cli): Prepare for clap v4 (rust-lang/cargo#11116)
- fix(cli): Error trailing args rather than ignore (rust-lang/cargo#11119)
- Add a minor clarification (rust-lang/cargo#11093)
- doc(changelog): mention CVE fixes (rust-lang/cargo#11104)
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 27, 2022
@bors
Auto merge of rust-lang#102365 - weihanglo:update-cargo, r=ehuss
470e518 
Update cargo

22 commits in 73ba3f35e0205844418260722c11602113179c4a..f5fed93ba24607980647962c59863bbabb03ce14 2022-09-18 06:38:16 +0000 to 2022-09-27 12:03:57 +0000

- build-scripts.md: Use em dash consistently. (rust-lang/cargo#11150)
- Indicate how Cargo locates the manifest (rust-lang/cargo#10770)
- Reduce references to `[project]` within cargo (rust-lang/cargo#11135)
- Iteratively construct target cfg (rust-lang/cargo#11114)
- update comment about `CARGO_BIN_EXE_` (rust-lang/cargo#11146)
- Call out that not all config values can be set via env vars (rust-lang/cargo#11139)
- Bump to 0.67.0, update changelog (rust-lang/cargo#11137)
- ci: update toolchain for building api doc (rust-lang/cargo#11134)
- Http publish not noop (rust-lang/cargo#11111)
- Improve errors for TOML fields that support workspace inheritance (rust-lang/cargo#11113)
- switch to `std::task::ready!()` where possible (rust-lang/cargo#11130)
- Report cmd aliasing failure with more contexts (rust-lang/cargo#11087)
- minor: remove unused mut (rust-lang/cargo#11127)
- fix(cli): Forward non-UTF8 arguments to external subcommands (rust-lang/cargo#11118)
- This change adds an example to the authors attribute in the manifest. (rust-lang/cargo#10938)
- Add support for relative git submodule paths (rust-lang/cargo#11106)
- make unknown features on `cargo add` more discoverable (rust-lang/cargo#11098)
- Unlink old final artifacts before compilation (rust-lang/cargo#11122)
- refactor(cli): Prepare for clap v4 (rust-lang/cargo#11116)
- fix(cli): Error trailing args rather than ignore (rust-lang/cargo#11119)
- Add a minor clarification (rust-lang/cargo#11093)
- doc(changelog): mention CVE fixes (rust-lang/cargo#11104)
@ehuss ehuss added this to the 1.66.0 milestone Oct 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@Eh2406 Eh2406

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Milestone

1.66.0

Development

Successfully merging this pull request may close these issues.

Take advantage of new poll task::ready

5 participants

@Muscraft @Eh2406 @bors @ehuss @rust-highfive