-
Notifications
You must be signed in to change notification settings - Fork 14.3k
release/20.x: [libc++][TZDB] Fixes mapping of nonexisting time. (#127330) #127531
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.
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
Conversation
@ldionne What do you think about merging this PR to the release branch? |
@llvm/pr-subscribers-libcxx Author: None (llvmbot) ChangesBackport 941f7cb Requested by: @mordante Full diff: https://github.com/llvm/llvm-project/pull/127531.diff 2 Files Affected:
diff --git a/libcxx/include/__chrono/time_zone.h b/libcxx/include/__chrono/time_zone.h
index ab5c22eceaaf1..d18d59d2736bf 100644
--- a/libcxx/include/__chrono/time_zone.h
+++ b/libcxx/include/__chrono/time_zone.h
@@ -103,10 +103,14 @@ class _LIBCPP_AVAILABILITY_TZDB time_zone {
to_sys(const local_time<_Duration>& __time, choose __z) const {
local_info __info = get_info(__time);
switch (__info.result) {
- case local_info::unique:
- case local_info::nonexistent: // first and second are the same
+ case local_info::unique: // first and second are the same
return sys_time<common_type_t<_Duration, seconds>>{__time.time_since_epoch() - __info.first.offset};
+ case local_info::nonexistent:
+ // first and second are the same
+ // All non-existing values are converted to the same time.
+ return sys_time<common_type_t<_Duration, seconds>>{__info.first.end};
+
case local_info::ambiguous:
switch (__z) {
case choose::earliest:
diff --git a/libcxx/test/std/time/time.zone/time.zone.timezone/time.zone.members/to_sys_choose.pass.cpp b/libcxx/test/std/time/time.zone/time.zone.timezone/time.zone.members/to_sys_choose.pass.cpp
index bad4ef352e9b9..1147c9fadf9ae 100644
--- a/libcxx/test/std/time/time.zone/time.zone.timezone/time.zone.members/to_sys_choose.pass.cpp
+++ b/libcxx/test/std/time/time.zone/time.zone.timezone/time.zone.members/to_sys_choose.pass.cpp
@@ -88,7 +88,7 @@ static void test_nonexistent() {
// Pick an historic date where it's well known what the time zone rules were.
// This makes it unlikely updates to the database change these rules.
std::chrono::local_time<std::chrono::seconds> time{
- (std::chrono::sys_days{std::chrono::March / 30 / 1986} + 2h + 30min).time_since_epoch()};
+ (std::chrono::sys_days{std::chrono::March / 30 / 1986} + 2h).time_since_epoch()};
std::chrono::sys_seconds expected{time.time_since_epoch() - 1h};
@@ -100,6 +100,13 @@ static void test_nonexistent() {
assert(tz->to_sys(time + 0us, std::chrono::choose::latest) == expected);
assert(tz->to_sys(time + 0ms, std::chrono::choose::earliest) == expected);
assert(tz->to_sys(time + 0s, std::chrono::choose::latest) == expected);
+
+ // The entire nonexisting hour should map to the same time.
+ // For nonexistant the value of std::chrono::choose has no effect.
+ assert(tz->to_sys(time + 1s, std::chrono::choose::earliest) == expected);
+ assert(tz->to_sys(time + 1min, std::chrono::choose::latest) == expected);
+ assert(tz->to_sys(time + 30min, std::chrono::choose::earliest) == expected);
+ assert(tz->to_sys(time + 59min + 59s, std::chrono::choose::latest) == expected);
}
// Tests ambiguous conversions.
@@ -120,7 +127,7 @@ static void test_ambiguous() {
// Pick an historic date where it's well known what the time zone rules were.
// This makes it unlikely updates to the database change these rules.
std::chrono::local_time<std::chrono::seconds> time{
- (std::chrono::sys_days{std::chrono::September / 28 / 1986} + 2h + 30min).time_since_epoch()};
+ (std::chrono::sys_days{std::chrono::September / 28 / 1986} + 2h).time_since_epoch()};
std::chrono::sys_seconds earlier{time.time_since_epoch() - 2h};
std::chrono::sys_seconds later{time.time_since_epoch() - 1h};
@@ -133,6 +140,12 @@ static void test_ambiguous() {
assert(tz->to_sys(time + 0us, std::chrono::choose::latest) == later);
assert(tz->to_sys(time + 0ms, std::chrono::choose::earliest) == earlier);
assert(tz->to_sys(time + 0s, std::chrono::choose::latest) == later);
+
+ // Test times in the ambigious hour
+ assert(tz->to_sys(time + 1s, std::chrono::choose::earliest) == earlier + 1s);
+ assert(tz->to_sys(time + 1min, std::chrono::choose::latest) == later + 1min);
+ assert(tz->to_sys(time + 30min, std::chrono::choose::earliest) == earlier + 30min);
+ assert(tz->to_sys(time + 59min + 59s, std::chrono::choose::latest) == later + 59min + 59s);
}
// This test does the basic validations of this function. The library function
|
All non-existing local times in a contiguous range should map to the same time point. This fixes a bug, were the times inside the range were mapped to the wrong time. Fixes: llvm#113654 (cherry picked from commit 941f7cb)
@mordante (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR. |
Backport 941f7cb
Requested by: @mordante