[gcc r14-8321] libstdc++: Fix std::format for floating-point chrono::time_point [PR113500]

[Jonathan Wakely <redi@gcc.gnu.org>]

https://gcc.gnu.org/g:7431fcea6b72beb54abb1932c254ac0e76bd0bde

commit r14-8321-g7431fcea6b72beb54abb1932c254ac0e76bd0bde
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Sun Jan 21 18:16:14 2024 +0000

    libstdc++: Fix std::format for floating-point chrono::time_point [PR113500]
    
    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.

Diff:
---
 libstdc++-v3/include/bits/chrono_io.h            | 71 +++++++++++++++++-------
 libstdc++-v3/testsuite/std/time/clock/file/io.cc | 17 ++++++
 libstdc++-v3/testsuite/std/time/clock/gps/io.cc  | 27 +++++++++
 libstdc++-v3/testsuite/std/time/clock/utc/io.cc  |  4 ++
 libstdc++-v3/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