Skip to content

Commit dbd1c18

Browse files
committed
Auto merge of #11130 - Muscraft:use-ready-macro, r=Eh2406
switch to `std::task::ready!()` where possible In Rust 1.64.0, [`std::task::ready`](https://doc.rust-lang.org/stable/std/task/macro.ready.html) was [stabilized](https://blog.rust-lang.org/2022/09/22/Rust-1.64.0.html#stabilized-apis). Using `ready!()` can make what is happening clearer in the code as it expands to roughly: ```rust 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`](https://github.com/rust-lang/cargo/blob/247ca7fd04e5c7cc383bcac7d72316e98fbea2bc/src/cargo/sources/registry/index.rs#L425-L429), so I left it out and will add it in if needed. Close #11128 r? `@Eh2406`
2 parents 247ca7f + 0c2e0fe commit dbd1c18

File tree

4 files changed

+44
-76
lines changed

4 files changed

+44
-76
lines changed

src/cargo/core/registry.rs

Lines changed: 22 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use std::collections::{HashMap, HashSet};
2-
use std::task::Poll;
2+
use std::task::{ready, Poll};
33

44
use crate::core::PackageSet;
55
use crate::core::{Dependency, PackageId, QueryKind, Source, SourceId, SourceMap, Summary};
@@ -482,10 +482,7 @@ impl<'cfg> PackageRegistry<'cfg> {
482482
for &s in self.overrides.iter() {
483483
let src = self.sources.get_mut(s).unwrap();
484484
let dep = Dependency::new_override(dep.package_name(), s);
485-
let mut results = match src.query_vec(&dep, QueryKind::Exact) {
486-
Poll::Ready(results) => results?,
487-
Poll::Pending => return Poll::Pending,
488-
};
485+
let mut results = ready!(src.query_vec(&dep, QueryKind::Exact))?;
489486
if !results.is_empty() {
490487
return Poll::Ready(Ok(Some(results.remove(0))));
491488
}
@@ -580,10 +577,7 @@ impl<'cfg> Registry for PackageRegistry<'cfg> {
580577
assert!(self.patches_locked);
581578
let (override_summary, n, to_warn) = {
582579
// Look for an override and get ready to query the real source.
583-
let override_summary = match self.query_overrides(dep) {
584-
Poll::Ready(override_summary) => override_summary?,
585-
Poll::Pending => return Poll::Pending,
586-
};
580+
let override_summary = ready!(self.query_overrides(dep))?;
587581

588582
// Next up on our list of candidates is to check the `[patch]`
589583
// section of the manifest. Here we look through all patches
@@ -880,23 +874,17 @@ fn summary_for_patch(
880874
// No summaries found, try to help the user figure out what is wrong.
881875
if let Some(locked) = locked {
882876
// Since the locked patch did not match anything, try the unlocked one.
883-
let orig_matches = match source.query_vec(orig_patch, QueryKind::Exact) {
884-
Poll::Pending => return Poll::Pending,
885-
Poll::Ready(deps) => deps,
886-
}
887-
.unwrap_or_else(|e| {
888-
log::warn!(
889-
"could not determine unlocked summaries for dep {:?}: {:?}",
890-
orig_patch,
891-
e
892-
);
893-
Vec::new()
894-
});
877+
let orig_matches =
878+
ready!(source.query_vec(orig_patch, QueryKind::Exact)).unwrap_or_else(|e| {
879+
log::warn!(
880+
"could not determine unlocked summaries for dep {:?}: {:?}",
881+
orig_patch,
882+
e
883+
);
884+
Vec::new()
885+
});
895886

896-
let summary = match summary_for_patch(orig_patch, &None, orig_matches, source) {
897-
Poll::Pending => return Poll::Pending,
898-
Poll::Ready(summary) => summary?,
899-
};
887+
let summary = ready!(summary_for_patch(orig_patch, &None, orig_matches, source))?;
900888

901889
// The unlocked version found a match. This returns a value to
902890
// indicate that this entry should be unlocked.
@@ -905,18 +893,15 @@ fn summary_for_patch(
905893
// Try checking if there are *any* packages that match this by name.
906894
let name_only_dep = Dependency::new_override(orig_patch.package_name(), orig_patch.source_id());
907895

908-
let name_summaries = match source.query_vec(&name_only_dep, QueryKind::Exact) {
909-
Poll::Pending => return Poll::Pending,
910-
Poll::Ready(deps) => deps,
911-
}
912-
.unwrap_or_else(|e| {
913-
log::warn!(
914-
"failed to do name-only summary query for {:?}: {:?}",
915-
name_only_dep,
916-
e
917-
);
918-
Vec::new()
919-
});
896+
let name_summaries =
897+
ready!(source.query_vec(&name_only_dep, QueryKind::Exact)).unwrap_or_else(|e| {
898+
log::warn!(
899+
"failed to do name-only summary query for {:?}: {:?}",
900+
name_only_dep,
901+
e
902+
);
903+
Vec::new()
904+
});
920905
let mut vers = name_summaries
921906
.iter()
922907
.map(|summary| summary.version())

src/cargo/sources/registry/http_remote.rs

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use std::collections::{HashMap, HashSet};
1919
use std::fs::{self, File};
2020
use std::path::{Path, PathBuf};
2121
use std::str;
22-
use std::task::Poll;
22+
use std::task::{ready, Poll};
2323
use std::time::Duration;
2424
use url::Url;
2525

@@ -383,10 +383,7 @@ impl<'cfg> RegistryData for HttpRegistry<'cfg> {
383383

384384
// Load the registry config.
385385
if self.registry_config.is_none() && path != Path::new("config.json") {
386-
match self.config()? {
387-
Poll::Ready(_) => {}
388-
Poll::Pending => return Poll::Pending,
389-
}
386+
ready!(self.config()?);
390387
}
391388

392389
let mut handle = ops::http_handle(self.config)?;
@@ -515,11 +512,11 @@ impl<'cfg> RegistryData for HttpRegistry<'cfg> {
515512
}
516513
}
517514

518-
match self.load(Path::new(""), Path::new("config.json"), None)? {
519-
Poll::Ready(LoadResponse::Data {
515+
match ready!(self.load(Path::new(""), Path::new("config.json"), None)?) {
516+
LoadResponse::Data {
520517
raw_data,
521518
index_version: _,
522-
}) => {
519+
} => {
523520
trace!("config loaded");
524521
self.registry_config = Some(serde_json::from_slice(&raw_data)?);
525522
if paths::create_dir_all(&config_json_path.parent().unwrap()).is_ok() {
@@ -529,13 +526,12 @@ impl<'cfg> RegistryData for HttpRegistry<'cfg> {
529526
}
530527
Poll::Ready(Ok(self.registry_config.clone()))
531528
}
532-
Poll::Ready(LoadResponse::NotFound) => {
529+
LoadResponse::NotFound => {
533530
Poll::Ready(Err(anyhow::anyhow!("config.json not found in registry")))
534531
}
535-
Poll::Ready(LoadResponse::CacheValid) => {
532+
LoadResponse::CacheValid => {
536533
panic!("config.json is not stored in the index cache")
537534
}
538-
Poll::Pending => Poll::Pending,
539535
}
540536
}
541537

src/cargo/sources/registry/index.rs

Lines changed: 11 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ use std::fs;
8080
use std::io::ErrorKind;
8181
use std::path::Path;
8282
use std::str;
83-
use std::task::Poll;
83+
use std::task::{ready, Poll};
8484

8585
/// Crates.io treats hyphen and underscores as interchangeable, but the index and old Cargo do not.
8686
/// Therefore, the index must store uncanonicalized version of the name so old Cargo's can find it.
@@ -268,10 +268,7 @@ impl<'cfg> RegistryIndex<'cfg> {
268268
pub fn hash(&mut self, pkg: PackageId, load: &mut dyn RegistryData) -> Poll<CargoResult<&str>> {
269269
let req = OptVersionReq::exact(pkg.version());
270270
let summary = self.summaries(pkg.name(), &req, load)?;
271-
let summary = match summary {
272-
Poll::Ready(mut summary) => summary.next(),
273-
Poll::Pending => return Poll::Pending,
274-
};
271+
let summary = ready!(summary).next();
275272
Poll::Ready(Ok(summary
276273
.ok_or_else(|| internal(format!("no hash listed for {}", pkg)))?
277274
.summary
@@ -302,10 +299,7 @@ impl<'cfg> RegistryIndex<'cfg> {
302299
// has run previously this will parse a Cargo-specific cache file rather
303300
// than the registry itself. In effect this is intended to be a quite
304301
// cheap operation.
305-
let summaries = match self.load_summaries(name, load)? {
306-
Poll::Ready(summaries) => summaries,
307-
Poll::Pending => return Poll::Pending,
308-
};
302+
let summaries = ready!(self.load_summaries(name, load)?);
309303

310304
// Iterate over our summaries, extract all relevant ones which match our
311305
// version requirement, and then parse all corresponding rows in the
@@ -422,19 +416,19 @@ impl<'cfg> RegistryIndex<'cfg> {
422416
f: &mut dyn FnMut(Summary),
423417
) -> Poll<CargoResult<()>> {
424418
if self.config.offline() {
425-
match self.query_inner_with_online(dep, load, yanked_whitelist, f, false)? {
426-
Poll::Ready(0) => {}
427-
Poll::Ready(_) => return Poll::Ready(Ok(())),
428-
Poll::Pending => return Poll::Pending,
429-
}
430-
// If offline, and there are no matches, try again with online.
419+
// This should only return `Poll::Ready(Ok(()))` if there is at least 1 match.
420+
//
421+
// If there are 0 matches it should fall through and try again with online.
431422
// This is necessary for dependencies that are not used (such as
432423
// target-cfg or optional), but are not downloaded. Normally the
433424
// build should succeed if they are not downloaded and not used,
434425
// but they still need to resolve. If they are actually needed
435426
// then cargo will fail to download and an error message
436427
// indicating that the required dependency is unavailable while
437428
// offline will be displayed.
429+
if ready!(self.query_inner_with_online(dep, load, yanked_whitelist, f, false)?) > 0 {
430+
return Poll::Ready(Ok(()));
431+
}
438432
}
439433
self.query_inner_with_online(dep, load, yanked_whitelist, f, true)
440434
.map_ok(|_| ())
@@ -450,10 +444,7 @@ impl<'cfg> RegistryIndex<'cfg> {
450444
) -> Poll<CargoResult<usize>> {
451445
let source_id = self.source_id;
452446

453-
let summaries = match self.summaries(dep.package_name(), dep.version_req(), load)? {
454-
Poll::Ready(summaries) => summaries,
455-
Poll::Pending => return Poll::Pending,
456-
};
447+
let summaries = ready!(self.summaries(dep.package_name(), dep.version_req(), load))?;
457448

458449
let summaries = summaries
459450
// First filter summaries for `--offline`. If we're online then
@@ -582,10 +573,7 @@ impl Summaries {
582573
Err(e) => log::debug!("cache missing for {:?} error: {}", relative, e),
583574
}
584575

585-
let response = match load.load(root, relative, index_version.as_deref())? {
586-
Poll::Pending => return Poll::Pending,
587-
Poll::Ready(response) => response,
588-
};
576+
let response = ready!(load.load(root, relative, index_version.as_deref())?);
589577

590578
match response {
591579
LoadResponse::CacheValid => {

src/cargo/sources/registry/remote.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use std::fs::File;
1515
use std::mem;
1616
use std::path::Path;
1717
use std::str;
18-
use std::task::Poll;
18+
use std::task::{ready, Poll};
1919

2020
/// A remote registry is a registry that lives at a remote URL (such as
2121
/// crates.io). The git index is cloned locally, and `.crate` files are
@@ -236,13 +236,12 @@ impl<'cfg> RegistryData for RemoteRegistry<'cfg> {
236236
debug!("loading config");
237237
self.prepare()?;
238238
self.config.assert_package_cache_locked(&self.index_path);
239-
match self.load(Path::new(""), Path::new("config.json"), None)? {
240-
Poll::Ready(LoadResponse::Data { raw_data, .. }) => {
239+
match ready!(self.load(Path::new(""), Path::new("config.json"), None)?) {
240+
LoadResponse::Data { raw_data, .. } => {
241241
trace!("config loaded");
242242
Poll::Ready(Ok(Some(serde_json::from_slice(&raw_data)?)))
243243
}
244-
Poll::Ready(_) => Poll::Ready(Ok(None)),
245-
Poll::Pending => Poll::Pending,
244+
_ => Poll::Ready(Ok(None)),
246245
}
247246
}
248247

0 commit comments

Comments
 (0)