Skip to content

Commit d2644d9

Browse files
authored
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
2 parents fb98f7a + f1d4e48 commit d2644d9

File tree

2 files changed

+5
-4
lines changed

2 files changed

+5
-4
lines changed

library/core/src/time.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -656,10 +656,10 @@ impl Duration {
656656
#[rustc_const_stable(feature = "duration_consts_2", since = "1.58.0")]
657657
pub const fn checked_div(self, rhs: u32) -> Option<Duration> {
658658
if rhs != 0 {
659-
let secs = self.secs / (rhs as u64);
660-
let carry = self.secs - secs * (rhs as u64);
661-
let extra_nanos = carry * (NANOS_PER_SEC as u64) / (rhs as u64);
662-
let nanos = self.nanos.0 / rhs + (extra_nanos as u32);
659+
let (secs, extra_secs) = (self.secs / (rhs as u64), self.secs % (rhs as u64));
660+
let (mut nanos, extra_nanos) = (self.nanos.0 / rhs, self.nanos.0 % rhs);
661+
nanos +=
662+
((extra_secs * (NANOS_PER_SEC as u64) + extra_nanos as u64) / (rhs as u64)) as u32;
663663
debug_assert!(nanos < NANOS_PER_SEC);
664664
Some(Duration::new(secs, nanos))
665665
} else {

library/core/tests/time.rs

+1
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,7 @@ fn saturating_mul() {
170170
fn div() {
171171
assert_eq!(Duration::new(0, 1) / 2, Duration::new(0, 0));
172172
assert_eq!(Duration::new(1, 1) / 3, Duration::new(0, 333_333_333));
173+
assert_eq!(Duration::new(1, 1) / 7, Duration::new(0, 142_857_143));
173174
assert_eq!(Duration::new(99, 999_999_000) / 100, Duration::new(0, 999_999_990));
174175
}
175176

0 commit comments

Comments
 (0)