Fix precision loss in Duration::mul_f32/f64 and div_f32/f64

AndrewTKent · AndrewTKent · commit fe77711ad44a · 2026-01-27T15:35:50.000-08:00
The previous implementation converted the entire Duration to f64 before
multiplying/dividing, which caused precision loss because Duration has
~93 bits of precision (u64 secs + u32 nanos) but f64 only has 53 bits
of mantissa.

This caused issues like:
- `Duration::MAX.mul_f32(1.0)` panicking due to overflow
- `d.mul_f64(1.0)` not preserving the exact value

The fix multiplies/divides seconds and nanoseconds separately, then
combines the results. This preserves precision for the common case
while properly handling edge cases (NaN, infinity, overflow).

Changes:
- Add helper function `duration_from_float_secs()` to reduce duplication
- Add `#[track_caller]` to all four methods for better panic messages
- Add explicit infinity check with `is_infinite()`
- Check overflow before casting to u64 and use `checked_add()`
- Clamp final nanoseconds with `.max(0.0)` for floating-point errors
- Update doctest expected values to match new (correct) behavior
- Add comprehensive tests for edge cases
diff --git a/library/core/src/time.rs b/library/core/src/time.rs
@@ -20,6 +20,7 @@
 //! ```
 
 use crate::fmt;
+use crate::intrinsics;
 use crate::iter::Sum;
 use crate::num::niche_types::Nanoseconds;
 use crate::ops::{Add, AddAssign, Div, DivAssign, Mul, MulAssign, Sub, SubAssign};
@@ -1019,8 +1020,13 @@ impl Duration {
     #[must_use = "this returns the result of the operation, \
                   without modifying the original"]
     #[inline]
+    #[track_caller]
     pub fn mul_f64(self, rhs: f64) -> Duration {
-        Duration::from_secs_f64(rhs * self.as_secs_f64())
+        // Multiply seconds and nanoseconds separately to preserve precision.
+        // This avoids the precision loss from converting the full Duration to f64.
+        let secs_product = (self.secs as f64) * rhs;
+        let nanos_product = (self.nanos.as_inner() as f64) * rhs;
+        Self::duration_from_float_secs(secs_product, nanos_product, "Duration::mul_f64")
     }
 
     /// Multiplies `Duration` by `f32`.
@@ -1033,15 +1039,22 @@ impl Duration {
     /// use std::time::Duration;
     ///
     /// let dur = Duration::new(2, 700_000_000);
-    /// assert_eq!(dur.mul_f32(3.14), Duration::new(8, 478_000_641));
+    /// assert_eq!(dur.mul_f32(3.14), Duration::new(8, 478_000_283));
     /// assert_eq!(dur.mul_f32(3.14e5), Duration::new(847_800, 0));
     /// ```
     #[stable(feature = "duration_float", since = "1.38.0")]
     #[must_use = "this returns the result of the operation, \
                   without modifying the original"]
     #[inline]
+    #[track_caller]
     pub fn mul_f32(self, rhs: f32) -> Duration {
-        Duration::from_secs_f32(rhs * self.as_secs_f32())
+        // Multiply seconds and nanoseconds separately to preserve precision.
+        // This avoids the precision loss from converting the full Duration to f32.
+        // Use f64 for intermediate calculations to preserve precision.
+        let rhs_f64 = rhs as f64;
+        let secs_product = (self.secs as f64) * rhs_f64;
+        let nanos_product = (self.nanos.as_inner() as f64) * rhs_f64;
+        Self::duration_from_float_secs(secs_product, nanos_product, "Duration::mul_f32")
     }
 
     /// Divides `Duration` by `f64`.
@@ -1061,8 +1074,13 @@ impl Duration {
     #[must_use = "this returns the result of the operation, \
                   without modifying the original"]
     #[inline]
+    #[track_caller]
     pub fn div_f64(self, rhs: f64) -> Duration {
-        Duration::from_secs_f64(self.as_secs_f64() / rhs)
+        // Divide seconds and nanoseconds separately to preserve precision.
+        // This avoids the precision loss from converting the full Duration to f64.
+        let secs_quotient = (self.secs as f64) / rhs;
+        let nanos_quotient = (self.nanos.as_inner() as f64) / rhs;
+        Self::duration_from_float_secs(secs_quotient, nanos_quotient, "Duration::div_f64")
     }
 
     /// Divides `Duration` by `f32`.
@@ -1077,15 +1095,84 @@ impl Duration {
     /// let dur = Duration::new(2, 700_000_000);
     /// // note that due to rounding errors result is slightly
     /// // different from 0.859_872_611
-    /// assert_eq!(dur.div_f32(3.14), Duration::new(0, 859_872_580));
+    /// assert_eq!(dur.div_f32(3.14), Duration::new(0, 859_872_583));
     /// assert_eq!(dur.div_f32(3.14e5), Duration::new(0, 8_599));
     /// ```
     #[stable(feature = "duration_float", since = "1.38.0")]
     #[must_use = "this returns the result of the operation, \
                   without modifying the original"]
     #[inline]
+    #[track_caller]
     pub fn div_f32(self, rhs: f32) -> Duration {
-        Duration::from_secs_f32(self.as_secs_f32() / rhs)
+        // Divide seconds and nanoseconds separately to preserve precision.
+        // This avoids the precision loss from converting the full Duration to f32.
+        // Use f64 for intermediate calculations to preserve precision.
+        let rhs_f64 = rhs as f64;
+        let secs_quotient = (self.secs as f64) / rhs_f64;
+        let nanos_quotient = (self.nanos.as_inner() as f64) / rhs_f64;
+        Self::duration_from_float_secs(secs_quotient, nanos_quotient, "Duration::div_f32")
+    }
+
+    /// Helper function to create a Duration from floating-point seconds and nanoseconds.
+    /// Used by mul_f32, mul_f64, div_f32, div_f64 to avoid code duplication.
+    ///
+    /// # Arguments
+    /// * `secs_f64` - The seconds component (may have fractional part)
+    /// * `nanos_f64` - The nanoseconds component (may have fractional part)
+    /// * `fn_name` - The name of the calling function for panic messages
+    #[inline]
+    #[track_caller]
+    fn duration_from_float_secs(secs_f64: f64, nanos_f64: f64, fn_name: &str) -> Duration {
+        // Check for negative, NaN, or infinity
+        // Note: !(x >= 0.0) catches negative numbers, NaN, and -infinity
+        // We also explicitly check for +infinity which would pass the >= 0.0 check
+        if !(secs_f64 >= 0.0 && nanos_f64 >= 0.0)
+            || secs_f64.is_infinite()
+            || nanos_f64.is_infinite()
+        {
+            panic!("{fn_name}: time is negative, NaN, or infinite");
+        }
+
+        // Split seconds into integer and fractional parts
+        let secs_whole = intrinsics::truncf64(secs_f64);
+        let secs_frac = secs_f64 - secs_whole;
+
+        // Check for overflow BEFORE casting to u64
+        if secs_whole > u64::MAX as f64 {
+            panic!("overflow in {fn_name}");
+        }
+
+        // Convert fractional seconds to nanoseconds and add to nanos_f64
+        let nanos_from_secs = secs_frac * (NANOS_PER_SEC as f64);
+        let total_nanos = nanos_f64 + nanos_from_secs;
+
+        // Handle nanoseconds overflow into seconds
+        let extra_secs = intrinsics::truncf64(total_nanos / (NANOS_PER_SEC as f64));
+        // Clamp to 0.0 to handle potential floating-point errors that could make this negative
+        let final_nanos = (total_nanos - extra_secs * (NANOS_PER_SEC as f64)).max(0.0);
+
+        // Round nanoseconds to nearest integer
+        let nanos_rounded = intrinsics::roundf64(final_nanos) as u32;
+
+        // Handle edge case where rounding pushes nanos to NANOS_PER_SEC
+        let (nanos_final, secs_adjust) = if nanos_rounded >= NANOS_PER_SEC {
+            (nanos_rounded - NANOS_PER_SEC, 1u64)
+        } else {
+            (nanos_rounded, 0u64)
+        };
+
+        // Compute total seconds with overflow checking
+        let secs_whole_u64 = secs_whole as u64;
+        let extra_secs_u64 = extra_secs as u64;
+
+        let total_secs = secs_whole_u64
+            .checked_add(extra_secs_u64)
+            .and_then(|s| s.checked_add(secs_adjust));
+
+        match total_secs {
+            Some(secs) => Duration::new(secs, nanos_final),
+            None => panic!("overflow in {fn_name}"),
+        }
     }
 
     /// Divides `Duration` by `Duration` and returns `f64`.
diff --git a/library/coretests/tests/time.rs b/library/coretests/tests/time.rs
@@ -598,3 +598,248 @@ fn from_neg_zero() {
     assert_eq!(Duration::from_secs_f32(-0.0), Duration::ZERO);
     assert_eq!(Duration::from_secs_f64(-0.0), Duration::ZERO);
 }
+
+/// Test that multiplying/dividing by 1.0 preserves precision.
+/// Regression test for <https://github.com/rust-lang/rust/issues/149794>
+#[test]
+fn mul_div_float_precision() {
+    // Multiplying by 1.0 should preserve the exact value
+    let d1 = Duration::from_millis(100);
+    assert_eq!(d1.mul_f32(1.0), d1);
+    assert_eq!(d1.mul_f64(1.0), d1);
+
+    let d2 = Duration::new(1, 111);
+    assert_eq!(d2.mul_f32(1.0), d2);
+    assert_eq!(d2.mul_f64(1.0), d2);
+
+    // Dividing by 1.0 should preserve the exact value
+    assert_eq!(d1.div_f32(1.0), d1);
+    assert_eq!(d1.div_f64(1.0), d1);
+    assert_eq!(d2.div_f32(1.0), d2);
+    assert_eq!(d2.div_f64(1.0), d2);
+
+    // Multiplying by small integers should match integer multiplication
+    assert_eq!(d1.mul_f32(2.0), d1 * 2);
+    assert_eq!(d1.mul_f32(3.0), d1 * 3);
+    assert_eq!(d1.mul_f64(2.0), d1 * 2);
+    assert_eq!(d1.mul_f64(3.0), d1 * 3);
+
+    assert_eq!(d2.mul_f32(2.0), d2 * 2);
+    assert_eq!(d2.mul_f32(3.0), d2 * 3);
+    assert_eq!(d2.mul_f64(2.0), d2 * 2);
+    assert_eq!(d2.mul_f64(3.0), d2 * 3);
+
+    // Duration::MAX.mul_f32(1.0) should not panic and should preserve the value
+    assert_eq!(Duration::MAX.mul_f32(1.0), Duration::MAX);
+    assert_eq!(Duration::MAX.mul_f64(1.0), Duration::MAX);
+    assert_eq!(Duration::MAX.div_f32(1.0), Duration::MAX);
+    assert_eq!(Duration::MAX.div_f64(1.0), Duration::MAX);
+
+    // Fractional multipliers should work correctly
+    assert_eq!(Duration::from_secs(10).mul_f64(0.5), Duration::from_secs(5));
+    assert_eq!(Duration::from_secs(10).div_f64(2.0), Duration::from_secs(5));
+
+    // Nanosecond precision should be maintained
+    let d3 = Duration::new(0, 123_456_789);
+    assert_eq!(d3.mul_f64(1.0), d3);
+    assert_eq!(d3.div_f64(1.0), d3);
+}
+
+#[test]
+#[should_panic]
+fn mul_f64_negative() {
+    let _ = Duration::from_secs(1).mul_f64(-1.0);
+}
+
+#[test]
+#[should_panic]
+fn mul_f32_negative() {
+    let _ = Duration::from_secs(1).mul_f32(-1.0);
+}
+
+#[test]
+#[should_panic]
+fn div_f64_negative() {
+    let _ = Duration::from_secs(1).div_f64(-1.0);
+}
+
+#[test]
+#[should_panic]
+fn div_f32_negative() {
+    let _ = Duration::from_secs(1).div_f32(-1.0);
+}
+
+#[test]
+#[should_panic]
+fn mul_f64_nan() {
+    let _ = Duration::from_secs(1).mul_f64(f64::NAN);
+}
+
+#[test]
+#[should_panic]
+fn mul_f32_nan() {
+    let _ = Duration::from_secs(1).mul_f32(f32::NAN);
+}
+
+#[test]
+#[should_panic]
+fn div_f64_nan() {
+    let _ = Duration::from_secs(1).div_f64(f64::NAN);
+}
+
+#[test]
+#[should_panic]
+fn div_f32_nan() {
+    let _ = Duration::from_secs(1).div_f32(f32::NAN);
+}
+
+#[test]
+#[should_panic]
+fn mul_f64_infinity() {
+    let _ = Duration::from_secs(1).mul_f64(f64::INFINITY);
+}
+
+#[test]
+#[should_panic]
+fn mul_f32_infinity() {
+    let _ = Duration::from_secs(1).mul_f32(f32::INFINITY);
+}
+
+#[test]
+#[should_panic]
+fn div_f64_zero() {
+    let _ = Duration::from_secs(1).div_f64(0.0);
+}
+
+#[test]
+#[should_panic]
+fn div_f32_zero() {
+    let _ = Duration::from_secs(1).div_f32(0.0);
+}
+
+#[test]
+#[should_panic]
+fn mul_f64_overflow() {
+    let _ = Duration::MAX.mul_f64(2.0);
+}
+
+#[test]
+#[should_panic]
+fn mul_f32_overflow() {
+    let _ = Duration::MAX.mul_f32(2.0);
+}
+
+#[test]
+#[should_panic]
+fn mul_f64_neg_infinity() {
+    let _ = Duration::from_secs(1).mul_f64(f64::NEG_INFINITY);
+}
+
+#[test]
+#[should_panic]
+fn mul_f32_neg_infinity() {
+    let _ = Duration::from_secs(1).mul_f32(f32::NEG_INFINITY);
+}
+
+#[test]
+fn div_f64_neg_infinity() {
+    // Division by negative infinity produces -0.0, which is treated as zero
+    assert_eq!(Duration::from_secs(1).div_f64(f64::NEG_INFINITY), Duration::ZERO);
+}
+
+#[test]
+fn div_f32_neg_infinity() {
+    assert_eq!(Duration::from_secs(1).div_f32(f32::NEG_INFINITY), Duration::ZERO);
+}
+
+#[test]
+fn div_f64_infinity() {
+    assert_eq!(Duration::from_secs(1).div_f64(f64::INFINITY), Duration::ZERO);
+    assert_eq!(Duration::MAX.div_f64(f64::INFINITY), Duration::ZERO);
+}
+
+#[test]
+fn div_f32_infinity() {
+    assert_eq!(Duration::from_secs(1).div_f32(f32::INFINITY), Duration::ZERO);
+    assert_eq!(Duration::MAX.div_f32(f32::INFINITY), Duration::ZERO);
+}
+
+#[test]
+fn mul_div_zero_duration() {
+    assert_eq!(Duration::ZERO.mul_f64(1.0), Duration::ZERO);
+    assert_eq!(Duration::ZERO.mul_f64(1000.0), Duration::ZERO);
+    assert_eq!(Duration::ZERO.mul_f32(1.0), Duration::ZERO);
+    assert_eq!(Duration::ZERO.mul_f32(1000.0), Duration::ZERO);
+
+    assert_eq!(Duration::ZERO.div_f64(1.0), Duration::ZERO);
+    assert_eq!(Duration::ZERO.div_f64(1000.0), Duration::ZERO);
+    assert_eq!(Duration::ZERO.div_f32(1.0), Duration::ZERO);
+    assert_eq!(Duration::ZERO.div_f32(1000.0), Duration::ZERO);
+
+    assert_eq!(Duration::from_secs(100).mul_f64(0.0), Duration::ZERO);
+    assert_eq!(Duration::from_secs(100).mul_f32(0.0), Duration::ZERO);
+}
+
+#[test]
+fn mul_div_subnormal() {
+    let subnormal_f64 = f64::from_bits(1);
+    let subnormal_f32 = f32::from_bits(1);
+
+    assert_eq!(Duration::from_secs(1).mul_f64(subnormal_f64), Duration::ZERO);
+    assert_eq!(Duration::from_secs(1).mul_f32(subnormal_f32), Duration::ZERO);
+
+    assert_eq!(Duration::from_nanos(1).mul_f64(1e-20), Duration::ZERO);
+    assert_eq!(Duration::from_nanos(1).mul_f32(1e-20), Duration::ZERO);
+}
+
+#[test]
+fn mul_div_rounding_boundaries() {
+    let almost_one_sec = Duration::new(0, 999_999_999);
+    assert_eq!(almost_one_sec.mul_f64(1.0), almost_one_sec);
+    assert_eq!(almost_one_sec.div_f64(1.0), almost_one_sec);
+
+    let half_sec = Duration::from_millis(500);
+    assert_eq!(half_sec.mul_f64(2.0), Duration::from_secs(1));
+    assert_eq!(half_sec.mul_f32(2.0), Duration::from_secs(1));
+
+    let d = Duration::new(0, 999_999_999);
+    assert_eq!(d.mul_f64(2.0), Duration::new(1, 999_999_998));
+
+    let d = Duration::from_secs(1);
+    assert_eq!(d.mul_f64(2.0), Duration::from_secs(2));
+    assert_eq!(d.mul_f64(4.0), Duration::from_secs(4));
+    assert_eq!(d.mul_f64(8.0), Duration::from_secs(8));
+    assert_eq!(d.mul_f64(0.5), Duration::from_millis(500));
+    assert_eq!(d.mul_f64(0.25), Duration::from_millis(250));
+    assert_eq!(d.mul_f64(0.125), Duration::from_millis(125));
+}
+
+#[test]
+fn mul_div_small_durations() {
+    let one_ns = Duration::from_nanos(1);
+    assert_eq!(one_ns.mul_f64(1.0), one_ns);
+    assert_eq!(one_ns.mul_f64(2.0), Duration::from_nanos(2));
+    assert_eq!(one_ns.div_f64(1.0), one_ns);
+
+    let one_us = Duration::from_micros(1);
+    assert_eq!(one_us.mul_f64(1.0), one_us);
+    assert_eq!(one_us.mul_f64(1000.0), Duration::from_millis(1));
+    assert_eq!(one_us.div_f64(1.0), one_us);
+
+    let one_ms = Duration::from_millis(1);
+    assert_eq!(one_ms.mul_f64(1.0), one_ms);
+    assert_eq!(one_ms.mul_f64(1000.0), Duration::from_secs(1));
+    assert_eq!(one_ms.div_f64(1.0), one_ms);
+}
+
+#[test]
+fn mul_div_large_factors() {
+    let d = Duration::from_nanos(1);
+    assert_eq!(d.mul_f64(1e9), Duration::from_secs(1));
+    assert_eq!(d.mul_f64(1e6), Duration::from_millis(1));
+
+    let d = Duration::from_secs(1);
+    assert_eq!(d.div_f64(1e9), Duration::from_nanos(1));
+    assert_eq!(d.div_f64(1e6), Duration::from_micros(1));
+    assert_eq!(d.div_f64(1e3), Duration::from_millis(1));
+}
