Skip to content

Commit

Permalink
simplify SourceID Ord/Eq (#14980)
Browse files Browse the repository at this point in the history
### What does this PR try to resolve?

This is a followup to #14800. Like that PR, this is a small incremental
change that does not pull its own weight. If this PR is accepted, the
next PR will unlock large performance wins. I am not posting them
together because the logic of why this PR is correct is subtle and
deserves to be discussed and reviewed without unrelated code changes.

### How should we test and review this PR?

All tests pass on all commits. This **should** be reviewed one commit at
a time.

### Additional information

I pushed one commit at a time, so that CI can confirm that the assert
(in the first commit) is never hit.
  • Loading branch information
weihanglo authored Jan 7, 2025
2 parents 83615cf + a84336e commit b696870
Showing 1 changed file with 3 additions and 14 deletions.
17 changes: 3 additions & 14 deletions src/cargo/core/source_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -591,21 +591,10 @@ impl Ord for SourceId {
return Ordering::Equal;
}

// Sort first based on `kind`, deferring to the URL comparison below if
// Sort first based on `kind`, deferring to the URL comparison if
// the kinds are equal.
match self.inner.kind.cmp(&other.inner.kind) {
Ordering::Equal => {}
other => return other,
}

// If the `kind` and the `url` are equal, then for git sources we also
// ensure that the canonical urls are equal.
match (&self.inner.kind, &other.inner.kind) {
(SourceKind::Git(_), SourceKind::Git(_)) => {
self.inner.canonical_url.cmp(&other.inner.canonical_url)
}
_ => self.inner.url.cmp(&other.inner.url),
}
let ord_kind = self.inner.kind.cmp(&other.inner.kind);
ord_kind.then_with(|| self.inner.canonical_url.cmp(&other.inner.canonical_url))
}
}

Expand Down

0 comments on commit b696870

Please sign in to comment.