diff options
| author | Matthias Krüger <matthias.krueger@famsik.de> | 2023-08-28 08:13:57 +0200 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2023-08-28 08:13:57 +0200 |
| commit | d2644d9fe97d4ffae93af836975903f5800c7fb0 (patch) | |
| tree | c86df372df8682cb0671791c6a552ef4e66c5901 | |
| parent | fb98f7adc3f74e331c21e5ec9bf2577b707b1523 (diff) | |
| parent | f1d4e48c9c6409aff557b0117d48f2d7d1e05280 (diff) | |
| download | rust-d2644d9fe97d4ffae93af836975903f5800c7fb0.tar.gz rust-d2644d9fe97d4ffae93af836975903f5800c7fb0.zip | |
Rollup merge of #114238 - jhpratt:fix-duration-div, r=thomcc
Fix implementation of `Duration::checked_div`
I ran across this while running some sanity checks on `time`. Quickcheck immediately found a bug, and as I'd modified the code from `std` I knew there was a bug here as well.
tl;dr this code fails ([playground](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=1189a3efcdfc192c27d6d87815359353))
```rust
use std::time::Duration;
fn main() {
assert_eq!(
Duration::new(1, 1).checked_div(7),
Some(Duration::new(0, 142_857_143)),
);
}
```
The existing code determines that 1/7 = 0 (seconds), 1/7 = 0 (nanoseconds), 1 billion / 7 = 142,857,142 (extra nanoseconds). The billion comes from multiplying the remainder of the seconds (1) by the number of nanoseconds in a second. However, **this wrongly ignores any remaining nanoseconds**. This PR takes that into consideration, adds a test, and also changes the roundabout way of calculating the remainder into directly computing it.
Note: This is _not_ a rounding error. This result divides evenly.
`@rustbot` label +A-time +C-bug +S-waiting-on-reviewer +T-libs
| -rw-r--r-- | library/core/src/time.rs | 8 | ||||
| -rw-r--r-- | library/core/tests/time.rs | 1 |
2 files changed, 5 insertions, 4 deletions
diff --git a/library/core/src/time.rs b/library/core/src/time.rs index b08d5782ab6..1e8d28979e6 100644 --- a/library/core/src/time.rs +++ b/library/core/src/time.rs @@ -656,10 +656,10 @@ impl Duration { #[rustc_const_stable(feature = "duration_consts_2", since = "1.58.0")] pub const fn checked_div(self, rhs: u32) -> Option<Duration> { if rhs != 0 { - let secs = self.secs / (rhs as u64); - let carry = self.secs - secs * (rhs as u64); - let extra_nanos = carry * (NANOS_PER_SEC as u64) / (rhs as u64); - let nanos = self.nanos.0 / rhs + (extra_nanos as u32); + let (secs, extra_secs) = (self.secs / (rhs as u64), self.secs % (rhs as u64)); + let (mut nanos, extra_nanos) = (self.nanos.0 / rhs, self.nanos.0 % rhs); + nanos += + ((extra_secs * (NANOS_PER_SEC as u64) + extra_nanos as u64) / (rhs as u64)) as u32; debug_assert!(nanos < NANOS_PER_SEC); Some(Duration::new(secs, nanos)) } else { diff --git a/library/core/tests/time.rs b/library/core/tests/time.rs index 872611937cc..bd6e63edbb9 100644 --- a/library/core/tests/time.rs +++ b/library/core/tests/time.rs @@ -170,6 +170,7 @@ fn saturating_mul() { fn div() { assert_eq!(Duration::new(0, 1) / 2, Duration::new(0, 0)); assert_eq!(Duration::new(1, 1) / 3, Duration::new(0, 333_333_333)); + assert_eq!(Duration::new(1, 1) / 7, Duration::new(0, 142_857_143)); assert_eq!(Duration::new(99, 999_999_000) / 100, Duration::new(0, 999_999_990)); } |
