From d50546a5925b8689f6bcca7155a7af89f2839a32 Mon Sep 17 00:00:00 2001 From: Kang Seonghoon Date: Sat, 6 Aug 2016 19:06:28 +0900 Subject: [PATCH] More documentation for NaiveTime, and addition fixes. While writing documentation tests for NaiveTime it was found that the addition involving leap seconds is *still* slightly broken. (A consequence of having less tests, well.) The addition routine has been rewritten to be explicit about leap seconds while passing all other tests, so the rewrite does not change the intention. --- src/datetime.rs | 6 +- src/format/mod.rs | 4 +- src/naive/date.rs | 21 +++++++ src/naive/time.rs | 151 +++++++++++++++++++++++++++++++++++++++++----- src/offset/mod.rs | 17 +++++- 5 files changed, 177 insertions(+), 22 deletions(-) diff --git a/src/datetime.rs b/src/datetime.rs index d1d6b60..a0326cc 100644 --- a/src/datetime.rs +++ b/src/datetime.rs @@ -11,7 +11,7 @@ use std::cmp::Ordering; use std::ops::{Add, Sub}; use {Weekday, Timelike, Datelike}; -use offset::{TimeZone, Offset}; +use offset::{TimeZone, Offset, add_with_leapsecond}; use offset::utc::UTC; use offset::local::Local; use offset::fixed::FixedOffset; @@ -49,7 +49,7 @@ impl DateTime { /// Unlike `date`, this is not associated to the time zone. #[inline] pub fn time(&self) -> NaiveTime { - self.datetime.time() + self.offset.local_minus_utc() + add_with_leapsecond(&self.datetime.time(), &self.offset.local_minus_utc()) } /// Returns the number of non-leap seconds since January 1, 1970 0:00:00 UTC @@ -141,7 +141,7 @@ impl DateTime { /// Returns a view to the naive local datetime. #[inline] pub fn naive_local(&self) -> NaiveDateTime { - self.datetime + self.offset.local_minus_utc() + add_with_leapsecond(&self.datetime, &self.offset.local_minus_utc()) } } diff --git a/src/format/mod.rs b/src/format/mod.rs index 271c4be..cf10831 100644 --- a/src/format/mod.rs +++ b/src/format/mod.rs @@ -10,7 +10,7 @@ use std::error::Error; use {Datelike, Timelike}; use div::{div_floor, mod_floor}; use duration::Duration; -use offset::Offset; +use offset::{Offset, add_with_leapsecond}; use naive::date::NaiveDate; use naive::time::NaiveTime; @@ -303,7 +303,7 @@ pub fn format<'a, I>(w: &mut fmt::Formatter, date: Option<&NaiveDate>, time: Opt (Some(d), Some(t), None) => Some(d.and_time(*t).timestamp()), (Some(d), Some(t), Some(&(_, off))) => - Some((d.and_time(*t) - off).timestamp()), + Some(add_with_leapsecond(&d.and_time(*t), &-off).timestamp()), (_, _, _) => None }), }; diff --git a/src/naive/date.rs b/src/naive/date.rs index 06f629b..3815a2a 100644 --- a/src/naive/date.rs +++ b/src/naive/date.rs @@ -852,6 +852,17 @@ impl NaiveDate { /// assert_eq!(d.format_with_items(fmt.clone()).to_string(), "2015-09-05"); /// assert_eq!(d.format("%Y-%m-%d").to_string(), "2015-09-05"); /// ~~~~ + /// + /// The resulting `DelayedFormat` can be formatted directly via the `Display` trait. + /// + /// ~~~~ + /// use chrono::NaiveDate; + /// use chrono::format::strftime::StrftimeItems; + /// + /// let fmt = StrftimeItems::new("%Y-%m-%d"); + /// let d = NaiveDate::from_ymd(2015, 9, 5); + /// assert_eq!(format!("{}", d.format_with_items(fmt.clone())), "2015-09-05"); + /// ~~~~ #[inline] pub fn format_with_items<'a, I>(&self, items: I) -> DelayedFormat where I: Iterator> + Clone { @@ -881,6 +892,16 @@ impl NaiveDate { /// assert_eq!(d.format("%Y-%m-%d").to_string(), "2015-09-05"); /// assert_eq!(d.format("%A, %-d %B, %C%y").to_string(), "Saturday, 5 September, 2015"); /// ~~~~ + /// + /// The resulting `DelayedFormat` can be formatted directly via the `Display` trait. + /// + /// ~~~~ + /// use chrono::NaiveDate; + /// + /// let d = NaiveDate::from_ymd(2015, 9, 5); + /// assert_eq!(format!("{}", d.format("%Y-%m-%d")), "2015-09-05"); + /// assert_eq!(format!("{}", d.format("%A, %-d %B, %C%y")), "Saturday, 5 September, 2015"); + /// ~~~~ #[inline] pub fn format<'a>(&self, fmt: &'a str) -> DelayedFormat> { self.format_with_items(StrftimeItems::new(fmt)) diff --git a/src/naive/time.rs b/src/naive/time.rs index 9b5524b..09c1808 100644 --- a/src/naive/time.rs +++ b/src/naive/time.rs @@ -406,6 +406,18 @@ impl NaiveTime { /// assert_eq!(t.format_with_items(fmt.clone()).to_string(), "23:56:04"); /// assert_eq!(t.format("%H:%M:%S").to_string(), "23:56:04"); /// ~~~~ + /// + /// The resulting `DelayedFormat` can be formatted directly via the `Display` trait. + /// + /// ~~~~ + /// use chrono::NaiveTime; + /// use chrono::format::strftime::StrftimeItems; + /// + /// let fmt = StrftimeItems::new("%H:%M:%S"); + /// let t = NaiveTime::from_hms(23, 56, 4); + /// assert_eq!(format!("{}", t.format_with_items(fmt.clone())), "23:56:04"); + /// assert_eq!(format!("{}", t.format("%H:%M:%S")), "23:56:04"); + /// ~~~~ #[inline] pub fn format_with_items<'a, I>(&self, items: I) -> DelayedFormat where I: Iterator> + Clone { @@ -436,6 +448,17 @@ impl NaiveTime { /// assert_eq!(t.format("%H:%M:%S%.6f").to_string(), "23:56:04.012345"); /// assert_eq!(t.format("%-I:%M %p").to_string(), "11:56 PM"); /// ~~~~ + /// + /// The resulting `DelayedFormat` can be formatted directly via the `Display` trait. + /// + /// ~~~~ + /// use chrono::NaiveTime; + /// + /// let t = NaiveTime::from_hms_nano(23, 56, 4, 12_345_678); + /// assert_eq!(format!("{}", t.format("%H:%M:%S")), "23:56:04"); + /// assert_eq!(format!("{}", t.format("%H:%M:%S%.6f")), "23:56:04.012345"); + /// assert_eq!(format!("{}", t.format("%-I:%M %p")), "11:56 PM"); + /// ~~~~ #[inline] pub fn format<'a>(&self, fmt: &'a str) -> DelayedFormat> { self.format_with_items(StrftimeItems::new(fmt)) @@ -661,30 +684,125 @@ impl hash::Hash for NaiveTime { } } +/// An addition of `Duration` to `NaiveTime` wraps around and never overflows or underflows. +/// In particular the addition ignores integral number of days. +/// +/// As a part of Chrono's [leap second handling](./index.html#leap-second-handling), +/// the addition assumes that **there is no leap second ever**, +/// except when the `NaiveTime` itself represents a leap second +/// in which case the assumption becomes that **there is exactly single leap second ever**. +/// +/// Therefore, assuming that `03:00:60` is a leap second: +/// +/// - `03:00:00 + 1s = 03:00:01`. +/// - `03:00:59 + 1s = 03:01:00`. +/// - `03:00:59 + 60s = 03:02:00`. +/// - `03:00:60 + 1s = 03:01:00`. +/// Note that the sum is identical to the previous; the associativity would no longer hold. +/// - `03:00:60 + 60s = 03:01:59`. +/// - `03:00:60 + 61s = 03:02:00`. +/// - `03:00:60.1 + 0.8s = 03:00:60.9`. +/// +/// It can be thought that the clock *forgets* a leap second once it ticked any direction. +/// +/// # Example +/// +/// ~~~~ +/// use chrono::{NaiveTime, Duration}; +/// +/// let hmsm = |h,m,s,milli| NaiveTime::from_hms_milli(h, m, s, milli); +/// assert_eq!(hmsm(3, 5, 7, 0) + Duration::zero(), hmsm(3, 5, 7, 0)); +/// assert_eq!(hmsm(3, 5, 7, 0) + Duration::seconds(1), hmsm(3, 5, 8, 0)); +/// assert_eq!(hmsm(3, 5, 7, 0) + Duration::seconds(-1), hmsm(3, 5, 6, 0)); +/// assert_eq!(hmsm(3, 5, 7, 0) + Duration::seconds(60 + 4), hmsm(3, 6, 11, 0)); +/// assert_eq!(hmsm(3, 5, 7, 0) + Duration::seconds(7*60*60 - 6*60), hmsm(9, 59, 7, 0)); +/// assert_eq!(hmsm(3, 5, 7, 0) + Duration::milliseconds(80), hmsm(3, 5, 7, 80)); +/// assert_eq!(hmsm(3, 5, 7, 950) + Duration::milliseconds(280), hmsm(3, 5, 8, 230)); +/// assert_eq!(hmsm(3, 5, 7, 950) + Duration::milliseconds(-980), hmsm(3, 5, 6, 970)); +/// ~~~~ +/// +/// The addition wraps around. +/// +/// ~~~~ +/// # use chrono::{NaiveTime, Duration}; +/// # let hmsm = |h,m,s,milli| NaiveTime::from_hms_milli(h, m, s, milli); +/// assert_eq!(hmsm(3, 5, 7, 0) + Duration::seconds(22*60*60), hmsm(1, 5, 7, 0)); +/// assert_eq!(hmsm(3, 5, 7, 0) + Duration::seconds(-8*60*60), hmsm(19, 5, 7, 0)); +/// assert_eq!(hmsm(3, 5, 7, 0) + Duration::days(800), hmsm(3, 5, 7, 0)); +/// ~~~~ +/// +/// Leap seconds are handled, but the addition assumes that it is the only leap second happened. +/// +/// ~~~~ +/// # use chrono::{NaiveTime, Duration}; +/// # let hmsm = |h,m,s,milli| NaiveTime::from_hms_milli(h, m, s, milli); +/// assert_eq!(hmsm(3, 5, 59, 1_300) + Duration::zero(), hmsm(3, 5, 59, 1_300)); +/// assert_eq!(hmsm(3, 5, 59, 1_300) + Duration::milliseconds(-500), hmsm(3, 5, 59, 800)); +/// assert_eq!(hmsm(3, 5, 59, 1_300) + Duration::milliseconds(500), hmsm(3, 5, 59, 1_800)); +/// assert_eq!(hmsm(3, 5, 59, 1_300) + Duration::milliseconds(800), hmsm(3, 6, 0, 100)); +/// assert_eq!(hmsm(3, 5, 59, 1_300) + Duration::seconds(10), hmsm(3, 6, 9, 300)); +/// assert_eq!(hmsm(3, 5, 59, 1_300) + Duration::seconds(-10), hmsm(3, 5, 50, 300)); +/// assert_eq!(hmsm(3, 5, 59, 1_300) + Duration::days(1), hmsm(3, 5, 59, 300)); +/// ~~~~ impl Add for NaiveTime { type Output = NaiveTime; - fn add(self, rhs: Duration) -> NaiveTime { - // there is no direct interface in `Duration` to get only the nanosecond part, - // so we need to do the additional calculation here. - let mut rhssecs = rhs.num_seconds(); - let mut rhs2 = rhs - Duration::seconds(rhssecs); - if rhs2 < Duration::zero() { // possible when rhs < 0 - rhssecs -= 1; - rhs2 = rhs2 + Duration::seconds(1); + fn add(self, mut rhs: Duration) -> NaiveTime { + let mut secs = self.secs; + let mut frac = self.frac; + + if frac >= 1_000_000_000 { + let rfrac = 2_000_000_000 - frac; + if rhs >= Duration::nanoseconds(rfrac as i64) { + rhs = rhs - Duration::nanoseconds(rfrac as i64); + secs += 1; + frac = 0; + } else if rhs < Duration::nanoseconds(-(frac as i64)) { + rhs = rhs + Duration::nanoseconds(frac as i64); + frac = 0; + } else { + frac = (frac as i64 + rhs.num_nanoseconds().unwrap()) as u32; + debug_assert!(frac < 2_000_000_000); + return NaiveTime { secs: secs, frac: frac }; + } } - 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; + debug_assert!(secs <= 86400); + debug_assert!(frac < 1_000_000_000); - // always ignore leap seconds after the current whole second - let maxnanos = if self.frac >= 1_000_000_000 {2_000_000_000} else {1_000_000_000}; + let rhssecs = rhs.num_seconds(); + let rhsfrac = (rhs - Duration::seconds(rhssecs)).num_nanoseconds().unwrap(); + debug_assert!(Duration::seconds(rhssecs) + Duration::nanoseconds(rhsfrac) == rhs); + let rhssecs = (rhssecs % 86400) as i32; + let rhsfrac = rhsfrac as i32; + debug_assert!(-86400 < rhssecs && rhssecs < 86400); + debug_assert!(-1_000_000_000 < rhsfrac && rhsfrac < 1_000_000_000); - if nanos >= maxnanos { - nanos -= maxnanos; + let mut secs = secs as i32 + rhssecs; + let mut frac = frac as i32 + rhsfrac; + debug_assert!(-86400 < secs && secs <= 2 * 86400 + 1); + debug_assert!(-1_000_000_000 < frac && frac < 2_000_000_000); + + if frac < 0 { + frac += 1_000_000_000; + secs -= 1; + } else if frac >= 1_000_000_000 { + frac -= 1_000_000_000; secs += 1; } - NaiveTime { secs: secs % 86400, frac: nanos } + debug_assert!(-86400 <= secs && secs <= 2 * 86400 + 1); + debug_assert!(0 <= frac && frac < 1_000_000_000); + + if secs < 0 { + secs += 86400 * 2; + } + if secs >= 86400 * 2 { + secs -= 86400 * 2; + } else if secs >= 86400 { + secs -= 86400; + } + debug_assert!(0 <= secs && secs < 86400); + + NaiveTime { secs: secs as u32, frac: frac as u32 } } } @@ -1031,6 +1149,7 @@ mod tests { check(hmsm(3, 5, 7, 900), Duration::zero(), hmsm(3, 5, 7, 900)); check(hmsm(3, 5, 7, 900), Duration::milliseconds(100), hmsm(3, 5, 8, 0)); check(hmsm(3, 5, 7, 1_300), Duration::milliseconds(800), hmsm(3, 5, 8, 100)); + check(hmsm(3, 5, 7, 1_300), Duration::milliseconds(1800), hmsm(3, 5, 9, 100)); 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::days(12345), hmsm(3, 5, 7, 900)); diff --git a/src/offset/mod.rs b/src/offset/mod.rs index 6e623b3..6037a1e 100644 --- a/src/offset/mod.rs +++ b/src/offset/mod.rs @@ -21,8 +21,10 @@ */ use std::fmt; +use std::ops::Add; use Weekday; +use Timelike; use duration::Duration; use naive::date::NaiveDate; use naive::time::NaiveTime; @@ -31,6 +33,18 @@ use date::Date; use datetime::DateTime; use format::{parse, Parsed, ParseResult, StrftimeItems}; +/// Same to `*lhs + *rhs`, but keeps the leap second information. +/// `rhs` should *not* have a fractional second. +// TODO this should be replaced by the addition with FixedOffset in 0.3! +pub fn add_with_leapsecond>(lhs: &T, rhs: &Duration) -> T { + debug_assert!(*rhs == Duration::seconds(rhs.num_seconds())); + + // extract and temporarily remove the fractional part and later recover it + let nanos = lhs.nanosecond(); + let lhs = lhs.with_nanosecond(0).unwrap(); + (lhs + *rhs).with_nanosecond(nanos).unwrap() +} + /// The conversion result from the local time to the timezone-aware datetime types. #[derive(Clone, PartialEq, Debug)] pub enum LocalResult { @@ -305,7 +319,8 @@ pub trait TimeZone: Sized + Clone { /// Converts the local `NaiveDateTime` to the timezone-aware `DateTime` if possible. fn from_local_datetime(&self, local: &NaiveDateTime) -> LocalResult> { self.offset_from_local_datetime(local).map(|offset| { - DateTime::from_utc(*local - offset.local_minus_utc(), offset) + let utc = add_with_leapsecond(local, &-offset.local_minus_utc()); + DateTime::from_utc(utc, offset) }) }