Avoid panics on checked, saturating arithmetic

time-rs · Oct 25, 2021 · ea8e71c · ea8e71c
1 parent 4217882
commit ea8e71c
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 2 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -43,6 +43,7 @@ The format is based on [Keep a Changelog]. This project adheres to [Semantic Ver
 
 - Cross-building to Windows now succeeds.
 - A parse error on a `UtcOffset` component now indicates the error comes from the offset.
+- `Duration::checked_sub` and `Duration::saturating_sub` no longer panic in edge cases.
 
 ## 0.3.3 [2021-09-25]
 

diff --git a/src/duration.rs b/src/duration.rs
@@ -556,7 +556,18 @@ impl Duration {
     /// assert_eq!(5.seconds().checked_sub(10.seconds()), Some((-5).seconds()));
     /// ```
     pub const fn checked_sub(self, rhs: Self) -> Option<Self> {
-        self.checked_add(Self::new_unchecked(-rhs.seconds, -rhs.nanoseconds))
+        let mut seconds = const_try_opt!(self.seconds.checked_sub(rhs.seconds));
+        let mut nanoseconds = self.nanoseconds - rhs.nanoseconds;
+
+        if nanoseconds >= 1_000_000_000 || seconds < 0 && nanoseconds > 0 {
+            nanoseconds -= 1_000_000_000;
+            seconds = const_try_opt!(seconds.checked_add(1));
+        } else if nanoseconds <= -1_000_000_000 || seconds > 0 && nanoseconds < 0 {
+            nanoseconds += 1_000_000_000;
+            seconds = const_try_opt!(seconds.checked_sub(1));
+        }
+
+        Some(Self::new_unchecked(seconds, nanoseconds))
     }
 
     /// Computes `self * rhs`, returning `None` if an overflow occurred.
@@ -653,7 +664,30 @@ impl Duration {
     /// assert_eq!(5.seconds().saturating_sub(10.seconds()), (-5).seconds());
     /// ```
     pub const fn saturating_sub(self, rhs: Self) -> Self {
-        self.saturating_add(Self::new_unchecked(-rhs.seconds, -rhs.nanoseconds))
+        let (mut seconds, overflow) = self.seconds.overflowing_sub(rhs.seconds);
+        if overflow {
+            if self.seconds > 0 {
+                return Self::MAX;
+            }
+            return Self::MIN;
+        }
+        let mut nanoseconds = self.nanoseconds - rhs.nanoseconds;
+
+        if nanoseconds >= 1_000_000_000 || seconds < 0 && nanoseconds > 0 {
+            nanoseconds -= 1_000_000_000;
+            seconds = match seconds.checked_add(1) {
+                Some(seconds) => seconds,
+                None => return Self::MAX,
+            };
+        } else if nanoseconds <= -1_000_000_000 || seconds > 0 && nanoseconds < 0 {
+            nanoseconds += 1_000_000_000;
+            seconds = match seconds.checked_sub(1) {
+                Some(seconds) => seconds,
+                None => return Self::MIN,
+            };
+        }
+
+        Self::new_unchecked(seconds, nanoseconds)
     }
 
     /// Computes `self * rhs`, saturating if an overflow occurred.

diff --git a/tests/integration/duration.rs b/tests/integration/duration.rs
@@ -257,13 +257,18 @@ fn checked_add() {
     assert_eq!(5.seconds().checked_add(5.seconds()), Some(10.seconds()));
     assert_eq!(Duration::MAX.checked_add(1.nanoseconds()), None);
     assert_eq!((-5).seconds().checked_add(5.seconds()), Some(0.seconds()));
+    assert_eq!(
+        1.seconds().checked_add((-1).milliseconds()),
+        Some(999.milliseconds())
+    );
 }
 
 #[test]
 fn checked_sub() {
     assert_eq!(5.seconds().checked_sub(5.seconds()), Some(0.seconds()));
     assert_eq!(Duration::MIN.checked_sub(1.nanoseconds()), None);
     assert_eq!(5.seconds().checked_sub(10.seconds()), Some((-5).seconds()));
+    assert_eq!(Duration::MIN.checked_sub(Duration::MIN), Some(0.seconds()));
 }
 
 #[test]
@@ -298,6 +303,10 @@ fn saturating_add() {
         1_600.milliseconds().saturating_add(1_600.milliseconds()),
         3_200.milliseconds()
     );
+    assert_eq!(
+        1.seconds().saturating_add((-1).milliseconds()),
+        (999).milliseconds()
+    );
 }
 
 #[test]
@@ -308,11 +317,24 @@ fn saturating_sub() {
         Duration::MAX.saturating_sub((-1).nanoseconds()),
         Duration::MAX
     );
+    assert_eq!(Duration::MAX.saturating_sub((-1).seconds()), Duration::MAX);
     assert_eq!(5.seconds().saturating_sub(10.seconds()), (-5).seconds());
     assert_eq!(
         (-1_600).milliseconds().saturating_sub(1_600.milliseconds()),
         (-3_200).milliseconds()
     );
+    assert_eq!(0.seconds().saturating_sub(Duration::MIN), Duration::MIN);
+    assert_eq!(Duration::MIN.saturating_sub(5.seconds()), Duration::MIN);
+    assert_eq!(
+        1_200.milliseconds().saturating_sub(600.milliseconds()),
+        600.milliseconds()
+    );
+    assert_eq!(
+        (-1_200)
+            .milliseconds()
+            .saturating_sub((-600).milliseconds()),
+        (-600).milliseconds()
+    );
 }
 
 #[test]