[libcxx-commits] [PATCH] D138826: [libc++][chrono] Fixes formatter duration.

[Mark de Wever via Phabricator via libcxx-commits]

Mordante updated this revision to Diff 478645.
Mordante added a comment.

Addresses review comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138826/new/

https://reviews.llvm.org/D138826

Files:
  libcxx/include/__chrono/convert_to_tm.h
  libcxx/test/std/time/time.syn/formatter.duration.pass.cpp


Index: libcxx/test/std/time/time.syn/formatter.duration.pass.cpp
===================================================================
--- libcxx/test/std/time/time.syn/formatter.duration.pass.cpp
+++ libcxx/test/std/time/time.syn/formatter.duration.pass.cpp
@@ -550,7 +550,7 @@
            "\n"),
         lfmt,
         std::chrono::duration<int, std::ratio<86400>>(7));
-#else // defined(__APPLE__) || defined(_AIX)
+#else  // defined(__APPLE__) || defined(_AIX)
   check(loc,
         SV("%H='00'\t"
            "%OH='〇'\t"
@@ -1076,6 +1076,22 @@
   check_exception("End of input while parsing the modifier E", SV("{:%E"), 0ms);
   check_exception("End of input while parsing the modifier O", SV("{:%O"), 0ms);
 
+  // Make sure the the required values work, based on their minimum number of required bits
+  check(SV("23:47:16.854775807"),
+        SV("{:%T}"),
+        std::chrono::nanoseconds{0x7fff'ffff'ffff'ffffll}); // 64 bit signed value max
+  check(SV("23:35:09.481983"),
+        SV("{:%T}"),
+        std::chrono::microseconds{0x003f'ffff'ffff'ffffll});                                 // 55 bit signed value max
+  check(SV("06:20:44.415"), SV("{:%T}"), std::chrono::milliseconds{0x0000'fff'ffff'ffffll}); // 45 bit signed value max
+  check(SV("01:53:03"), SV("{:%T}"), std::chrono::seconds{0x0000'0003'ffff'ffffll});         // 35 bit signed value max
+  check(SV("12:15:00"), SV("{:%T}"), std::chrono::minutes{0x0fff'ffff});                     // 29 bit signed value max
+  check(SV("15:00:00"), SV("{:%T}"), std::chrono::hours{0x003f'ffff});                       // 23 bit signed value max
+  check(SV("00:00:00"), SV("{:%T}"), std::chrono::days{0x0ff'ffff});                         // 25 bit signed value max
+  check(SV("00:00:00"), SV("{:%T}"), std::chrono::weeks{0x003f'ffff});                       // 22 bit signed value max
+  check(SV("21:11:42"), SV("{:%T}"), std::chrono::months{0x0007'ffff});                      // 20 bit signed value max
+  check(SV("05:42:00"), SV("{:%T}"), std::chrono::years{0xffff});                            // 17 bit signed value max
+
   // Precision not allowed
   check_exception("Expected '%' or '}' in the chrono format-string", SV("{:.3}"), 0ms);
 }
Index: libcxx/include/__chrono/convert_to_tm.h
===================================================================
--- libcxx/include/__chrono/convert_to_tm.h
+++ libcxx/include/__chrono/convert_to_tm.h
@@ -17,6 +17,7 @@
 #include <__chrono/year.h>
 #include <__concepts/same_as.h>
 #include <__config>
+#include <__type_traits/is_convertible.h>
 #include <cstdint>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
@@ -41,12 +42,23 @@
     //   ...  However, if a flag refers to a "time of day" (e.g. %H, %I, %p,
     //   etc.), then a specialization of duration is interpreted as the time of
     //   day elapsed since midnight.
-    uint64_t __sec = chrono::duration_cast<chrono::seconds>(__value).count();
-    __sec %= 24 * 3600;
-    __result.tm_hour = __sec / 3600;
-    __sec %= 3600;
-    __result.tm_min = __sec / 60;
-    __result.tm_sec = __sec % 60;
+
+    // Not all values can be converted to hours, it may run into ratio
+    // conversion errors. In that case the conversion to seconds works.
+    if constexpr (is_convertible_v<_ChronoT, chrono::hours>) {
+      auto __hour      = chrono::floor<chrono::hours>(__value);
+      auto __sec       = chrono::duration_cast<chrono::seconds>(__value - __hour);
+      __result.tm_hour = __hour.count() % 24;
+      __result.tm_min  = __sec.count() / 60;
+      __result.tm_sec  = __sec.count() % 60;
+    } else {
+      uint64_t __sec = chrono::duration_cast<chrono::seconds>(__value).count();
+      __sec %= 24 * 3600;
+      __result.tm_hour = __sec / 3600;
+      __sec %= 3600;
+      __result.tm_min = __sec / 60;
+      __result.tm_sec = __sec % 60;
+    }
   } else if constexpr (same_as<_ChronoT, chrono::day>)
     __result.tm_mday = static_cast<unsigned>(__value);
   else if constexpr (same_as<_ChronoT, chrono::month>)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D138826.478645.patch
Type: text/x-patch
Size: 4020 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20221129/d5375776/attachment-0001.bin>