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.

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

[libc++][TZDB] Fixes reverse time lookups. #89498

Conversation

mordante
Copy link
Member

Testing with the get_info() returning a local_info revealed some issues in the reverse lookup. This needed an additional quirck. Also the skipping when not in the current continuation optimization was wrong. It prevented merging two sys_info objects.

Testing with the get_info() returning a local_info revealed some issues in
the reverse lookup. This needed an additional quirck. Also the skipping
when not in the current continuation optimization was wrong. It prevented
merging two sys_info objects.
@mordante mordante requested a review from a team as a code owner April 20, 2024 10:41
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Apr 20, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 20, 2024

@llvm/pr-subscribers-libcxx

Author: Mark de Wever (mordante)

Changes

Testing with the get_info() returning a local_info revealed some issues in the reverse lookup. This needed an additional quirck. Also the skipping when not in the current continuation optimization was wrong. It prevented merging two sys_info objects.


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

2 Files Affected:

  • (modified) libcxx/src/time_zone.cpp (+15-4)
  • (modified) libcxx/test/std/time/time.zone/time.zone.timezone/time.zone.members/get_info.sys_time.pass.cpp (+76)
diff --git a/libcxx/src/time_zone.cpp b/libcxx/src/time_zone.cpp
index aef6ac674a11e6..928f3d2855e456 100644
--- a/libcxx/src/time_zone.cpp
+++ b/libcxx/src/time_zone.cpp
@@ -567,11 +567,22 @@ __first_rule(seconds __stdoff, const vector<__tz::__rule>& __rules) {
         false};
   }
 
-  __named_rule_until __continuation_end{__continuation};
-  if (__time >= __continuation_end.__until() && !__continuation_end.__needs_adjustment())
-    // note std::unexpected<sys_seconds>(__end); is ambiguous with std::unexpected() in <exception>,
-    return __sys_info_result{std::unexpect, __continuation_end.__until()};
+  if (__rule->__save.__time != 0s) {
+    // another fix for America/Punta_Arenas when not at the start of the
+    // sys_info object.
+    seconds __save = __rule->__save.__time;
+    if (__continuation_begin >= __rule_begin - __save && __time < __next.first) {
+      return __sys_info{
+          sys_info{__continuation_begin,
+                   __next.first,
+                   __continuation.__stdoff + __save,
+                   chrono::duration_cast<minutes>(__save),
+                   chrono::__format(__continuation, __rule->__letters, __save)},
+          false};
+    }
+  }
 
+  __named_rule_until __continuation_end{__continuation};
   while (__next.second != __rules.end()) {
 #ifdef PRINT
     std::print(
diff --git a/libcxx/test/std/time/time.zone/time.zone.timezone/time.zone.members/get_info.sys_time.pass.cpp b/libcxx/test/std/time/time.zone/time.zone.timezone/time.zone.members/get_info.sys_time.pass.cpp
index 25d2ff11d09341..1a1705d5ae59a5 100644
--- a/libcxx/test/std/time/time.zone/time.zone.timezone/time.zone.members/get_info.sys_time.pass.cpp
+++ b/libcxx/test/std/time/time.zone/time.zone.timezone/time.zone.members/get_info.sys_time.pass.cpp
@@ -1299,6 +1299,78 @@ static void test_america_indiana_knox() {
                tz->get_info(to_sys_seconds(2006y, std::chrono::October, 29d, 6h, 59min, 59s)));
 }
 
+static void test_america_punta_arenas() {
+  // Z America/Punta_Arenas -4:43:40 - LMT 1890
+  // ...
+  // -4 - -04 1919 Jul
+  // -4:42:45 - SMT 1927 S
+  // -5 x -05/-04 1932 S
+  // ...
+  //
+  // R x 1927 1931 - S 1 0 1 -
+  // R x 1928 1932 - Ap 1 0 0 -
+  // ...
+
+  using namespace std::literals::chrono_literals;
+  const std::chrono::time_zone* tz = std::chrono::locate_zone("America/Punta_Arenas");
+
+  assert_equal(
+      std::chrono::sys_info(
+          to_sys_seconds(1927y, std::chrono::September, 1d, 4h, 42min, 45s),
+          to_sys_seconds(1928y, std::chrono::April, 1d, 4h),
+          -4h,
+          60min,
+          "-04"),
+      tz->get_info(to_sys_seconds(1927y, std::chrono::September, 1d, 4h, 42min, 45s)));
+
+  assert_equal(
+      std::chrono::sys_info(
+          to_sys_seconds(1927y, std::chrono::September, 1d, 4h, 42min, 45s),
+          to_sys_seconds(1928y, std::chrono::April, 1d, 4h),
+          -4h,
+          60min,
+          "-04"),
+      tz->get_info(to_sys_seconds(1928y, std::chrono::April, 1d, 3h, 59min, 59s)));
+}
+
+static void test_europ_ljubljana() {
+  // Z Europe/Ljubljana 0:58:4 - LMT 1884
+  // 1 - CET 1941 Ap 18 23
+  // 1 c CE%sT 1945 May 8 2s
+  // 1 1 CEST 1945 S 16 2s
+  // 1 - CET 1982 N 27
+  // 1 E CE%sT
+  //
+  // ...
+  // R c 1943 o - O 4 2s 0 -
+  // R c 1944 1945 - Ap M>=1 2s 1 S
+  // R c 1944 o - O 2 2s 0 -
+  // R c 1945 o - S 16 2s 0 -
+  // R c 1977 1980 - Ap Su>=1 2s 1 S
+  // ...
+
+  using namespace std::literals::chrono_literals;
+  const std::chrono::time_zone* tz = std::chrono::locate_zone("Europe/Ljubljana");
+
+  assert_equal(
+      std::chrono::sys_info(
+          to_sys_seconds(1945y, std::chrono::April, 2d, 1h),
+          to_sys_seconds(1945y, std::chrono::September, 16d, 1h),
+          2h,
+          60min,
+          "CEST"),
+      tz->get_info(to_sys_seconds(1945y, std::chrono::April, 2d, 1h)));
+
+  assert_equal(
+      std::chrono::sys_info(
+          to_sys_seconds(1945y, std::chrono::April, 2d, 1h),
+          to_sys_seconds(1945y, std::chrono::September, 16d, 1h),
+          2h,
+          60min,
+          "CEST"),
+      tz->get_info(to_sys_seconds(1945y, std::chrono::September, 16d, 0h, 59min, 59s)));
+}
+
 int main(int, const char**) {
   // Basic tests
   test_gmt();
@@ -1333,5 +1405,9 @@ int main(int, const char**) {
   test_america_ciudad_juarez();
   test_america_indiana_knox();
 
+  // Reverse search bugs
+  test_america_punta_arenas();
+  test_europ_ljubljana();
+
   return 0;
 }

@mordante
Copy link
Member Author

This was not properly stacked, abandoned.

@mordante mordante closed this Apr 20, 2024
@mordante mordante deleted the users/mordante/fixes_reverse_time_lookups branch April 20, 2024 13:19
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
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants