Skip to content

Commit

Permalink
Implement [Partial]Ord for Range (pubgrub-rs#250)
Browse files Browse the repository at this point in the history
* Implement `[Partial]Ord` for `Range` (pubgrub-rs#31)

In uv, we want to have a stable ordering of certain types that
`Range<Version>`, both for determinism internally and for
deterministically ordered output files.

This PR adds `impl<V: PartialOrd> PartialOrd for Range<V>` and `impl<V:
Ord> Ord for Range<V>` implementations to `Range`. We use a simple
ordering scheme where we zip the segments and compare all bounds in
order. If all bounds are equal, the longer range is considered greater.
(And if all zipped bounds are equal and we have the same number of
segments, the ranges are equal). Not that this is distinct from contains
operations, `r1 < r2` (implemented by `Ord`) and `r1 ⊂ r2` (`subset_of`)
have no relationship.

* Better docs

* Add reverse sorting check
  • Loading branch information
konstin authored Aug 22, 2024
1 parent 68fd200 commit eb528c3
Showing 1 changed file with 177 additions and 0 deletions.
177 changes: 177 additions & 0 deletions src/range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,154 @@ impl<V: Ord> Range<V> {
}
}

/// Implementing `PartialOrd` for start `Bound` of an interval.
///
/// Legend: `∞` is unbounded, `[1,2]` is `>=1,<=2`, `]1,2[` is `>1,<2`.
///
/// ```text
/// left: ∞-------]
/// right: [-----]
/// left is smaller, since it starts earlier.
///
/// left: [-----]
/// right: ]-----]
/// left is smaller, since it starts earlier.
/// ```
fn cmp_bounds_start<V: PartialOrd>(left: Bound<&V>, right: Bound<&V>) -> Option<Ordering> {
Some(match (left, right) {
// left: ∞-----
// right: ∞-----
(Unbounded, Unbounded) => Ordering::Equal,
// left: [---
// right: ∞-----
(Included(_left), Unbounded) => Ordering::Greater,
// left: ]---
// right: ∞-----
(Excluded(_left), Unbounded) => Ordering::Greater,
// left: ∞-----
// right: [---
(Unbounded, Included(_right)) => Ordering::Less,
// left: [----- OR [----- OR [-----
// right: [--- OR [----- OR [---
(Included(left), Included(right)) => left.partial_cmp(right)?,
(Excluded(left), Included(right)) => match left.partial_cmp(right)? {
// left: ]-----
// right: [---
Ordering::Less => Ordering::Less,
// left: ]-----
// right: [---
Ordering::Equal => Ordering::Greater,
// left: ]---
// right: [-----
Ordering::Greater => Ordering::Greater,
},
// left: ∞-----
// right: ]---
(Unbounded, Excluded(_right)) => Ordering::Less,
(Included(left), Excluded(right)) => match left.partial_cmp(right)? {
// left: [-----
// right: ]---
Ordering::Less => Ordering::Less,
// left: [-----
// right: ]---
Ordering::Equal => Ordering::Less,
// left: [---
// right: ]-----
Ordering::Greater => Ordering::Greater,
},
// left: ]----- OR ]----- OR ]---
// right: ]--- OR ]----- OR ]-----
(Excluded(left), Excluded(right)) => left.partial_cmp(right)?,
})
}

/// Implementing `PartialOrd` for end `Bound` of an interval.
///
/// We flip the unbounded ranges from `-∞` to `∞`, while `V`-valued bounds checks remain the same.
///
/// Legend: `∞` is unbounded, `[1,2]` is `>=1,<=2`, `]1,2[` is `>1,<2`.
///
/// ```text
/// left: [--------∞
/// right: [-----]
/// left is greater, since it starts earlier.
///
/// left: [-----[
/// right: [-----]
/// left is smaller, since it ends earlier.
/// ```
fn cmp_bounds_end<V: PartialOrd>(left: Bound<&V>, right: Bound<&V>) -> Option<Ordering> {
Some(match (left, right) {
// left: -----∞
// right: -----∞
(Unbounded, Unbounded) => Ordering::Equal,
// left: ---]
// right: -----∞
(Included(_left), Unbounded) => Ordering::Less,
// left: ---[
// right: -----∞
(Excluded(_left), Unbounded) => Ordering::Less,
// left: -----∞
// right: ---]
(Unbounded, Included(_right)) => Ordering::Greater,
// left: -----] OR -----] OR ---]
// right: ---] OR -----] OR -----]
(Included(left), Included(right)) => left.partial_cmp(right)?,
(Excluded(left), Included(right)) => match left.partial_cmp(right)? {
// left: ---[
// right: -----]
Ordering::Less => Ordering::Less,
// left: -----[
// right: -----]
Ordering::Equal => Ordering::Less,
// left: -----[
// right: ---]
Ordering::Greater => Ordering::Greater,
},
(Unbounded, Excluded(_right)) => Ordering::Greater,
(Included(left), Excluded(right)) => match left.partial_cmp(right)? {
// left: ---]
// right: -----[
Ordering::Less => Ordering::Less,
// left: -----]
// right: -----[
Ordering::Equal => Ordering::Greater,
// left: -----]
// right: ---[
Ordering::Greater => Ordering::Greater,
},
// left: -----[ OR -----[ OR ---[
// right: ---[ OR -----[ OR -----[
(Excluded(left), Excluded(right)) => left.partial_cmp(right)?,
})
}

impl<V: PartialOrd> PartialOrd for Range<V> {
/// A simple ordering scheme where we zip the segments and compare all bounds in order. If all
/// bounds are equal, the longer range is considered greater. (And if all zipped bounds are
/// equal and we have the same number of segments, the ranges are equal).
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
for (left, right) in self.segments.iter().zip(other.segments.iter()) {
let start_cmp = cmp_bounds_start(left.start_bound(), right.start_bound())?;
if start_cmp != Ordering::Equal {
return Some(start_cmp);
}
let end_cmp = cmp_bounds_end(left.end_bound(), right.end_bound())?;
if end_cmp != Ordering::Equal {
return Some(end_cmp);
}
}
Some(self.segments.len().cmp(&other.segments.len()))
}
}

impl<V: Ord> Ord for Range<V> {
fn cmp(&self, other: &Self) -> Ordering {
self.partial_cmp(other)
.expect("PartialOrd must be `Some(Ordering)` for types that implement `Ord`")
}
}

/// The ordering of the version wrt to the interval.
/// ```text
/// |-------|
Expand Down Expand Up @@ -1069,4 +1217,33 @@ pub mod tests {
range.simplify(versions.into_iter())
);
}

#[test]
fn version_ord() {
let versions: &[Range<u32>] = &[
Range::strictly_lower_than(1u32),
Range::lower_than(1u32),
Range::singleton(1u32),
Range::between(1u32, 3u32),
Range::higher_than(1u32),
Range::strictly_higher_than(1u32),
Range::singleton(2u32),
Range::singleton(2u32).union(&Range::singleton(3u32)),
Range::singleton(2u32)
.union(&Range::singleton(3u32))
.union(&Range::singleton(4u32)),
Range::singleton(2u32).union(&Range::singleton(4u32)),
Range::singleton(3u32),
];

let mut versions_sorted = versions.to_vec();
versions_sorted.sort();
assert_eq!(versions_sorted, versions);

// Check that the sorting isn't just stable because we're returning equal.
let mut version_reverse_sorted = versions.to_vec();
version_reverse_sorted.reverse();
version_reverse_sorted.sort();
assert_eq!(version_reverse_sorted, versions);
}
}

0 comments on commit eb528c3

Please sign in to comment.