Skip to content

Commit f60f62a

Browse files
committed
Fix: Remove non-compliant Ord and Eq implementations for AssetCapacity
The implementations for these traits shouldn't panic and can cause UB if they do. The only thing we need them for is the `min()` method, but we can just implement this manually instead. Fixes #1206.
1 parent 5389d44 commit f60f62a

1 file changed

Lines changed: 21 additions & 12 deletions

File tree

src/asset/capacity.rs

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,23 @@ pub enum AssetCapacity {
1313
Discrete(u32, Capacity),
1414
}
1515

16+
impl AssetCapacity {
17+
/// Return the smaller of `self` or `other`.
18+
///
19+
/// # Panics
20+
///
21+
/// Panics if the comparison is not meaningful. This happens if either `AssetCapacity` contains
22+
/// a NaN value, one is discrete and the other continuous or if both are discrete and the unit
23+
/// size differs.
24+
pub fn min(self, other: AssetCapacity) -> AssetCapacity {
25+
match self.partial_cmp(&other) {
26+
None => panic!("Comparing invalid AssetCapacity values ({self:?} and {other:?})"),
27+
Some(Ordering::Greater) => other,
28+
_ => self,
29+
}
30+
}
31+
}
32+
1633
impl Add for AssetCapacity {
1734
type Output = Self;
1835

@@ -49,23 +66,15 @@ impl Sub for AssetCapacity {
4966
}
5067
}
5168

52-
impl Eq for AssetCapacity {}
53-
5469
impl PartialOrd for AssetCapacity {
5570
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
56-
Some(self.cmp(other))
57-
}
58-
}
59-
60-
impl Ord for AssetCapacity {
61-
fn cmp(&self, other: &Self) -> Ordering {
6271
match (self, other) {
63-
(AssetCapacity::Continuous(a), AssetCapacity::Continuous(b)) => a.total_cmp(b),
72+
(AssetCapacity::Continuous(a), AssetCapacity::Continuous(b)) => a.partial_cmp(b),
6473
(AssetCapacity::Discrete(units1, size1), AssetCapacity::Discrete(units2, size2)) => {
65-
Self::check_same_unit_size(*size1, *size2);
66-
units1.cmp(units2)
74+
// NB: Also returns `None` if either is NaN
75+
(*size1 == *size2).then(|| units1.cmp(units2))
6776
}
68-
_ => panic!("Cannot compare different types of AssetCapacity ({self:?} and {other:?})"),
77+
_ => None,
6978
}
7079
}
7180
}

0 commit comments

Comments
 (0)