fixed a subtle bug for `Naive(Date)Time + Duration`. (#37)

The exact condition is that the `Duration` to be added is negative
and has a fractional second part. This was not a problem when
`Duration` was Chrono's own type, but a new code for external (then
libstd, now libtime) `Duration` was not tested for this condition
by accident. Consequently this condition caused an underflow in
the fractional part, resulting in a slight inaccuracy.

Fixes #37.
This commit is contained in:
Kang Seonghoon 2015-05-15 02:06:34 +09:00
parent 6d4054a3a7
commit 5ff21f4077
2 changed files with 20 additions and 2 deletions

View File

@ -458,5 +458,13 @@ mod tests {
assert_eq!(dt.format("%c").to_string(), "Sat Jun 30 23:59:60 2012"); assert_eq!(dt.format("%c").to_string(), "Sat Jun 30 23:59:60 2012");
assert_eq!(dt.format("%s").to_string(), "1341100799"); // not 1341100800, it's intentional. assert_eq!(dt.format("%s").to_string(), "1341100799"); // not 1341100800, it's intentional.
} }
#[test]
fn test_datetime_add_sub_invariant() { // issue #37
let base = NaiveDate::from_ymd(2000, 1, 1).and_hms(0, 0, 0);
let t = -946684799990000;
let time = base + Duration::microseconds(t);
assert_eq!(t, (time - base).num_microseconds().unwrap());
}
} }

View File

@ -200,8 +200,14 @@ impl Add<Duration> for NaiveTime {
fn add(self, rhs: Duration) -> NaiveTime { fn add(self, rhs: Duration) -> NaiveTime {
// there is no direct interface in `Duration` to get only the nanosecond part, // there is no direct interface in `Duration` to get only the nanosecond part,
// so we need to do the additional calculation here. // so we need to do the additional calculation here.
let rhs2 = rhs - Duration::seconds(rhs.num_seconds()); let mut rhssecs = rhs.num_seconds();
let mut secs = self.secs + (rhs.num_seconds() % 86400 + 86400) as u32; let mut rhs2 = rhs - Duration::seconds(rhssecs);
if rhs2 < Duration::zero() { // possible when rhs < 0
rhssecs -= 1;
rhs2 = rhs2 + Duration::seconds(1);
}
debug_assert!(rhs2 >= Duration::zero());
let mut secs = self.secs + (rhssecs % 86400 + 86400) as u32;
let mut nanos = self.frac + rhs2.num_nanoseconds().unwrap() as u32; let mut nanos = self.frac + rhs2.num_nanoseconds().unwrap() as u32;
// always ignore leap seconds after the current whole second // always ignore leap seconds after the current whole second
@ -363,6 +369,10 @@ mod tests {
check(hmsm(3, 5, 7, 900), Duration::seconds(86399), hmsm(3, 5, 6, 900)); // overwrap check(hmsm(3, 5, 7, 900), Duration::seconds(86399), hmsm(3, 5, 6, 900)); // overwrap
check(hmsm(3, 5, 7, 900), Duration::seconds(-86399), hmsm(3, 5, 8, 900)); check(hmsm(3, 5, 7, 900), Duration::seconds(-86399), hmsm(3, 5, 8, 900));
check(hmsm(3, 5, 7, 900), Duration::days(12345), hmsm(3, 5, 7, 900)); check(hmsm(3, 5, 7, 900), Duration::days(12345), hmsm(3, 5, 7, 900));
// regression tests for #37
check(hmsm(0, 0, 0, 0), Duration::milliseconds(-990), hmsm(23, 59, 59, 10));
check(hmsm(0, 0, 0, 0), Duration::milliseconds(-9990), hmsm(23, 59, 50, 10));
} }
#[test] #[test]