Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Windows: base implementation on GetTimeZoneInformationForYear #1017

Merged
merged 4 commits into from
Feb 10, 2024
Merged

Windows: base implementation on GetTimeZoneInformationForYear #1017

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7e9a472
Windows: rewrite using `GetTimeZoneInformationForYear`
pitdicker Apr 17, 2023
7fdd17f
Remove obsolete test
pitdicker Apr 17, 2023
38edc49
Extend test to more distant dates
pitdicker Apr 17, 2023
3d7daa5
Add test for issue 651
pitdicker Apr 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
47 changes: 12 additions & 35 deletions src/datetime/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1264,6 +1264,18 @@ fn test_datetime_from_timestamp_millis() {
);
}

#[test]
#[cfg(feature = "clock")]
fn test_datetime_before_windows_api_limits() {
// dt corresponds to `FILETIME = 147221225472` from issue 651.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you refer to the issue by making it a link?

// (https://github.com/chronotope/chrono/issues/651)
// This used to fail on Windows for timezones with an offset of -5:00 or greater.
// The API limits years to 1601..=30827.
let dt = NaiveDate::from_ymd_opt(1601, 1, 1).unwrap().and_hms_milli_opt(4, 5, 22, 122).unwrap();
let local_dt = Local.from_utc_datetime(&dt);
dbg!(local_dt);
}

pitdicker marked this conversation as resolved.
Show resolved Hide resolved
#[test]
#[cfg(feature = "clock")]
fn test_years_elapsed() {
Expand Down Expand Up @@ -1499,41 +1511,6 @@ fn test_core_duration_max() {
utc_dt += Duration::MAX;
}

#[test]
#[cfg(all(target_os = "windows", feature = "clock"))]
fn test_from_naive_date_time_windows() {
let min_year = NaiveDate::from_ymd_opt(1601, 1, 3).unwrap().and_hms_opt(0, 0, 0).unwrap();

let max_year = NaiveDate::from_ymd_opt(30827, 12, 29).unwrap().and_hms_opt(23, 59, 59).unwrap();

let too_low_year =
NaiveDate::from_ymd_opt(1600, 12, 29).unwrap().and_hms_opt(23, 59, 59).unwrap();

let too_high_year = NaiveDate::from_ymd_opt(30829, 1, 3).unwrap().and_hms_opt(0, 0, 0).unwrap();

let _ = Local.from_utc_datetime(&min_year);
let _ = Local.from_utc_datetime(&max_year);

let _ = Local.from_local_datetime(&min_year);
let _ = Local.from_local_datetime(&max_year);

let local_too_low = Local.from_local_datetime(&too_low_year);
let local_too_high = Local.from_local_datetime(&too_high_year);

assert_eq!(local_too_low, LocalResult::None);
assert_eq!(local_too_high, LocalResult::None);

let err = std::panic::catch_unwind(|| {
Local.from_utc_datetime(&too_low_year);
});
assert!(err.is_err());

let err = std::panic::catch_unwind(|| {
Local.from_utc_datetime(&too_high_year);
});
assert!(err.is_err());
}

#[test]
#[cfg(feature = "clock")]
fn test_datetime_local_from_preserves_offset() {
Expand Down
17 changes: 17 additions & 0 deletions src/naive/datetime/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -790,6 +790,23 @@ impl NaiveDateTime {
NaiveDateTime { date, time }
}

/// Subtracts given `FixedOffset` from the current datetime.
/// The resulting value may be outside the valid range of [`NaiveDateTime`].
///
/// This can be useful for intermediate values, but the resulting out-of-range `NaiveDate`
/// should not be exposed to library users.
#[must_use]
#[allow(unused)] // currently only used in `Local` but not on all platforms
pub(crate) fn overflowing_sub_offset(self, rhs: FixedOffset) -> NaiveDateTime {
let (time, days) = self.time.overflowing_sub_offset(rhs);
let date = match days {
-1 => self.date.pred_opt().unwrap_or(NaiveDate::BEFORE_MIN),
1 => self.date.succ_opt().unwrap_or(NaiveDate::AFTER_MAX),
_ => self.date,
};
NaiveDateTime { date, time }
}

/// Subtracts given `TimeDelta` from the current date and time.
///
/// As a part of Chrono's [leap second handling](./struct.NaiveTime.html#leap-second-handling),
Expand Down
264 changes: 261 additions & 3 deletions src/offset/local/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@

//! The local (system) time zone.

#[cfg(windows)]
use std::cmp::Ordering;

#[cfg(any(feature = "rkyv", feature = "rkyv-16", feature = "rkyv-32", feature = "rkyv-64"))]
use rkyv::{Archive, Deserialize, Serialize};

Expand Down Expand Up @@ -183,11 +186,96 @@ impl TimeZone for Local {
}
}

#[cfg(windows)]
#[derive(Copy, Clone, Eq, PartialEq)]
struct Transition {
djc marked this conversation as resolved.
Show resolved Hide resolved
transition_utc: NaiveDateTime,
offset_before: FixedOffset,
offset_after: FixedOffset,
}

#[cfg(windows)]
impl Transition {
fn new(
transition_local: NaiveDateTime,
offset_before: FixedOffset,
offset_after: FixedOffset,
) -> Transition {
// It is no problem if the transition time in UTC falls a couple of hours inside the buffer
// space around the `NaiveDateTime` range (although it is very theoretical to have a
// transition at midnight around `NaiveDate::(MIN|MAX)`.
let transition_utc = transition_local.overflowing_sub_offset(offset_before);
Transition { transition_utc, offset_before, offset_after }
}
}

#[cfg(windows)]
impl PartialOrd for Transition {
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
Some(self.transition_utc.cmp(&other.transition_utc))
}
}

#[cfg(windows)]
impl Ord for Transition {
fn cmp(&self, other: &Self) -> Ordering {
self.transition_utc.cmp(&other.transition_utc)
}
}

// Calculate the time in UTC given a local time and transitions.
// `transitions` must be sorted.
#[cfg(windows)]
fn lookup_with_dst_transitions(
transitions: &[Transition],
dt: NaiveDateTime,
) -> LocalResult<FixedOffset> {
for t in transitions.iter() {
// A transition can result in the wall clock time going forward (creating a gap) or going
// backward (creating a fold). We are interested in the earliest and latest wall time of the
// transition, as this are the times between which `dt` does may not exist or is ambiguous.
//
// It is no problem if the transition times falls a couple of hours inside the buffer
// space around the `NaiveDateTime` range (although it is very theoretical to have a
// transition at midnight around `NaiveDate::(MIN|MAX)`.
let (offset_min, offset_max) =
match t.offset_after.local_minus_utc() > t.offset_before.local_minus_utc() {
true => (t.offset_before, t.offset_after),
false => (t.offset_after, t.offset_before),
};
let wall_earliest = t.transition_utc.overflowing_add_offset(offset_min);
let wall_latest = t.transition_utc.overflowing_add_offset(offset_max);

if dt < wall_earliest {
return LocalResult::Single(t.offset_before);
} else if dt <= wall_latest {
return match t.offset_after.local_minus_utc().cmp(&t.offset_before.local_minus_utc()) {
Ordering::Equal => LocalResult::Single(t.offset_before),
Ordering::Less => LocalResult::Ambiguous(t.offset_before, t.offset_after),
Ordering::Greater => {
if dt == wall_earliest {
LocalResult::Single(t.offset_before)
} else if dt == wall_latest {
LocalResult::Single(t.offset_after)
} else {
LocalResult::None
}
}
};
}
}
LocalResult::Single(transitions.last().unwrap().offset_after)
}

#[cfg(test)]
mod tests {
use super::Local;
#[cfg(windows)]
use crate::offset::local::{lookup_with_dst_transitions, Transition};
use crate::offset::TimeZone;
use crate::{Datelike, TimeDelta, Utc};
#[cfg(windows)]
use crate::{FixedOffset, LocalResult, NaiveDate, NaiveDateTime};

#[test]
fn verify_correct_offsets() {
Expand All @@ -204,8 +292,7 @@ mod tests {

#[test]
fn verify_correct_offsets_distant_past() {
// let distant_past = Local::now() - TimeDelta::days(365 * 100);
let distant_past = Local::now() - TimeDelta::days(250 * 31);
let distant_past = Local::now() - TimeDelta::days(365 * 500);
let from_local = Local.from_local_datetime(&distant_past.naive_local()).unwrap();
let from_utc = Local.from_utc_datetime(&distant_past.naive_utc());

Expand All @@ -218,7 +305,7 @@ mod tests {

#[test]
fn verify_correct_offsets_distant_future() {
let distant_future = Local::now() + TimeDelta::days(250 * 31);
let distant_future = Local::now() + TimeDelta::days(365 * 35000);
let from_local = Local.from_local_datetime(&distant_future.naive_local()).unwrap();
let from_utc = Local.from_utc_datetime(&distant_future.naive_utc());

Expand Down Expand Up @@ -264,6 +351,177 @@ mod tests {
}
}

#[test]
#[cfg(windows)]
fn test_lookup_with_dst_transitions() {
let ymdhms = |y, m, d, h, n, s| {
NaiveDate::from_ymd_opt(y, m, d).unwrap().and_hms_opt(h, n, s).unwrap()
};

#[track_caller]
#[allow(clippy::too_many_arguments)]
fn compare_lookup(
transitions: &[Transition],
y: i32,
m: u32,
d: u32,
h: u32,
n: u32,
s: u32,
result: LocalResult<FixedOffset>,
) {
let dt = NaiveDate::from_ymd_opt(y, m, d).unwrap().and_hms_opt(h, n, s).unwrap();
assert_eq!(lookup_with_dst_transitions(transitions, dt), result);
}

// dst transition before std transition
// dst offset > std offset
let std = FixedOffset::east_opt(3 * 60 * 60).unwrap();
let dst = FixedOffset::east_opt(4 * 60 * 60).unwrap();
let transitions = [
Transition::new(ymdhms(2023, 3, 26, 2, 0, 0), std, dst),
Transition::new(ymdhms(2023, 10, 29, 3, 0, 0), dst, std),
];
compare_lookup(&transitions, 2023, 3, 26, 1, 0, 0, LocalResult::Single(std));
compare_lookup(&transitions, 2023, 3, 26, 2, 0, 0, LocalResult::Single(std));
compare_lookup(&transitions, 2023, 3, 26, 2, 30, 0, LocalResult::None);
compare_lookup(&transitions, 2023, 3, 26, 3, 0, 0, LocalResult::Single(dst));
compare_lookup(&transitions, 2023, 3, 26, 4, 0, 0, LocalResult::Single(dst));

compare_lookup(&transitions, 2023, 10, 29, 1, 0, 0, LocalResult::Single(dst));
compare_lookup(&transitions, 2023, 10, 29, 2, 0, 0, LocalResult::Ambiguous(dst, std));
compare_lookup(&transitions, 2023, 10, 29, 2, 30, 0, LocalResult::Ambiguous(dst, std));
compare_lookup(&transitions, 2023, 10, 29, 3, 0, 0, LocalResult::Ambiguous(dst, std));
compare_lookup(&transitions, 2023, 10, 29, 4, 0, 0, LocalResult::Single(std));

// std transition before dst transition
// dst offset > std offset
let std = FixedOffset::east_opt(-5 * 60 * 60).unwrap();
let dst = FixedOffset::east_opt(-4 * 60 * 60).unwrap();
let transitions = [
Transition::new(ymdhms(2023, 3, 24, 3, 0, 0), dst, std),
Transition::new(ymdhms(2023, 10, 27, 2, 0, 0), std, dst),
];
compare_lookup(&transitions, 2023, 3, 24, 1, 0, 0, LocalResult::Single(dst));
compare_lookup(&transitions, 2023, 3, 24, 2, 0, 0, LocalResult::Ambiguous(dst, std));
compare_lookup(&transitions, 2023, 3, 24, 2, 30, 0, LocalResult::Ambiguous(dst, std));
compare_lookup(&transitions, 2023, 3, 24, 3, 0, 0, LocalResult::Ambiguous(dst, std));
compare_lookup(&transitions, 2023, 3, 24, 4, 0, 0, LocalResult::Single(std));

compare_lookup(&transitions, 2023, 10, 27, 1, 0, 0, LocalResult::Single(std));
compare_lookup(&transitions, 2023, 10, 27, 2, 0, 0, LocalResult::Single(std));
compare_lookup(&transitions, 2023, 10, 27, 2, 30, 0, LocalResult::None);
compare_lookup(&transitions, 2023, 10, 27, 3, 0, 0, LocalResult::Single(dst));
compare_lookup(&transitions, 2023, 10, 27, 4, 0, 0, LocalResult::Single(dst));

// dst transition before std transition
// dst offset < std offset
let std = FixedOffset::east_opt(3 * 60 * 60).unwrap();
let dst = FixedOffset::east_opt((2 * 60 + 30) * 60).unwrap();
let transitions = [
Transition::new(ymdhms(2023, 3, 26, 2, 30, 0), std, dst),
Transition::new(ymdhms(2023, 10, 29, 2, 0, 0), dst, std),
];
compare_lookup(&transitions, 2023, 3, 26, 1, 0, 0, LocalResult::Single(std));
compare_lookup(&transitions, 2023, 3, 26, 2, 0, 0, LocalResult::Ambiguous(std, dst));
compare_lookup(&transitions, 2023, 3, 26, 2, 15, 0, LocalResult::Ambiguous(std, dst));
compare_lookup(&transitions, 2023, 3, 26, 2, 30, 0, LocalResult::Ambiguous(std, dst));
compare_lookup(&transitions, 2023, 3, 26, 3, 0, 0, LocalResult::Single(dst));

compare_lookup(&transitions, 2023, 10, 29, 1, 0, 0, LocalResult::Single(dst));
compare_lookup(&transitions, 2023, 10, 29, 2, 0, 0, LocalResult::Single(dst));
compare_lookup(&transitions, 2023, 10, 29, 2, 15, 0, LocalResult::None);
compare_lookup(&transitions, 2023, 10, 29, 2, 30, 0, LocalResult::Single(std));
compare_lookup(&transitions, 2023, 10, 29, 3, 0, 0, LocalResult::Single(std));

// std transition before dst transition
// dst offset < std offset
let std = FixedOffset::east_opt(-(4 * 60 + 30) * 60).unwrap();
let dst = FixedOffset::east_opt(-5 * 60 * 60).unwrap();
let transitions = [
Transition::new(ymdhms(2023, 3, 24, 2, 0, 0), dst, std),
Transition::new(ymdhms(2023, 10, 27, 2, 30, 0), std, dst),
];
compare_lookup(&transitions, 2023, 3, 24, 1, 0, 0, LocalResult::Single(dst));
compare_lookup(&transitions, 2023, 3, 24, 2, 0, 0, LocalResult::Single(dst));
compare_lookup(&transitions, 2023, 3, 24, 2, 15, 0, LocalResult::None);
compare_lookup(&transitions, 2023, 3, 24, 2, 30, 0, LocalResult::Single(std));
compare_lookup(&transitions, 2023, 3, 24, 3, 0, 0, LocalResult::Single(std));

compare_lookup(&transitions, 2023, 10, 27, 1, 0, 0, LocalResult::Single(std));
compare_lookup(&transitions, 2023, 10, 27, 2, 0, 0, LocalResult::Ambiguous(std, dst));
compare_lookup(&transitions, 2023, 10, 27, 2, 15, 0, LocalResult::Ambiguous(std, dst));
compare_lookup(&transitions, 2023, 10, 27, 2, 30, 0, LocalResult::Ambiguous(std, dst));
compare_lookup(&transitions, 2023, 10, 27, 3, 0, 0, LocalResult::Single(dst));

// offset stays the same
let std = FixedOffset::east_opt(3 * 60 * 60).unwrap();
let transitions = [
Transition::new(ymdhms(2023, 3, 26, 2, 0, 0), std, std),
Transition::new(ymdhms(2023, 10, 29, 3, 0, 0), std, std),
];
compare_lookup(&transitions, 2023, 3, 26, 2, 0, 0, LocalResult::Single(std));
compare_lookup(&transitions, 2023, 10, 29, 3, 0, 0, LocalResult::Single(std));

// single transition
let std = FixedOffset::east_opt(3 * 60 * 60).unwrap();
let dst = FixedOffset::east_opt(4 * 60 * 60).unwrap();
let transitions = [Transition::new(ymdhms(2023, 3, 26, 2, 0, 0), std, dst)];
compare_lookup(&transitions, 2023, 3, 26, 1, 0, 0, LocalResult::Single(std));
compare_lookup(&transitions, 2023, 3, 26, 2, 0, 0, LocalResult::Single(std));
compare_lookup(&transitions, 2023, 3, 26, 2, 30, 0, LocalResult::None);
compare_lookup(&transitions, 2023, 3, 26, 3, 0, 0, LocalResult::Single(dst));
compare_lookup(&transitions, 2023, 3, 26, 4, 0, 0, LocalResult::Single(dst));
}

#[test]
#[cfg(windows)]
fn test_lookup_with_dst_transitions_limits() {
// Transition beyond UTC year end doesn't panic in year of `NaiveDate::MAX`
let std = FixedOffset::east_opt(3 * 60 * 60).unwrap();
let dst = FixedOffset::east_opt(4 * 60 * 60).unwrap();
let transitions = [
Transition::new(NaiveDateTime::MAX.with_month(7).unwrap(), std, dst),
Transition::new(NaiveDateTime::MAX, dst, std),
];
assert_eq!(
lookup_with_dst_transitions(&transitions, NaiveDateTime::MAX.with_month(3).unwrap()),
LocalResult::Single(std)
);
assert_eq!(
lookup_with_dst_transitions(&transitions, NaiveDateTime::MAX.with_month(8).unwrap()),
LocalResult::Single(dst)
);
// Doesn't panic with `NaiveDateTime::MAX` as argument (which would be out of range when
// converted to UTC).
assert_eq!(
lookup_with_dst_transitions(&transitions, NaiveDateTime::MAX),
LocalResult::Ambiguous(dst, std)
);

// Transition before UTC year end doesn't panic in year of `NaiveDate::MIN`
let std = FixedOffset::west_opt(3 * 60 * 60).unwrap();
let dst = FixedOffset::west_opt(4 * 60 * 60).unwrap();
let transitions = [
Transition::new(NaiveDateTime::MIN, std, dst),
Transition::new(NaiveDateTime::MIN.with_month(6).unwrap(), dst, std),
];
assert_eq!(
lookup_with_dst_transitions(&transitions, NaiveDateTime::MIN.with_month(3).unwrap()),
LocalResult::Single(dst)
);
assert_eq!(
lookup_with_dst_transitions(&transitions, NaiveDateTime::MIN.with_month(8).unwrap()),
LocalResult::Single(std)
);
// Doesn't panic with `NaiveDateTime::MIN` as argument (which would be out of range when
// converted to UTC).
assert_eq!(
lookup_with_dst_transitions(&transitions, NaiveDateTime::MIN),
LocalResult::Ambiguous(std, dst)
);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worthwhile adding edge cases to this test function test_lookup_with_dst_transitions? i.e. should this also test transitions at or near DateTime::MIN and DateTime::MAX (or whatever the minimum and maximum should on Windows)? At first glance, that looks important to me.


#[test]
#[cfg(feature = "rkyv-validation")]
fn test_rkyv_validation() {
Expand Down