diff --git a/integer/src/cmp.rs b/integer/src/cmp.rs index ec08e346..3c91916d 100644 --- a/integer/src/cmp.rs +++ b/integer/src/cmp.rs @@ -8,7 +8,6 @@ use crate::{ ibig::IBig, repr::TypedReprRef::{self, *}, ubig::UBig, - Sign::*, }; use core::cmp::Ordering; @@ -24,6 +23,7 @@ pub fn cmp_same_len(lhs: &[Word], rhs: &[Word]) -> Ordering { /// # Panics /// /// Panic if lhs or rhs has leading zero words (including the case where lhs == 0 or rhs == 0) +#[inline] pub fn cmp_in_place(lhs: &[Word], rhs: &[Word]) -> Ordering { debug_assert!(*lhs.last().unwrap() != 0 && *rhs.last().unwrap() != 0); lhs.len() @@ -53,7 +53,9 @@ impl<'a> Ord for TypedReprRef<'a> { impl Ord for UBig { #[inline] fn cmp(&self, other: &UBig) -> Ordering { - self.repr().cmp(&other.repr()) + // UBig is non-negative, so comparing magnitudes is the full + // comparison; no sign handling needed. + self.0.magnitude_cmp(&other.0) } } @@ -67,14 +69,7 @@ impl PartialOrd for UBig { impl Ord for IBig { #[inline] fn cmp(&self, other: &IBig) -> Ordering { - let (lhs_sign, lhs_mag) = self.as_sign_repr(); - let (rhs_sign, rhs_mag) = other.as_sign_repr(); - match (lhs_sign, rhs_sign) { - (Positive, Positive) => lhs_mag.cmp(&rhs_mag), - (Positive, Negative) => Ordering::Greater, - (Negative, Positive) => Ordering::Less, - (Negative, Negative) => rhs_mag.cmp(&lhs_mag), - } + self.0.signed_cmp(&other.0) } } diff --git a/integer/src/repr.rs b/integer/src/repr.rs index c92a8a69..738f66d5 100644 --- a/integer/src/repr.rs +++ b/integer/src/repr.rs @@ -8,6 +8,7 @@ use crate::{ Sign, }; use core::{ + cmp::Ordering, fmt::{self, Write}, hash::{Hash, Hasher}, hint::unreachable_unchecked, @@ -559,6 +560,62 @@ impl PartialEq for Repr { } impl Eq for Repr {} +impl Repr { + /// Compare magnitudes (absolute values) of two `Repr`s, ignoring sign. + /// + /// This bypasses the `as_sign_typed` → `TypedReprRef::cmp` chain for the + /// inline case: when both sides are inline we read both words from each + /// union and compare a single `DoubleWord`. Same-scale heap comparison + /// falls through to the existing length-then-words logic. + #[inline] + pub fn magnitude_cmp(&self, other: &Repr) -> Ordering { + let abs_a = self.capacity.get().unsigned_abs(); + let abs_b = other.capacity.get().unsigned_abs(); + // SAFETY: capacity discriminates inline vs heap; for either branch we + // touch only the live union field. + unsafe { + if abs_a <= 2 && abs_b <= 2 { + let dw_a = double_word(self.data.inline[0], self.data.inline[1]); + let dw_b = double_word(other.data.inline[0], other.data.inline[1]); + dw_a.cmp(&dw_b) + } else if abs_a <= 2 { + Ordering::Less + } else if abs_b <= 2 { + Ordering::Greater + } else { + let slice_a = slice::from_raw_parts(self.data.heap.0, self.data.heap.1); + let slice_b = slice::from_raw_parts(other.data.heap.0, other.data.heap.1); + let len_cmp = slice_a.len().cmp(&slice_b.len()); + if len_cmp != Ordering::Equal { + return len_cmp; + } + slice_a.iter().rev().cmp(slice_b.iter().rev()) + } + } + } + + /// Compare two signed (`IBig`-shaped) `Repr`s. + /// + /// Uses the canonical-positive-zero invariant: zero always has positive + /// capacity, so a sign disagreement is a strict ordering and never a tie. + #[inline] + pub fn signed_cmp(&self, other: &Repr) -> Ordering { + let pos_a = self.capacity.get() > 0; + let pos_b = other.capacity.get() > 0; + match (pos_a, pos_b) { + (true, false) => return Ordering::Greater, + (false, true) => return Ordering::Less, + _ => {} + } + let mag = self.magnitude_cmp(other); + if pos_a { + mag + } else { + mag.reverse() + } + } +} + impl fmt::Debug for Repr { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let (sign, words) = self.as_sign_slice(); diff --git a/integer/tests/cmp.rs b/integer/tests/cmp.rs index e066f93f..7e02db82 100644 --- a/integer/tests/cmp.rs +++ b/integer/tests/cmp.rs @@ -59,3 +59,48 @@ fn test_abs_ord() { assert!(ubig!(12).abs_cmp(&ibig!(-12)).is_eq()); assert!(ubig!(12).abs_cmp(&ibig!(-14)).is_le()); } + +#[test] +fn test_cmp_across_representations() { + // Regression test for UBig::cmp (magnitude_cmp) / IBig::cmp (signed_cmp): + // capacity only selects the inline-vs-heap branch; ordering is decided by + // the actual words. Every pair below shares a scale/capacity yet differs in + // value (or sits on the inline/heap boundary). + use core::cmp::Ordering::*; + let ubig_cases = [ + // both single-word inline (capacity 1) + (ubig!(5), ubig!(7), Less), + (ubig!(5), ubig!(5), Equal), + // both two-word inline (capacity 2): 2^64 vs 2^64+1, 2^96 vs 2^64 + (ubig!(0x10000000000000000), ubig!(0x10000000000000001), Less), + (ubig!(0x1000000000000000000000000), ubig!(0x10000000000000000), Greater), + // capacity 1 vs capacity 2 (mixed inline scale): 2^64-1 vs 2^64 + (ubig!(0xffffffffffffffff), ubig!(0x10000000000000000), Less), + // both heap, same length, differ only in the low word + ((ubig!(1) << 130) + ubig!(1), (ubig!(1) << 130) + ubig!(2), Less), + // both heap, different length + (ubig!(1) << 200, ubig!(1) << 130, Greater), + // inline/heap boundary: 2^128-1 (inline) vs 2^128 (heap) + ((ubig!(1) << 128) - ubig!(1), ubig!(1) << 128, Less), + ]; + for (a, b, want) in &ubig_cases { + assert_eq!(a.cmp(b), *want, "{a} cmp {b}"); + assert_eq!(b.cmp(a), want.reverse(), "{b} cmp {a}"); + assert_eq!(a.partial_cmp(b), Some(*want)); + } + let ibig_cases = [ + (ibig!(-5), ibig!(3), Less), // neg < pos + (ibig!(-5), ibig!(-3), Less), // more negative < less negative + (ibig!(0), ibig!(-1), Greater), + (ibig!(0), ibig!(1), Less), + (ibig!(-5), ibig!(-5), Equal), + // across inline/heap, both negative + (-(ibig!(1) << 200), -(ibig!(1) << 130), Less), + (-((ibig!(1) << 128) - ibig!(1)), -(ibig!(1) << 128), Greater), + ]; + for (a, b, want) in &ibig_cases { + assert_eq!(a.cmp(b), *want, "{a} cmp {b}"); + assert_eq!(b.cmp(a), want.reverse(), "{b} cmp {a}"); + assert_eq!(a.partial_cmp(b), Some(*want)); + } +}