Critical Generic comparison bug found

.github/workflows/ci.yml

@@ -43,7 +43,7 @@ jobs:
         uses: actions-rs/cargo@v1
         with:
           command: test
-          args: --lib --all-features -- --test-threads=1
+          args: --all-features -- --test-threads=1
 
       - name: Test Documentation
         uses: actions-rs/cargo@v1

src/duration.rs

@@ -443,14 +443,42 @@ pub struct Generic<T> {
     scaling_factor: Fraction,
 }
 
+impl<T: TimeInt> Generic<T> {
+    /// Try to create a new, equivalent `Generic` with the given _scaling factor_
+    fn try_into_scaling_factor(self, scaling_factor: Fraction) -> Result<Self, ConversionError> {
+        let new_int = TimeInt::checked_mul_fraction(
+            &self.integer,
+            &self
+                .scaling_factor
+                .checked_div(&scaling_factor)
+                .ok_or(ConversionError::Overflow)?,
+        )
+        .ok_or(ConversionError::Overflow)?;
+
+        Ok(Self::new(new_int, scaling_factor))
+    }
+}
+
 impl<T: TimeInt> PartialOrd<Generic<T>> for Generic<T> {
     /// See [Comparisons](trait.Duration.html#comparisons)
     fn partial_cmp(&self, rhs: &Generic<T>) -> Option<core::cmp::Ordering> {
-        Some(
-            self.integer
-                .checked_mul_fraction(&self.scaling_factor)?
-                .cmp(&rhs.integer.checked_mul_fraction(&rhs.scaling_factor)?),
-        )
+        if self.scaling_factor == rhs.scaling_factor {
+            Some(self.integer.cmp(&rhs.integer))
+        } else if self.scaling_factor < rhs.scaling_factor {
+            // convert to the smaller scaling factor (rhs -> self)
+            // if conversion fails, we know self is less than rhs
+            match rhs.try_into_scaling_factor(self.scaling_factor) {
+                Ok(converted_rhs) => Some(self.integer.cmp(&converted_rhs.integer)),
+                Err(_) => Some(core::cmp::Ordering::Less),
+            }
+        } else {
+            // convert to the smaller scaling factor (self -> rhs)
+            // if conversion fails, we know rhs is less than self
+            match self.try_into_scaling_factor(rhs.scaling_factor) {
+                Ok(converted_self) => Some(converted_self.integer.cmp(&rhs.integer)),
+                Err(_) => Some(core::cmp::Ordering::Greater),
+            }
+        }
     }
 }
 
@@ -884,9 +912,9 @@ pub mod units {
                     /// See [Comparisons](trait.Duration.html#comparisons)
                     fn partial_cmp(&self, rhs: &$big<RhsInt>) -> Option<core::cmp::Ordering> {
                         match Self::try_from(*rhs) {
-                        Ok(rhs) => Some(self.integer().cmp(&rhs.integer())),
-                        Err(_) => Some(core::cmp::Ordering::Less),
-                    }
+                            Ok(rhs) => Some(self.integer().cmp(&rhs.integer())),
+                            Err(_) => Some(core::cmp::Ordering::Less),
+                        }
                     }
                 }
             )+

src/time_int.rs

@@ -32,6 +32,14 @@ pub trait TimeInt:
     fn checked_div_fraction(&self, fraction: &Fraction) -> Option<Self> {
         self.checked_mul_fraction(&fraction.recip())
     }
+
+    /// Moves an integer into a comparable base for checking
+    fn checked_same_base(&self, fraction: &Fraction, rhs_fraction: &Fraction) -> Option<Self> {
+       let a_n = *fraction.numerator();
+       let b_d = *rhs_fraction.denominator();
+
+       self.checked_mul(&(b_d.into()))?.checked_mul(&(a_n.into()))
+    }
 }
 
 impl TimeInt for u32 {}

tests/duration.rs

@@ -56,6 +56,53 @@ fn comparisons() {
         Generic::new(1_000u32, Fraction::new(1, 1_000))
             == Generic::new(2_000u32, Fraction::new(1, 2_000))
     );
+
+    // Generic comparisons
+    assert!(
+        Generic::new(100u32, Fraction::new(1, 1000)) == Generic::new(10u32, Fraction::new(1, 100))
+    );
+    assert!(
+        Generic::new(100u32, Fraction::new(1, 1000)) <= Generic::new(10u32, Fraction::new(1, 100))
+    );
+    assert!(
+        Generic::new(100u32, Fraction::new(1, 1000)) <= Generic::new(11u32, Fraction::new(1, 100))
+    );
+    assert!(
+        Generic::new(200u32, Fraction::new(1, 1000)) >= Generic::new(10u32, Fraction::new(1, 100))
+    );
+    assert!(
+        Generic::new(200u32, Fraction::new(1, 1000)) >= Generic::new(10u32, Fraction::new(1, 100))
+    );
+    assert!(
+        Generic::new(100u32, Fraction::new(1, 1000)) != Generic::new(100u32, Fraction::new(1, 100))
+    );
+    assert!(
+        Generic::new(101u32, Fraction::new(1, 1000)) > Generic::new(100u32, Fraction::new(1, 1000))
+    );
+    assert!(
+        Generic::new(u32::MAX, Fraction::new(1, 100))
+            > Generic::new(100u32, Fraction::new(1, 1000))
+    );
+
+    /* Generic comparison stress test:
+    loop {
+        let mut rands: [u32; 6] = [0; 6];
+        for rand in rands.iter_mut() {
+            unsafe {
+                loop {
+                    core::arch::x86_64::_rdrand32_step(rand);
+                    if *rand != 0 {
+                        break;
+                    }
+                }
+            }
+        }
+        println!("rands: {:#?}", rands);
+        assert!(
+            Generic::new(rands[0], Fraction::new(rands[1], rands[2]))
+                != Generic::new(rands[3], Fraction::new(rands[4], rands[5]))
+        );
+    } */
 }
 
 #[test]