Tested aarch64-linux. Pushed to trunk.

Backport to gcc-13 would be good too, I think.

-- >8 --

Currently trying to use std::format with certain specializations of
std::chrono::time_point is ill-formed, due to one member function of the
__formatter_chrono type which tries to write a time_point to an ostream.
For sys_time<floating-point> or sys_time with a period greater than days
there is no operator<< that can be used.

That operator<< is only needed when using an empty chrono-specs in the
format string, like "{}", but the ill-formed expression gives an error
even if not actually used. This means it's not possible to format some
other specializations of chrono::time_point, even when using a non-empty
chrono-specs.

This fixes it by avoiding using 'os << t' for all chrono::time_point
specializations, and instead using std::format("{:L%F %T}", t). So that
we continue to reject std::format("{}", sys_time{1.0s}) a check for
empty chrono-specs is added to the formatter<sys_time<D>, C>
specialization.

While testing this I noticed that the output for %S with a
floating-point duration was incorrect, as the subseconds part was being
appended to the seconds without a decimal point, and without the correct
number of leading zeros.

libstdc++-v3/ChangeLog:

        PR libstdc++/113500
        * include/bits/chrono_io.h (__formatter_chrono::_M_S): Fix
        printing of subseconds with floating-point rep.
        (__formatter_chrono::_M_format_to_ostream): Do not write
        time_point specializations directly to the ostream.
        (formatter<chrono::sys_time<D>, C>::parse): Do not allow an
        empty chrono-spec if the type fails to meet the constraints for
        writing to an ostream with operator<<.
        * testsuite/std/time/clock/file/io.cc: Check formatting
        non-integral times with empty chrono-specs.
        * testsuite/std/time/clock/gps/io.cc: Likewise.
        * testsuite/std/time/clock/utc/io.cc: Likewise.
        * testsuite/std/time/hh_mm_ss/io.cc: Likewise.
---
 libstdc++-v3/include/bits/chrono_io.h         | 71 +++++++++++++------
 .../testsuite/std/time/clock/file/io.cc       | 17 +++++
 .../testsuite/std/time/clock/gps/io.cc        | 27 +++++++
 .../testsuite/std/time/clock/utc/io.cc        |  4 ++
 .../testsuite/std/time/hh_mm_ss/io.cc         | 28 +++++++-
 5 files changed, 124 insertions(+), 23 deletions(-)

diff --git a/libstdc++-v3/include/bits/chrono_io.h 
b/libstdc++-v3/include/bits/chrono_io.h
index 7b5876b24e6..82f2d39ec44 100644
--- a/libstdc++-v3/include/bits/chrono_io.h
+++ b/libstdc++-v3/include/bits/chrono_io.h
@@ -681,6 +681,7 @@ namespace __format
            return __fc.locale();
        }
 
+      // Format for empty chrono-specs, e.g. "{}" (C++20 [time.format] p6).
       // TODO: consider moving body of every operator<< into this function
       // and use std::format("{}", t) to implement those operators. That
       // would avoid std::format("{}", t) calling operator<< which calls
@@ -702,6 +703,22 @@ namespace __format
 
              if constexpr (__is_specialization_of<_Tp, __utc_leap_second>)
                __os << __t._M_date << ' ' << __t._M_time;
+             else if constexpr (chrono::__is_time_point_v<_Tp>)
+               {
+                 // Need to be careful here because not all specializations
+                 // of chrono::sys_time can be written to an ostream.
+                 // For the specializations of time_point that can be
+                 // formatted with an empty chrono-specs, either it's a
+                 // sys_time with period greater or equal to days:
+                 if constexpr (is_convertible_v<_Tp, chrono::sys_days>)
+                   __os << _S_date(__t);
+                 else // Or it's formatted as "{:L%F %T}":
+                   {
+                     auto __days = chrono::floor<chrono::days>(__t);
+                     __os << chrono::year_month_day(__days) << ' '
+                        << chrono::hh_mm_ss(__t - __days);
+                   }
+               }
              else
                {
                  if constexpr (chrono::__is_duration_v<_Tp>)
@@ -1122,39 +1139,43 @@ namespace __format
                                   'S', 'O');
            }
 
-         __out = __format::__write(std::move(__out),
-                                   _S_two_digits(__hms.seconds().count()));
-         if constexpr (__hms.fractional_width != 0)
+         if constexpr (__hms.fractional_width == 0)
+           __out = __format::__write(std::move(__out),
+                                     _S_two_digits(__hms.seconds().count()));
+         else
            {
              locale __loc = _M_locale(__ctx);
+             auto __s = __hms.seconds();
              auto __ss = __hms.subseconds();
              using rep = typename decltype(__ss)::rep;
              if constexpr (is_floating_point_v<rep>)
                {
+                 chrono::duration<rep> __fs = __s + __ss;
                  __out = std::format_to(std::move(__out), __loc,
-                                        _GLIBCXX_WIDEN("{:.{}Lg}"),
-                                        __ss.count(),
-                                        __hms.fractional_width);
-               }
-             else if constexpr (is_integral_v<rep>)
-               {
-                 const auto& __np
-                   = use_facet<numpunct<_CharT>>(__loc);
-                 __out = std::format_to(std::move(__out),
-                                        _GLIBCXX_WIDEN("{}{:0{}}"),
-                                        __np.decimal_point(),
-                                        __ss.count(),
+                                        _GLIBCXX_WIDEN("{:#0{}.{}Lf}"),
+                                        __fs.count(),
+                                        3 + __hms.fractional_width,
                                         __hms.fractional_width);
                }
              else
                {
                  const auto& __np
                    = use_facet<numpunct<_CharT>>(__loc);
+                 __out = __format::__write(std::move(__out),
+                                           _S_two_digits(__s.count()));
                  *__out++ = __np.decimal_point();
-                 auto __str = std::format(_S_empty_spec, __ss.count());
-                 __out = std::format_to(_GLIBCXX_WIDEN("{:0>{}s}"),
-                                                       __str,
-                                                       __hms.fractional_width);
+                 if constexpr (is_integral_v<rep>)
+                   __out = std::format_to(std::move(__out),
+                                          _GLIBCXX_WIDEN("{:0{}}"),
+                                          __ss.count(),
+                                          __hms.fractional_width);
+                 else
+                   {
+                     auto __str = std::format(_S_empty_spec, __ss.count());
+                     __out = std::format_to(_GLIBCXX_WIDEN("{:0>{}s}"),
+                                            __str,
+                                            __hms.fractional_width);
+                   }
                }
            }
          return __out;
@@ -1911,7 +1932,13 @@ namespace __format
       template<typename _ParseContext>
        constexpr typename _ParseContext::iterator
        parse(_ParseContext& __pc)
-       { return _M_f._M_parse(__pc, __format::_ZonedDateTime); }
+       {
+         auto __next = _M_f._M_parse(__pc, __format::_ZonedDateTime);
+         if constexpr (!__stream_insertable)
+           if (_M_f._M_spec._M_chrono_specs.empty())
+             __format::__invalid_chrono_spec(); // chrono-specs can't be empty
+         return __next;
+       }
 
       template<typename _FormatContext>
        typename _FormatContext::iterator
@@ -1920,6 +1947,10 @@ namespace __format
        { return _M_f._M_format(__t, __fc); }
 
     private:
+      static constexpr bool __stream_insertable
+       = requires (basic_ostream<_CharT>& __os,
+                   chrono::sys_time<_Duration> __t) { __os << __t; };
+
       __format::__formatter_chrono<_CharT> _M_f;
     };
 
diff --git a/libstdc++-v3/testsuite/std/time/clock/file/io.cc 
b/libstdc++-v3/testsuite/std/time/clock/file/io.cc
index 39eb0dbcf51..3eee6bf5cc2 100644
--- a/libstdc++-v3/testsuite/std/time/clock/file/io.cc
+++ b/libstdc++-v3/testsuite/std/time/clock/file/io.cc
@@ -17,6 +17,23 @@ test_ostream()
   VERIFY( ss1.str() == ss2.str() );
 }
 
+void
+test_format()
+{
+  using namespace std::chrono;
+  auto t = file_clock::now();
+
+  auto s = std::format("{}", t);
+  std::ostringstream ss;
+  ss << t;
+  VERIFY( s == ss.str() );
+
+  // PR libstdc++/113500
+  auto ft = clock_cast<file_clock>(sys_days(2024y/January/21)) + 0ms + 2.5s;
+  s = std::format("{}", ft);
+  VERIFY( s == "2024-01-17 00:00:02.500");
+}
+
 void
 test_parse()
 {
diff --git a/libstdc++-v3/testsuite/std/time/clock/gps/io.cc 
b/libstdc++-v3/testsuite/std/time/clock/gps/io.cc
index 6f4544fcd97..5d15713c69d 100644
--- a/libstdc++-v3/testsuite/std/time/clock/gps/io.cc
+++ b/libstdc++-v3/testsuite/std/time/clock/gps/io.cc
@@ -3,6 +3,7 @@
 
 #include <chrono>
 #include <format>
+#include <sstream>
 #include <testsuite_hooks.h>
 
 void
@@ -16,6 +17,32 @@ test_ostream()
 
   auto s = format("{0:%F %T %Z} == {1:%F %T %Z}", st, gt);
   VERIFY( s == "2000-01-01 00:00:00 UTC == 2000-01-01 00:00:13 GPS" );
+
+  std::ostringstream ss;
+  ss << gt;
+  VERIFY( ss.str() == "2000-01-01 00:00:13" );
+
+  gps_time<duration<float>> gtf = gt;
+  ss.str("");
+  ss.clear();
+  ss << (gps_time<duration<long double>>(gt) + 20ms);
+  VERIFY( ss.str() == "2000-01-01 00:00:13.020" );
+}
+
+void
+test_format()
+{
+  using std::format;
+  using namespace std::chrono;
+
+  auto st = sys_days{2000y/January/1};
+  auto gt = clock_cast<gps_clock>(st);
+  auto s = std::format("{}", gt);
+  VERIFY( s == "2000-01-01 00:00:13" );
+
+  // PR libstdc++/113500
+  s = std::format("{}", gt + 150ms + 10.5s);
+  VERIFY( s == "2000-01-01 00:00:35.650" );
 }
 
 void
diff --git a/libstdc++-v3/testsuite/std/time/clock/utc/io.cc 
b/libstdc++-v3/testsuite/std/time/clock/utc/io.cc
index 58e358f3dbf..55c53dc4057 100644
--- a/libstdc++-v3/testsuite/std/time/clock/utc/io.cc
+++ b/libstdc++-v3/testsuite/std/time/clock/utc/io.cc
@@ -112,6 +112,10 @@ test_format()
 
   s = std::format("{:%T}", leap + 1s);
   VERIFY( s == "00:00:00" );
+
+  // PR libstdc++/113500
+  s = std::format("{}", leap + 100ms + 2.5s);
+  VERIFY( s == "2017-01-01 00:00:01.600");
 }
 
 void
diff --git a/libstdc++-v3/testsuite/std/time/hh_mm_ss/io.cc 
b/libstdc++-v3/testsuite/std/time/hh_mm_ss/io.cc
index 8eb80422e6e..d651dfbcdc8 100644
--- a/libstdc++-v3/testsuite/std/time/hh_mm_ss/io.cc
+++ b/libstdc++-v3/testsuite/std/time/hh_mm_ss/io.cc
@@ -35,9 +35,31 @@ test_ostream()
     VERIFY( out.str() == "18:15:45.123" );
   }
 
-  ostringstream out;
-  out << hh_mm_ss{65745s};
-  VERIFY( out.str() == "18:15:45" );
+  {
+    ostringstream out;
+    out << hh_mm_ss{65745s};
+    VERIFY( out.str() == "18:15:45" );
+  }
+
+  {
+    ostringstream out;
+    out << hh_mm_ss{0.020s};
+    // hh_mm_ss<duration<long double>>::fractional_width == 0 so no subseconds:
+    VERIFY( out.str() == "00:00:00" );
+  }
+
+  {
+    ostringstream out;
+    out << hh_mm_ss<std::chrono::duration<long double, std::nano>>{0.020s};
+    // hh_mm_ss<duration<long double, nano>>::fractional_width == 9:
+    VERIFY( out.str() == "00:00:00.020000000" );
+  }
+
+  {
+    ostringstream out;
+    out << hh_mm_ss{65745s + 20ms};
+    VERIFY( out.str() == "18:15:45.020" );
+  }
 }
 
 void
-- 
2.43.0

Reply via email to