[libc++][chrono] Fixes (sys|local)_time formatters. #76456

mordante · 2023-12-27T17:46:08Z

The sys_time formatter is constrained, which was not implemented.
There is a sys_days formatter which was not implemented.
The local_time formatter uses the sys_time formatter in its implementation so "inherited" the same issues.

Fixes: #73849
Fixes: #67983

llvmbot · 2023-12-29T17:10:52Z

@llvm/pr-subscribers-libcxx

Author: Mark de Wever (mordante)

Changes

The sys_time formatter is constrained, which was not implemented.
There is a sys_days formatter which was not implemented.
The local_time formatter uses the sys_time formatter in its implementation so "inherited" the same issues.

Fixes: #73849
Fixes: #67983

Patch is 29.21 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/76456.diff

7 Files Affected:

(modified) libcxx/include/__chrono/ostream.h (+8-1)
(modified) libcxx/include/chrono (+4)
(modified) libcxx/test/std/time/time.clock/time.clock.local/ostream.pass.cpp (+12-27)
(added) libcxx/test/std/time/time.clock/time.clock.local/ostream.verify.cpp (+83)
(added) libcxx/test/std/time/time.clock/time.clock.system/sys_date.ostream.pass.cpp (+175)
(renamed) libcxx/test/std/time/time.clock/time.clock.system/sys_time.ostream.pass.cpp (-30)
(added) libcxx/test/std/time/time.clock/time.clock.system/sys_time.ostream.verify.cpp (+74)

diff --git a/libcxx/include/__chrono/ostream.h b/libcxx/include/__chrono/ostream.h
index f171944b5cab3d..b687ef8059d5f5 100644
--- a/libcxx/include/__chrono/ostream.h
+++ b/libcxx/include/__chrono/ostream.h
@@ -42,11 +42,18 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 namespace chrono {
 
 template <class _CharT, class _Traits, class _Duration>
+  requires(!treat_as_floating_point_v<typename _Duration::rep> && _Duration{1} < days{1})
 _LIBCPP_HIDE_FROM_ABI basic_ostream<_CharT, _Traits>&
-operator<<(basic_ostream<_CharT, _Traits>& __os, const sys_time<_Duration> __tp) {
+operator<<(basic_ostream<_CharT, _Traits>& __os, const sys_time<_Duration>& __tp) {
   return __os << std::format(__os.getloc(), _LIBCPP_STATICALLY_WIDEN(_CharT, "{:L%F %T}"), __tp);
 }
 
+template <class _CharT, class _Traits>
+_LIBCPP_HIDE_FROM_ABI basic_ostream<_CharT, _Traits>&
+operator<<(basic_ostream<_CharT, _Traits>& __os, const sys_days& __dp) {
+  return __os << year_month_day{__dp};
+}
+
 template <class _CharT, class _Traits, class _Duration>
 _LIBCPP_HIDE_FROM_ABI basic_ostream<_CharT, _Traits>&
 operator<<(basic_ostream<_CharT, _Traits>& __os, const file_time<_Duration> __tp) {
diff --git a/libcxx/include/chrono b/libcxx/include/chrono
index b3ed9acc5e5deb..c80fa78a56ba1b 100644
--- a/libcxx/include/chrono
+++ b/libcxx/include/chrono
@@ -296,6 +296,10 @@ template<class charT, class traits, class Duration>     // C++20
   basic_ostream<charT, traits>&
     operator<<(basic_ostream<charT, traits>& os, const sys_time<Duration>& tp);
 
+template<class charT, class traits>                    // C++20
+  basic_ostream<charT, traits>&
+    operator<<(basic_ostream<charT, traits>& os, const sys_days& dp);
+
 class file_clock                                        // C++20
 {
 public:
diff --git a/libcxx/test/std/time/time.clock/time.clock.local/ostream.pass.cpp b/libcxx/test/std/time/time.clock/time.clock.local/ostream.pass.cpp
index 4f4fd3f40e23bc..9fdef8d5adc782 100644
--- a/libcxx/test/std/time/time.clock/time.clock.local/ostream.pass.cpp
+++ b/libcxx/test/std/time/time.clock/time.clock.local/ostream.pass.cpp
@@ -75,9 +75,11 @@ static void test_c() {
   assert(stream_c_locale<CharT>(std::chrono::local_time<std::chrono::minutes>{20'576'131min}) ==
          SV("2009-02-13 23:31:00"));
   assert(stream_c_locale<CharT>(std::chrono::local_time<std::chrono::hours>{342'935h}) == SV("2009-02-13 23:00:00"));
-  assert(stream_c_locale<CharT>(std::chrono::local_days{std::chrono::days{14'288}}) == SV("2009-02-13 00:00:00"));
+
+  // These switch to sys_day formatter, which omits the time.
+  assert(stream_c_locale<CharT>(std::chrono::local_days{std::chrono::days{14'288}}) == SV("2009-02-13"));
   assert(stream_c_locale<CharT>(std::chrono::local_time<std::chrono::weeks>{std::chrono::weeks{2041}}) ==
-         SV("2009-02-12 00:00:00"));
+         SV("2009-02-12"));
 
   assert(stream_c_locale<CharT>(std::chrono::local_time<std::chrono::duration<signed char, std::ratio<2, 1>>>{
              std::chrono::duration<signed char, std::ratio<2, 1>>{60}}) == SV("1970-01-01 00:02:00"));
@@ -89,13 +91,6 @@ static void test_c() {
              std::chrono::duration<long, std::ratio<1, 10>>{36611}}) == SV("1970-01-01 01:01:01.1"));
   assert(stream_c_locale<CharT>(std::chrono::local_time<std::chrono::duration<long long, std::ratio<1, 100>>>{
              std::chrono::duration<long long, std::ratio<1, 100>>{12'345'678'9010}}) == SV("2009-02-13 23:31:30.10"));
-
-  assert(stream_c_locale<CharT>(std::chrono::local_time<std::chrono::duration<float, std::ratio<1, 1>>>{
-             std::chrono::duration<float, std::ratio<1, 1>>{123.456}}) == SV("1970-01-01 00:02:03"));
-  assert(stream_c_locale<CharT>(std::chrono::local_time<std::chrono::duration<double, std::ratio<1, 10>>>{
-             std::chrono::duration<double, std::ratio<1, 10>>{123.456}}) == SV("1970-01-01 00:00:12.3"));
-  assert(stream_c_locale<CharT>(std::chrono::local_time<std::chrono::duration<long double, std::ratio<1, 100>>>{
-             std::chrono::duration<long double, std::ratio<1, 100>>{123.456}}) == SV("1970-01-01 00:00:01.23"));
 }
 
 template <class CharT>
@@ -114,9 +109,11 @@ static void test_fr_FR() {
          SV("2009-02-13 23:31:00"));
   assert(stream_fr_FR_locale<CharT>(std::chrono::local_time<std::chrono::hours>{342'935h}) ==
          SV("2009-02-13 23:00:00"));
-  assert(stream_fr_FR_locale<CharT>(std::chrono::local_days{std::chrono::days{14'288}}) == SV("2009-02-13 00:00:00"));
+
+  // These switch to sys_day formatter, which omits the time.
+  assert(stream_fr_FR_locale<CharT>(std::chrono::local_days{std::chrono::days{14'288}}) == SV("2009-02-13"));
   assert(stream_fr_FR_locale<CharT>(std::chrono::local_time<std::chrono::weeks>{std::chrono::weeks{2041}}) ==
-         SV("2009-02-12 00:00:00"));
+         SV("2009-02-12"));
 
   assert(stream_fr_FR_locale<CharT>(std::chrono::local_time<std::chrono::duration<signed char, std::ratio<2, 1>>>{
              std::chrono::duration<signed char, std::ratio<2, 1>>{60}}) == SV("1970-01-01 00:02:00"));
@@ -128,13 +125,6 @@ static void test_fr_FR() {
              std::chrono::duration<long, std::ratio<1, 10>>{36611}}) == SV("1970-01-01 01:01:01,1"));
   assert(stream_fr_FR_locale<CharT>(std::chrono::local_time<std::chrono::duration<long long, std::ratio<1, 100>>>{
              std::chrono::duration<long long, std::ratio<1, 100>>{12'345'678'9010}}) == SV("2009-02-13 23:31:30,10"));
-
-  assert(stream_fr_FR_locale<CharT>(std::chrono::local_time<std::chrono::duration<float, std::ratio<1, 1>>>{
-             std::chrono::duration<float, std::ratio<1, 1>>{123.456}}) == SV("1970-01-01 00:02:03"));
-  assert(stream_fr_FR_locale<CharT>(std::chrono::local_time<std::chrono::duration<double, std::ratio<1, 10>>>{
-             std::chrono::duration<double, std::ratio<1, 10>>{123.456}}) == SV("1970-01-01 00:00:12,3"));
-  assert(stream_fr_FR_locale<CharT>(std::chrono::local_time<std::chrono::duration<long double, std::ratio<1, 100>>>{
-             std::chrono::duration<long double, std::ratio<1, 100>>{123.456}}) == SV("1970-01-01 00:00:01,23"));
 }
 
 template <class CharT>
@@ -153,9 +143,11 @@ static void test_ja_JP() {
          SV("2009-02-13 23:31:00"));
   assert(stream_ja_JP_locale<CharT>(std::chrono::local_time<std::chrono::hours>{342'935h}) ==
          SV("2009-02-13 23:00:00"));
-  assert(stream_ja_JP_locale<CharT>(std::chrono::local_days{std::chrono::days{14'288}}) == SV("2009-02-13 00:00:00"));
+
+  // These switch to sys_day formatter, which omits the time.
+  assert(stream_ja_JP_locale<CharT>(std::chrono::local_days{std::chrono::days{14'288}}) == SV("2009-02-13"));
   assert(stream_ja_JP_locale<CharT>(std::chrono::local_time<std::chrono::weeks>{std::chrono::weeks{2041}}) ==
-         SV("2009-02-12 00:00:00"));
+         SV("2009-02-12"));
 
   assert(stream_ja_JP_locale<CharT>(std::chrono::local_time<std::chrono::duration<signed char, std::ratio<2, 1>>>{
              std::chrono::duration<signed char, std::ratio<2, 1>>{60}}) == SV("1970-01-01 00:02:00"));
@@ -167,13 +159,6 @@ static void test_ja_JP() {
              std::chrono::duration<long, std::ratio<1, 10>>{36611}}) == SV("1970-01-01 01:01:01.1"));
   assert(stream_ja_JP_locale<CharT>(std::chrono::local_time<std::chrono::duration<long long, std::ratio<1, 100>>>{
              std::chrono::duration<long long, std::ratio<1, 100>>{12'345'678'9010}}) == SV("2009-02-13 23:31:30.10"));
-
-  assert(stream_ja_JP_locale<CharT>(std::chrono::local_time<std::chrono::duration<float, std::ratio<1, 1>>>{
-             std::chrono::duration<float, std::ratio<1, 1>>{123.456}}) == SV("1970-01-01 00:02:03"));
-  assert(stream_ja_JP_locale<CharT>(std::chrono::local_time<std::chrono::duration<double, std::ratio<1, 10>>>{
-             std::chrono::duration<double, std::ratio<1, 10>>{123.456}}) == SV("1970-01-01 00:00:12.3"));
-  assert(stream_ja_JP_locale<CharT>(std::chrono::local_time<std::chrono::duration<long double, std::ratio<1, 100>>>{
-             std::chrono::duration<long double, std::ratio<1, 100>>{123.456}}) == SV("1970-01-01 00:00:01.23"));
 }
 
 template <class CharT>
diff --git a/libcxx/test/std/time/time.clock/time.clock.local/ostream.verify.cpp b/libcxx/test/std/time/time.clock/time.clock.local/ostream.verify.cpp
new file mode 100644
index 00000000000000..a3051c2f3ddedc
--- /dev/null
+++ b/libcxx/test/std/time/time.clock/time.clock.local/ostream.verify.cpp
@@ -0,0 +1,83 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+// UNSUPPORTED: no-localization
+// UNSUPPORTED: GCC-ALWAYS_INLINE-FIXME
+
+// TODO FMT This test should not require std::to_chars(floating-point)
+// XFAIL: availability-fp_to_chars-missing
+
+// <chrono>
+
+// class system_clock;
+
+// template<class charT, class traits, class Duration>
+//   basic_ostream<charT, traits>&
+//     operator<<(basic_ostream<charT, traits>& os, const local_time<Duration>& tp);
+
+// The function uses the system_clock which has two overloads
+
+// template<class charT, class traits, class Duration>
+//   basic_ostream<charT, traits>&
+//     operator<<(basic_ostream<charT, traits>& os, const sys_time<Duration>& tp);
+// Constraints: treat_as_floating_point_v<typename Duration::rep> is false, and Duration{1} < days{1} is true.
+
+// template<class charT, class traits>
+//   basic_ostream<charT, traits>&
+//     operator<<(basic_ostream<charT, traits>& os, const sys_days& dp);
+
+#include <chrono>
+#include <ratio>
+#include <sstream>
+#include <type_traits>
+
+void test() {
+  std::stringstream sstr;
+
+  // floating-point values
+
+  sstr << // expected-error@*:* {{invalid operands to binary expression}}
+      std::chrono::local_time<std::chrono::duration<float, std::ratio<1, 1>>>{
+          std::chrono::duration<float, std::ratio<1, 1>>{0}};
+
+  sstr << // expected-error@*:* {{invalid operands to binary expression}}
+      std::chrono::local_time<std::chrono::duration<double, std::ratio<1, 1>>>{
+          std::chrono::duration<double, std::ratio<1, 1>>{0}};
+
+  sstr << // expected-error@*:* {{invalid operands to binary expression}}
+      std::chrono::local_time<std::chrono::duration<long double, std::ratio<1, 1>>>{
+          std::chrono::duration<long double, std::ratio<1, 1>>{0}};
+
+  // duration >= day
+
+  sstr << // valid since day has its own formatter
+      std::chrono::local_days{std::chrono::days{0}};
+
+  using rep = std::conditional_t<std::is_same_v<std::chrono::days::rep, int>, long, int>;
+  sstr << // a different rep does not matter,
+      std::chrono::local_time<std::chrono::duration<rep, std::ratio<86400>>>{
+          std::chrono::duration<rep, std::ratio<86400>>{0}};
+
+  sstr << // expected-error@*:* {{invalid operands to binary expression}}
+      std::chrono::local_time<std::chrono::duration<typename std::chrono::days::rep, std::ratio<86401>>>{
+          std::chrono::duration<typename std::chrono::days::rep, std::ratio<86401>>{0}};
+
+  sstr << // These are considered days.
+      std::chrono::local_time<std::chrono::weeks>{std::chrono::weeks{3}};
+
+  sstr << // These too.
+      std::chrono::local_time<std::chrono::duration<rep, std::ratio<20 * 86400>>>{
+          std::chrono::duration<rep, std::ratio<20 * 86400>>{0}};
+
+  sstr << // expected-error@*:* {{invalid operands to binary expression}}
+      std::chrono::local_time<std::chrono::months>{std::chrono::months{0}};
+
+  sstr << // expected-error@*:* {{invalid operands to binary expression}}
+      std::chrono::local_time<std::chrono::years>{std::chrono::years{0}};
+}
diff --git a/libcxx/test/std/time/time.clock/time.clock.system/sys_date.ostream.pass.cpp b/libcxx/test/std/time/time.clock/time.clock.system/sys_date.ostream.pass.cpp
new file mode 100644
index 00000000000000..7af3ebf7768072
--- /dev/null
+++ b/libcxx/test/std/time/time.clock/time.clock.system/sys_date.ostream.pass.cpp
@@ -0,0 +1,175 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+// UNSUPPORTED: no-localization
+// UNSUPPORTED: GCC-ALWAYS_INLINE-FIXME
+
+// TODO FMT This test should not require std::to_chars(floating-point)
+// XFAIL: availability-fp_to_chars-missing
+
+// TODO FMT Investigate Windows issues.
+// XFAIL: msvc
+
+// REQUIRES: locale.fr_FR.UTF-8
+// REQUIRES: locale.ja_JP.UTF-8
+
+// <chrono>
+
+// class system_Clock;
+
+// template<class charT, class traits>
+//   basic_ostream<charT, traits>&
+//     operator<<(basic_ostream<charT, traits>& os, const sys_days& dp);
+
+#include <cassert>
+#include <chrono>
+#include <ratio>
+#include <sstream>
+
+#include "make_string.h"
+#include "platform_support.h" // locale name macros
+#include "test_macros.h"
+#include "assert_macros.h"
+#include "concat_macros.h"
+
+#define SV(S) MAKE_STRING_VIEW(CharT, S)
+
+#define TEST_EQUAL(OUT, EXPECTED)                                                                                      \
+  TEST_REQUIRE(OUT == EXPECTED,                                                                                        \
+               TEST_WRITE_CONCATENATED(                                                                                \
+                   "\nExpression      ", #OUT, "\nExpected output ", EXPECTED, "\nActual output   ", OUT, '\n'));
+
+template <class CharT>
+static std::basic_string<CharT> stream_c_locale(const std::chrono::sys_days& dp) {
+  std::basic_stringstream<CharT> sstr;
+  sstr << dp;
+  return sstr.str();
+}
+
+template <class CharT>
+static std::basic_string<CharT> stream_fr_FR_locale(const std::chrono::sys_days& dp) {
+  std::basic_stringstream<CharT> sstr;
+  const std::locale locale(LOCALE_fr_FR_UTF_8);
+  sstr.imbue(locale);
+  sstr << dp;
+  return sstr.str();
+}
+
+template <class CharT>
+static std::basic_string<CharT> stream_ja_JP_locale(const std::chrono::sys_days& dp) {
+  std::basic_stringstream<CharT> sstr;
+  const std::locale locale(LOCALE_ja_JP_UTF_8);
+  sstr.imbue(locale);
+  sstr << dp;
+  return sstr.str();
+}
+
+template <class CharT>
+static void test() {
+  TEST_EQUAL(stream_c_locale<CharT>(std::chrono::sys_days{
+                 std::chrono::year_month_day{std::chrono::year{-32'768}, std::chrono::month{1}, std::chrono::day{1}}}),
+             SV("-32768-01-01 is not a valid date"));
+  TEST_EQUAL(stream_c_locale<CharT>(std::chrono::sys_days{
+                 std::chrono::year_month_day{std::chrono::year{1970}, std::chrono::month{1}, std::chrono::day{1}}}),
+             SV("1970-01-01"));
+  TEST_EQUAL(stream_c_locale<CharT>(std::chrono::sys_days{
+                 std::chrono::year_month_day{std::chrono::year{2000}, std::chrono::month{2}, std::chrono::day{29}}}),
+             SV("2000-02-29"));
+
+#if defined(_AIX)
+  TEST_EQUAL(stream_c_locale<CharT>(std::chrono::sys_days{
+                 std::chrono::year_month_day{std::chrono::year{32'767}, std::chrono::month{12}, std::chrono::day{31}}}),
+             SV("+32767-12-31"));
+#elif defined(_WIN32) // defined(_AIX)
+  TEST_EQUAL(stream_c_locale<CharT>(std::chrono::sys_days{
+                 std::chrono::year_month_day{std::chrono::year{32'767}, std::chrono::month{12}, std::chrono::day{31}}}),
+             SV(""));
+#else                 //  defined(_AIX)
+  TEST_EQUAL(stream_c_locale<CharT>(std::chrono::sys_days{
+                 std::chrono::year_month_day{std::chrono::year{32'767}, std::chrono::month{12}, std::chrono::day{31}}}),
+             SV("32767-12-31"));
+#endif                // defined(_AIX)
+
+  // multiples of days are considered days.
+  TEST_EQUAL(stream_c_locale<CharT>(std::chrono::sys_time<std::chrono::weeks>{std::chrono::weeks{3}}),
+             SV("1970-01-22"));
+  TEST_EQUAL(stream_c_locale<CharT>(std::chrono::sys_time<std::chrono::duration<int, std::ratio<30 * 86400>>>{
+                 std::chrono::duration<int, std::ratio<30 * 86400>>{1}}),
+             SV("1970-01-31"));
+
+  TEST_EQUAL(stream_fr_FR_locale<CharT>(std::chrono::sys_days{
+                 std::chrono::year_month_day{std::chrono::year{-32'768}, std::chrono::month{1}, std::chrono::day{1}}}),
+             SV("-32768-01-01 is not a valid date"));
+  TEST_EQUAL(stream_fr_FR_locale<CharT>(std::chrono::sys_days{
+                 std::chrono::year_month_day{std::chrono::year{1970}, std::chrono::month{1}, std::chrono::day{1}}}),
+             SV("1970-01-01"));
+  TEST_EQUAL(stream_fr_FR_locale<CharT>(std::chrono::sys_days{
+                 std::chrono::year_month_day{std::chrono::year{2000}, std::chrono::month{2}, std::chrono::day{29}}}),
+             SV("2000-02-29"));
+#if defined(_AIX)
+  TEST_EQUAL(stream_fr_FR_locale<CharT>(std::chrono::sys_days{
+                 std::chrono::year_month_day{std::chrono::year{32'767}, std::chrono::month{12}, std::chrono::day{31}}}),
+             SV("+32767-12-31"));
+#elif defined(_WIN32) // defined(_AIX)
+  TEST_EQUAL(stream_fr_FR_locale<CharT>(std::chrono::sys_days{
+                 std::chrono::year_month_day{std::chrono::year{32'767}, std::chrono::month{12}, std::chrono::day{31}}}),
+             SV(""));
+#else                 // defined(_AIX)
+  TEST_EQUAL(stream_fr_FR_locale<CharT>(std::chrono::sys_days{
+                 std::chrono::year_month_day{std::chrono::year{32'767}, std::chrono::month{12}, std::chrono::day{31}}}),
+             SV("32767-12-31"));
+#endif                // defined(_AIX)
+
+  // multiples of days are considered days.
+  TEST_EQUAL(stream_fr_FR_locale<CharT>(std::chrono::sys_time<std::chrono::weeks>{std::chrono::weeks{3}}),
+             SV("1970-01-22"));
+  TEST_EQUAL(stream_fr_FR_locale<CharT>(std::chrono::sys_time<std::chrono::duration<int, std::ratio<30 * 86400>>>{
+                 std::chrono::duration<int, std::ratio<30 * 86400>>{1}}),
+             SV("1970-01-31"));
+
+  TEST_EQUAL(stream_ja_JP_locale<CharT>(std::chrono::sys_days{
+                 std::chrono::year_month_day{std::chrono::year{-32'768}, std::chrono::month{1}, std::chrono::day{1}}}),
+             SV("-32768-01-01 is not a valid date"));
+  TEST_EQUAL(stream_ja_JP_locale<CharT>(std::chrono::sys_days{
+                 std::chrono::year_month_day{std::chrono::year{1970}, std::chrono::month{1}, std::chrono::day{1}}}),
+             SV("1970-01-01"));
+  TEST_EQUAL(stream_ja_JP_locale<CharT>(std::chrono::sys_days{
+                 std::chrono::year_month_day{std::chrono::year{2000}, std::chrono::month{2}, std::chrono::day{29}}}),
+             SV("2000-02-29"));
+#if defined(_AIX)
+  TEST_EQUAL(stream_ja_JP_locale<CharT>(std::chrono::sys_days{
+                 std::chrono::year_month_day{std::chrono::year{32'767}, std::chrono::month{12}, std::chrono::day{31}}}),
+             SV("+32767-12-31"));
+#elif defined(_WIN32) // defined(_AIX)
+  TEST_EQUAL(stream_ja_JP_locale<CharT>(std::chrono::sys_days{
+                 std::chrono::year_month_day{std::chrono::year{32'767}, std::chrono::month{12}, std::chrono::day{31}}}),
+             SV(""));
+#else                 // defined(_AIX)
+  TEST_EQUAL(stream_ja_JP_locale<CharT>(std::chrono::sys_days{
+                 std::chrono::year_month_day{std::chrono::year{32'767}, std::chrono::month{12}, std::chrono::day{31}}}),
+             SV("32767-12-31"));
+#endif                // defined(_AIX)
+
+  // multiples of days are considered days.
+  TEST_EQUAL(stream_ja_JP_locale<CharT>(std::chrono::sys_time<std::chrono::weeks>{std::chrono::weeks{3}}),
+             SV("1970-01-22"));
+  TEST_EQUAL(stream_ja_JP_locale<CharT>(std::chrono::sys_time<std::chrono::duration<int, std::ratio<30 * 86400>>>{
+                 std::chrono::duration<int, std::ratio<30 * 86400>>{1}}),
+             SV("1970-01-31"));
+}
+
+int main(int, char**) {
+  test<char>();
+
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+  test<wchar_t>();
+#endif
+
+  return 0;
+}
diff --git a/libcxx/test/s...
[truncated]

ldionne

LGTM w/ comments!

ldionne · 2024-01-16T17:33:59Z

libcxx/test/std/time/time.clock/time.clock.local/ostream.verify.cpp

+#include <sstream>
+#include <type_traits>
+
+void test() {


We need to test that operator<< has the appropriate constraints, which means that it needs to be SFINAE friendly. In other words, you want to test this via something like

template <class T> concept is_ostreamable = requires (std::stringstream& sstr, T const& val) { { sstr << val; } }; static_assert(!is_ostreamable<decltype(your-expression)>);

And at that point, this can be moved to ostream.pass.cpp or to a separate ostream.compile.pass.cpp (but really I think I'd leave it in the existing .pass.cpp as nothing more than an additional test case).

Actually that won't work. The operator<< is not constrained. For sys_time the operator<< is constrained. But this operator is defined as

Effects: os << sys_time<Duration>{lt.time_since_epoch()};

Note the wording is Effects: not Effects: Equivalent to, the latter would constrain the function, the former doesn't. Both MSVC STL and libstdc++ agree; local_time can not be SFINAE tested, system_time can. I'll leave a comment in the code instead.

At the LWG reflector Jonathan brought up a discussion about this issue and I've added this information. For now I want to see how that discussion pans out and whether an LWG issue should be filed.

ldionne · 2024-01-16T17:36:23Z

libcxx/test/std/time/time.clock/time.clock.system/sys_date.ostream.pass.cpp

+  TEST_EQUAL(stream_c_locale<CharT>(std::chrono::sys_days{
+                 std::chrono::year_month_day{std::chrono::year{32'767}, std::chrono::month{12}, std::chrono::day{31}}}),
+             SV("+32767-12-31"));
+#elif defined(_WIN32) // defined(_AIX)


Nitpick, but I find these defined (_AIX) comments kinda confusing since only the first block pertains to _AIX.

I'm not disagreeing, but this seems the style quite often used in libc++.

ldionne · 2024-01-16T17:37:50Z

libcxx/test/std/time/time.clock/time.clock.system/sys_time.ostream.verify.cpp

+
+// class system_clock;
+
+// template<class charT, class traits, class Duration>


Same comment here for using SFINAE. I see that for a few expressions, you ensure that they work instead of not working. For those, we should also test them positively in static_assert(is_ostreamable<expression>).

I'll SFINAE these, this function is indeed constrained.

- The sys_time formatter is constrained, which was not implemented. - There is a sys_days formatter which was not implemented. - The local_time formatter uses the sys_time formatter in its implementation so "inherited" the same issues. Fixes: llvm#73849 Fixes: llvm#67983

kiloalphaindia · 2024-05-27T13:09:17Z

Can we merge this into release/17.x?

ldionne · 2024-05-27T14:39:43Z

17.x is long done, do you mean 18.x?

kiloalphaindia · 2024-05-28T05:59:48Z

AFAIK It already is in 18.x.
I suppose there are plenty of installations still running on 17. Are you already considering 17 EOL?

mordante · 2024-05-28T16:02:20Z

There will be no new LLVM 17 releases anymore. The LLVM project only maintains 1 release branch.

mordante force-pushed the fixes_ostream_sys_time branch from 2bda353 to f15db9f Compare December 28, 2023 16:21

mordante marked this pull request as ready for review December 29, 2023 17:10

mordante requested a review from a team as a code owner December 29, 2023 17:10

llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Dec 29, 2023

ldionne self-assigned this Jan 16, 2024

ldionne approved these changes Jan 16, 2024

View reviewed changes

mordante force-pushed the fixes_ostream_sys_time branch from f15db9f to 58f229a Compare January 21, 2024 20:41

ldionne approved these changes Jan 22, 2024

View reviewed changes

mordante merged commit 042a6a1 into llvm:main Jan 22, 2024
43 checks passed

mordante deleted the fixes_ostream_sys_time branch January 22, 2024 18:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[libc++][chrono] Fixes (sys|local)_time formatters. #76456

[libc++][chrono] Fixes (sys|local)_time formatters. #76456

mordante commented Dec 27, 2023

llvmbot commented Dec 29, 2023

ldionne left a comment

ldionne Jan 16, 2024

mordante Jan 21, 2024

ldionne Jan 16, 2024

mordante Jan 21, 2024

ldionne Jan 16, 2024

mordante Jan 21, 2024

kiloalphaindia commented May 27, 2024

ldionne commented May 27, 2024

kiloalphaindia commented May 28, 2024

mordante commented May 28, 2024


		// class system_clock;

		// template<class charT, class traits, class Duration>

[libc++][chrono] Fixes (sys|local)_time formatters. #76456

[libc++][chrono] Fixes (sys|local)_time formatters. #76456

Conversation

mordante commented Dec 27, 2023

llvmbot commented Dec 29, 2023

ldionne left a comment

Choose a reason for hiding this comment

ldionne Jan 16, 2024

Choose a reason for hiding this comment

mordante Jan 21, 2024

Choose a reason for hiding this comment

ldionne Jan 16, 2024

Choose a reason for hiding this comment

mordante Jan 21, 2024

Choose a reason for hiding this comment

ldionne Jan 16, 2024

Choose a reason for hiding this comment

mordante Jan 21, 2024

Choose a reason for hiding this comment

kiloalphaindia commented May 27, 2024

ldionne commented May 27, 2024

kiloalphaindia commented May 28, 2024

mordante commented May 28, 2024