Skip to content

Conversation

mordante
Copy link
Member

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: #113654

@mordante mordante requested a review from a team as a code owner February 15, 2025 15:26
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Feb 15, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 15, 2025

@llvm/pr-subscribers-libcxx

Author: Mark de Wever (mordante)

Changes

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: #113654


Full diff: https://github.com/llvm/llvm-project/pull/127330.diff

2 Files Affected:

  • (modified) libcxx/include/__chrono/time_zone.h (+6-2)
  • (modified) libcxx/test/std/time/time.zone/time.zone.timezone/time.zone.members/to_sys_choose.pass.cpp (+15-2)
diff --git a/libcxx/include/__chrono/time_zone.h b/libcxx/include/__chrono/time_zone.h
index ab5c22eceaaf1..e0c4b4e68bda7 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
@mordante mordante force-pushed the review/fixes_mapping_of_nonexisting_time branch from 58b2e13 to ec81d6f Compare February 15, 2025 15:29
@mordante mordante added this to the LLVM 20.X Release milestone Feb 17, 2025
@mordante mordante merged commit 941f7cb into llvm:main Feb 17, 2025
76 checks passed
@mordante mordante deleted the review/fixes_mapping_of_nonexisting_time branch February 17, 2025 18:08
@mordante
Copy link
Member Author

/cherry-pick 941f7cb

@llvmbot
Copy link
Member

llvmbot commented Feb 17, 2025

/pull-request #127531

swift-ci pushed a commit to swiftlang/llvm-project that referenced this pull request Feb 20, 2025
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
Development

Successfully merging this pull request may close these issues.

chrono::time_zone::to_sys() on 'nonexistent time' deviates
3 participants