Skip to content

Commit 03b8738

Browse files
committed
Fix precision loss in Duration::mul_f32/f64 and div_f32/f64
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
1 parent e96bb7e commit 03b8738

File tree

2 files changed

+337
-7
lines changed

2 files changed

+337
-7
lines changed

library/core/src/time.rs

Lines changed: 92 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,10 @@
1919
//! assert_eq!(total, Duration::new(10, 7));
2020
//! ```
2121
22-
use crate::fmt;
2322
use crate::iter::Sum;
2423
use crate::num::niche_types::Nanoseconds;
2524
use crate::ops::{Add, AddAssign, Div, DivAssign, Mul, MulAssign, Sub, SubAssign};
25+
use crate::{fmt, intrinsics};
2626

2727
const NANOS_PER_SEC: u32 = 1_000_000_000;
2828
const NANOS_PER_MILLI: u32 = 1_000_000;
@@ -1019,8 +1019,13 @@ impl Duration {
10191019
#[must_use = "this returns the result of the operation, \
10201020
without modifying the original"]
10211021
#[inline]
1022+
#[track_caller]
10221023
pub fn mul_f64(self, rhs: f64) -> Duration {
1023-
Duration::from_secs_f64(rhs * self.as_secs_f64())
1024+
// Multiply seconds and nanoseconds separately to preserve precision.
1025+
// This avoids the precision loss from converting the full Duration to f64.
1026+
let secs_product = (self.secs as f64) * rhs;
1027+
let nanos_product = (self.nanos.as_inner() as f64) * rhs;
1028+
Self::duration_from_float_secs(secs_product, nanos_product, "Duration::mul_f64")
10241029
}
10251030

10261031
/// Multiplies `Duration` by `f32`.
@@ -1033,15 +1038,22 @@ impl Duration {
10331038
/// use std::time::Duration;
10341039
///
10351040
/// let dur = Duration::new(2, 700_000_000);
1036-
/// assert_eq!(dur.mul_f32(3.14), Duration::new(8, 478_000_641));
1041+
/// assert_eq!(dur.mul_f32(3.14), Duration::new(8, 478_000_283));
10371042
/// assert_eq!(dur.mul_f32(3.14e5), Duration::new(847_800, 0));
10381043
/// ```
10391044
#[stable(feature = "duration_float", since = "1.38.0")]
10401045
#[must_use = "this returns the result of the operation, \
10411046
without modifying the original"]
10421047
#[inline]
1048+
#[track_caller]
10431049
pub fn mul_f32(self, rhs: f32) -> Duration {
1044-
Duration::from_secs_f32(rhs * self.as_secs_f32())
1050+
// Multiply seconds and nanoseconds separately to preserve precision.
1051+
// This avoids the precision loss from converting the full Duration to f32.
1052+
// Use f64 for intermediate calculations to preserve precision.
1053+
let rhs_f64 = rhs as f64;
1054+
let secs_product = (self.secs as f64) * rhs_f64;
1055+
let nanos_product = (self.nanos.as_inner() as f64) * rhs_f64;
1056+
Self::duration_from_float_secs(secs_product, nanos_product, "Duration::mul_f32")
10451057
}
10461058

10471059
/// Divides `Duration` by `f64`.
@@ -1061,8 +1073,13 @@ impl Duration {
10611073
#[must_use = "this returns the result of the operation, \
10621074
without modifying the original"]
10631075
#[inline]
1076+
#[track_caller]
10641077
pub fn div_f64(self, rhs: f64) -> Duration {
1065-
Duration::from_secs_f64(self.as_secs_f64() / rhs)
1078+
// Divide seconds and nanoseconds separately to preserve precision.
1079+
// This avoids the precision loss from converting the full Duration to f64.
1080+
let secs_quotient = (self.secs as f64) / rhs;
1081+
let nanos_quotient = (self.nanos.as_inner() as f64) / rhs;
1082+
Self::duration_from_float_secs(secs_quotient, nanos_quotient, "Duration::div_f64")
10661083
}
10671084

10681085
/// Divides `Duration` by `f32`.
@@ -1077,15 +1094,83 @@ impl Duration {
10771094
/// let dur = Duration::new(2, 700_000_000);
10781095
/// // note that due to rounding errors result is slightly
10791096
/// // different from 0.859_872_611
1080-
/// assert_eq!(dur.div_f32(3.14), Duration::new(0, 859_872_580));
1097+
/// assert_eq!(dur.div_f32(3.14), Duration::new(0, 859_872_583));
10811098
/// assert_eq!(dur.div_f32(3.14e5), Duration::new(0, 8_599));
10821099
/// ```
10831100
#[stable(feature = "duration_float", since = "1.38.0")]
10841101
#[must_use = "this returns the result of the operation, \
10851102
without modifying the original"]
10861103
#[inline]
1104+
#[track_caller]
10871105
pub fn div_f32(self, rhs: f32) -> Duration {
1088-
Duration::from_secs_f32(self.as_secs_f32() / rhs)
1106+
// Divide seconds and nanoseconds separately to preserve precision.
1107+
// This avoids the precision loss from converting the full Duration to f32.
1108+
// Use f64 for intermediate calculations to preserve precision.
1109+
let rhs_f64 = rhs as f64;
1110+
let secs_quotient = (self.secs as f64) / rhs_f64;
1111+
let nanos_quotient = (self.nanos.as_inner() as f64) / rhs_f64;
1112+
Self::duration_from_float_secs(secs_quotient, nanos_quotient, "Duration::div_f32")
1113+
}
1114+
1115+
/// Helper function to create a Duration from floating-point seconds and nanoseconds.
1116+
/// Used by mul_f32, mul_f64, div_f32, div_f64 to avoid code duplication.
1117+
///
1118+
/// # Arguments
1119+
/// * `secs_f64` - The seconds component (may have fractional part)
1120+
/// * `nanos_f64` - The nanoseconds component (may have fractional part)
1121+
/// * `fn_name` - The name of the calling function for panic messages
1122+
#[inline]
1123+
#[track_caller]
1124+
fn duration_from_float_secs(secs_f64: f64, nanos_f64: f64, fn_name: &str) -> Duration {
1125+
// Check for negative, NaN, or infinity
1126+
// Note: !(x >= 0.0) catches negative numbers, NaN, and -infinity
1127+
// We also explicitly check for +infinity which would pass the >= 0.0 check
1128+
if !(secs_f64 >= 0.0 && nanos_f64 >= 0.0)
1129+
|| secs_f64.is_infinite()
1130+
|| nanos_f64.is_infinite()
1131+
{
1132+
panic!("{fn_name}: time is negative, NaN, or infinite");
1133+
}
1134+
1135+
// Split seconds into integer and fractional parts
1136+
let secs_whole = intrinsics::truncf64(secs_f64);
1137+
let secs_frac = secs_f64 - secs_whole;
1138+
1139+
// Check for overflow BEFORE casting to u64
1140+
if secs_whole > u64::MAX as f64 {
1141+
panic!("overflow in {fn_name}");
1142+
}
1143+
1144+
// Convert fractional seconds to nanoseconds and add to nanos_f64
1145+
let nanos_from_secs = secs_frac * (NANOS_PER_SEC as f64);
1146+
let total_nanos = nanos_f64 + nanos_from_secs;
1147+
1148+
// Handle nanoseconds overflow into seconds
1149+
let extra_secs = intrinsics::truncf64(total_nanos / (NANOS_PER_SEC as f64));
1150+
// Clamp to 0.0 to handle potential floating-point errors that could make this negative
1151+
let final_nanos = (total_nanos - extra_secs * (NANOS_PER_SEC as f64)).max(0.0);
1152+
1153+
// Round nanoseconds to nearest integer
1154+
let nanos_rounded = intrinsics::roundf64(final_nanos) as u32;
1155+
1156+
// Handle edge case where rounding pushes nanos to NANOS_PER_SEC
1157+
let (nanos_final, secs_adjust) = if nanos_rounded >= NANOS_PER_SEC {
1158+
(nanos_rounded - NANOS_PER_SEC, 1u64)
1159+
} else {
1160+
(nanos_rounded, 0u64)
1161+
};
1162+
1163+
// Compute total seconds with overflow checking
1164+
let secs_whole_u64 = secs_whole as u64;
1165+
let extra_secs_u64 = extra_secs as u64;
1166+
1167+
let total_secs =
1168+
secs_whole_u64.checked_add(extra_secs_u64).and_then(|s| s.checked_add(secs_adjust));
1169+
1170+
match total_secs {
1171+
Some(secs) => Duration::new(secs, nanos_final),
1172+
None => panic!("overflow in {fn_name}"),
1173+
}
10891174
}
10901175

10911176
/// Divides `Duration` by `Duration` and returns `f64`.

0 commit comments

Comments
 (0)