Hide Ord implementation behind feature flag

Closes korken89#36
jannic-dev-forks · Sep 2, 2022 · 87d4300 · 87d4300
1 parent 0a48063
commit 87d4300
Show file tree

Hide file tree

Showing 6 changed files with 59 additions and 39 deletions.
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -127,7 +127,7 @@ jobs:
         with:
           use-cross: false
           command: test
-          args: --target=${{ matrix.target }}
+          args: --target=${{ matrix.target }} --all-features
 
   # Refs: https://github.com/rust-lang/crater/blob/9ab6f9697c901c4a44025cf0a39b73ad5b37d198/.github/workflows/bors.yml#L125-L149
   #

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -16,6 +16,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ### Changed
 
+- Hide `impl Ord for Instant` and `impl PartialOrd for Instant` behind a new
+  feature flag `lenient_cmp`. (**Breaking change**)
+
 ## [v0.3.6] 
 
 ### Fixed

diff --git a/Cargo.toml b/Cargo.toml
@@ -19,5 +19,8 @@ gcd = ">=2.1,<3.0"
 version = ">=0.2.0,<0.4"
 optional = true
 
+[features]
+lenient_cmp = []
+
 [package.metadata.docs.rs]
 all-features = true
diff --git a/README.md b/README.md
@@ -17,3 +17,9 @@ This library is a heavily inspired of `std::chrono`'s `Duration` from C++ which
 * Selection of base happens at compile time
   * A common problem is that run time changing of base robs us of a lot of optimization opportunities, but since there are no traits and short-hands select the correct base at compile time.
 
+## Feature flags
+
+* `lenient_cmp`: Implement `Ord` for instants with the assumption that the compared instants are close to each other (no more than half the available numeric range apart).
+  This is useful, as it guarantees that equations like "timestamp + 1 microsecond > timestamp" are always true, even if timestamp wraps around (jumps from `u32::MAX` back to `0`).
+  However, this breaks the formal contract of the `Ord` trait, which requires a total ordering: `a < b` and `b < c` implies `a < c` for all `a`, `b`, `c`.
+
diff --git a/src/instant.rs b/src/instant.rs
@@ -175,13 +175,15 @@ macro_rules! impl_instant_for_integer {
             }
         }
 
+        #[cfg(feature="lenient_cmp")]
         impl<const NOM: u32, const DENOM: u32> PartialOrd for Instant<$i, NOM, DENOM> {
             #[inline]
             fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
                 Some(self.const_cmp(*other))
             }
         }
 
+        #[cfg(feature="lenient_cmp")]
         impl<const NOM: u32, const DENOM: u32> Ord for Instant<$i, NOM, DENOM> {
             #[inline]
             fn cmp(&self, other: &Self) -> Ordering {

diff --git a/src/lib.rs b/src/lib.rs
@@ -764,25 +764,28 @@ mod test {
 
     #[test]
     fn instant_compare_u32() {
-        // Wrapping
-        assert!(
-            Instant::<u32, 1, 1_000>::from_ticks(1)
-                > Instant::<u32, 1, 1_000>::from_ticks(u32::MAX)
-        );
-        assert!(
-            Instant::<u32, 1, 1_000>::from_ticks(u32::MAX - 1)
-                < Instant::<u32, 1, 1_000>::from_ticks(u32::MAX)
-        );
-
-        // Non-wrapping
-        assert!(Instant::<u32, 1, 1_000>::from_ticks(2) > Instant::<u32, 1, 1_000>::from_ticks(1));
-        assert!(Instant::<u32, 1, 1_000>::from_ticks(2) >= Instant::<u32, 1, 1_000>::from_ticks(1));
-        assert!(Instant::<u32, 1, 1_000>::from_ticks(1) >= Instant::<u32, 1, 1_000>::from_ticks(1));
-        assert!(Instant::<u32, 1, 1_000>::from_ticks(1) < Instant::<u32, 1, 1_000>::from_ticks(2));
-        assert!(Instant::<u32, 1, 1_000>::from_ticks(1) <= Instant::<u32, 1, 1_000>::from_ticks(1));
-        assert!(Instant::<u32, 1, 1_000>::from_ticks(1) <= Instant::<u32, 1, 1_000>::from_ticks(2));
-        assert!(Instant::<u32, 1, 1_000>::from_ticks(1) == Instant::<u32, 1, 1_000>::from_ticks(1));
-        assert!(Instant::<u32, 1, 1_000>::from_ticks(1) != Instant::<u32, 1, 1_000>::from_ticks(2));
+        #[cfg(feature = "lenient_cmp")]
+        {
+            // Wrapping
+            assert!(
+                Instant::<u32, 1, 1_000>::from_ticks(1)
+                    > Instant::<u32, 1, 1_000>::from_ticks(u32::MAX)
+            );
+            assert!(
+                Instant::<u32, 1, 1_000>::from_ticks(u32::MAX - 1)
+                    < Instant::<u32, 1, 1_000>::from_ticks(u32::MAX)
+            );
+
+            // Non-wrapping
+            assert!(Instant::<u32, 1, 1_000>::from_ticks(2) > Instant::<u32, 1, 1_000>::from_ticks(1));
+            assert!(Instant::<u32, 1, 1_000>::from_ticks(2) >= Instant::<u32, 1, 1_000>::from_ticks(1));
+            assert!(Instant::<u32, 1, 1_000>::from_ticks(1) >= Instant::<u32, 1, 1_000>::from_ticks(1));
+            assert!(Instant::<u32, 1, 1_000>::from_ticks(1) < Instant::<u32, 1, 1_000>::from_ticks(2));
+            assert!(Instant::<u32, 1, 1_000>::from_ticks(1) <= Instant::<u32, 1, 1_000>::from_ticks(1));
+            assert!(Instant::<u32, 1, 1_000>::from_ticks(1) <= Instant::<u32, 1, 1_000>::from_ticks(2));
+            assert!(Instant::<u32, 1, 1_000>::from_ticks(1) == Instant::<u32, 1, 1_000>::from_ticks(1));
+            assert!(Instant::<u32, 1, 1_000>::from_ticks(1) != Instant::<u32, 1, 1_000>::from_ticks(2));
+        }
 
         // Checked duration since non-wrapping
         assert_eq!(
@@ -816,25 +819,28 @@ mod test {
 
     #[test]
     fn instant_compare_u64() {
-        // Wrapping
-        assert!(
-            Instant::<u64, 1, 1_000>::from_ticks(1)
-                > Instant::<u64, 1, 1_000>::from_ticks(u64::MAX)
-        );
-        assert!(
-            Instant::<u64, 1, 1_000>::from_ticks(u64::MAX - 1)
-                < Instant::<u64, 1, 1_000>::from_ticks(u64::MAX)
-        );
-
-        // Non-wrapping
-        assert!(Instant::<u64, 1, 1_000>::from_ticks(2) > Instant::<u64, 1, 1_000>::from_ticks(1));
-        assert!(Instant::<u64, 1, 1_000>::from_ticks(2) >= Instant::<u64, 1, 1_000>::from_ticks(1));
-        assert!(Instant::<u64, 1, 1_000>::from_ticks(1) >= Instant::<u64, 1, 1_000>::from_ticks(1));
-        assert!(Instant::<u64, 1, 1_000>::from_ticks(1) < Instant::<u64, 1, 1_000>::from_ticks(2));
-        assert!(Instant::<u64, 1, 1_000>::from_ticks(1) <= Instant::<u64, 1, 1_000>::from_ticks(1));
-        assert!(Instant::<u64, 1, 1_000>::from_ticks(1) <= Instant::<u64, 1, 1_000>::from_ticks(2));
-        assert!(Instant::<u64, 1, 1_000>::from_ticks(1) == Instant::<u64, 1, 1_000>::from_ticks(1));
-        assert!(Instant::<u64, 1, 1_000>::from_ticks(1) != Instant::<u64, 1, 1_000>::from_ticks(2));
+        #[cfg(feature = "lenient_cmp")]
+        {
+            // Wrapping
+            assert!(
+                Instant::<u64, 1, 1_000>::from_ticks(1)
+                    > Instant::<u64, 1, 1_000>::from_ticks(u64::MAX)
+            );
+            assert!(
+                Instant::<u64, 1, 1_000>::from_ticks(u64::MAX - 1)
+                    < Instant::<u64, 1, 1_000>::from_ticks(u64::MAX)
+            );
+
+            // Non-wrapping
+            assert!(Instant::<u64, 1, 1_000>::from_ticks(2) > Instant::<u64, 1, 1_000>::from_ticks(1));
+            assert!(Instant::<u64, 1, 1_000>::from_ticks(2) >= Instant::<u64, 1, 1_000>::from_ticks(1));
+            assert!(Instant::<u64, 1, 1_000>::from_ticks(1) >= Instant::<u64, 1, 1_000>::from_ticks(1));
+            assert!(Instant::<u64, 1, 1_000>::from_ticks(1) < Instant::<u64, 1, 1_000>::from_ticks(2));
+            assert!(Instant::<u64, 1, 1_000>::from_ticks(1) <= Instant::<u64, 1, 1_000>::from_ticks(1));
+            assert!(Instant::<u64, 1, 1_000>::from_ticks(1) <= Instant::<u64, 1, 1_000>::from_ticks(2));
+            assert!(Instant::<u64, 1, 1_000>::from_ticks(1) == Instant::<u64, 1, 1_000>::from_ticks(1));
+            assert!(Instant::<u64, 1, 1_000>::from_ticks(1) != Instant::<u64, 1, 1_000>::from_ticks(2));
+        }
 
         // Checked duration since non-wrapping
         assert_eq!(