[PATCH 1/2] libstdc++: Fix std::format output for std::chrono::zoned_time

[PATCH 1/2] libstdc++: Fix std::format output for std::chrono::zoned_time

[Jonathan Wakely]

My first attempt to fix this was an overly complex kluge, but there was
a nice simple solution staring me in the face. I'm pretty happy with
this now.

Tested x86_64-linux.

-- >8 --

When formatting a chrono::zoned_time with an empty chrono-specs, we were
only formatting its _M_time member, but the ostream insertion operator
uses the format "{:L%F %T %Z}" which includes the time zone
abbreviation. The %Z should also be used when formatting with an empty
chrono-specs.

This commit makes _M_format_to_ostream handle __local_time_fmt
specializations directly, rather than calling itself recursively to
format the _M_time member. We need to be able to customize the output of
_M_format_to_ostream for __local_time_fmt, because we use that type for
gps_time and tai_time as well as for zoned_time and __local_time_fmt.
When formatting gps_time and tai_time we don't want to include the time
zone abbreviation in the "{}" output, but for zoned_time we do want to.
We can reuse the __is_neg flag passed to _M_format_to_ostream (via
_M_format) to say that we want the time zone abbreviation.  Currently
the __is_neg flag is only used for duration specializations, so it's
available for __local_time_fmt to use.

In addition to fixing the zoned_time output to use %Z, this commit also
changes the __local_time_fmt output to use %Z. Previously it didn't use
it, just like zoned_time.  The standard doesn't actually say how to
format local-time-format-t for an empty chrono-specs, but this behaviour
seems sensible and is what I'm proposing as part of LWG 4124.

While testing this I noticed that some chrono types were not being
tested with empty chrono-specs, so this adds more tests. I also noticed
that std/time/clock/local/io.cc was testing tai_time instead of
local_time, which was completely wrong. That's fixed now too.

libstdc++-v3/ChangeLog:

	* include/bits/chrono_io.h (__local_fmt_t): Remove unused
	declaration.
	(__formatter_chrono::_M_format_to_ostream): Add explicit
	handling for specializations of __local_time_fmt, including the
	time zone abbreviation in the output if __is_neg is true.
	(formatter<chrono::tai_time<D>>::format): Add comment.
	(formatter<chrono::gps_time<D>>::format): Likewise.
	(formatter<chrono::__detail::__local_time_fmt::format): Call
	_M_format with true for the __is_neg flag.
	* testsuite/std/time/clock/gps/io.cc: Remove unused variable.
	* testsuite/std/time/clock/local/io.cc: Fix test error that
	checked tai_time instead of local_time. Add tests for
	local-time-format-t formatting.
	* testsuite/std/time/clock/system/io.cc: Check empty
	chrono-specs.
	* testsuite/std/time/clock/tai/io.cc: Likewise.
	* testsuite/std/time/zoned_time/io.cc: Likewise.
---
 libstdc++-v3/include/bits/chrono_io.h         | 46 ++++++++---
 .../testsuite/std/time/clock/gps/io.cc        |  1 -
 .../testsuite/std/time/clock/local/io.cc      | 78 ++++++++++++++++++-
 .../testsuite/std/time/clock/system/io.cc     |  8 +-
 .../testsuite/std/time/clock/tai/io.cc        |  3 +
 .../testsuite/std/time/zoned_time/io.cc       | 11 +++
 6 files changed, 130 insertions(+), 17 deletions(-)

diff --git a/libstdc++-v3/include/bits/chrono_io.h b/libstdc++-v3/include/bits/chrono_io.h
index 72c66a0fef0..e7e7deb2cde 100644
--- a/libstdc++-v3/include/bits/chrono_io.h
+++ b/libstdc++-v3/include/bits/chrono_io.h
@@ -156,6 +156,7 @@ namespace __detail
 namespace __detail
 {
   // An unspecified type returned by `chrono::local_time_format`.
+  // This is called `local-time-format-t` in the standard.
   template<typename _Duration>
     struct __local_time_fmt
     {
@@ -163,8 +164,6 @@ namespace __detail
       const string* _M_abbrev;
       const seconds* _M_offset_sec;
     };
-
-  struct __local_fmt_t;
 }
 /// @endcond
 
@@ -695,13 +694,34 @@ namespace __format
 	  using ::std::chrono::__detail::__utc_leap_second;
 	  using ::std::chrono::__detail::__local_time_fmt;
 
+	  basic_ostringstream<_CharT> __os;
+	  __os.imbue(_M_locale(__fc));
+
 	  if constexpr (__is_specialization_of<_Tp, __local_time_fmt>)
-	    return _M_format_to_ostream(__t._M_time, __fc, false);
+	    {
+	      // Format as "{:L%F %T}"
+	      auto __days = chrono::floor<chrono::days>(__t._M_time);
+	      __os << chrono::year_month_day(__days) << ' '
+		   << chrono::hh_mm_ss(__t._M_time - __days);
+
+	      // For __local_time_fmt the __is_neg flags says whether to
+	      // append " %Z" to the result.
+	      if (__is_neg)
+		{
+		  if (!__t._M_abbrev) [[unlikely]]
+		    __format::__no_timezone_available();
+		  else if constexpr (is_same_v<_CharT, char>)
+		    __os << ' ' << *__t._M_abbrev;
+		  else
+		    {
+		      __os << L' ';
+		      for (char __c : *__t._M_abbrev)
+			__os << __c;
+		    }
+		}
+	    }
 	  else
 	    {
-	      basic_ostringstream<_CharT> __os;
-	      __os.imbue(_M_locale(__fc));
-
 	      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>)
@@ -727,11 +747,11 @@ namespace __format
 		      __os << _S_plus_minus[1];
 		  __os << __t;
 		}
-
-	      auto __str = std::move(__os).str();
-	      return __format::__write_padded_as_spec(__str, __str.size(),
-						      __fc, _M_spec);
 	    }
+
+	  auto __str = std::move(__os).str();
+	  return __format::__write_padded_as_spec(__str, __str.size(),
+						  __fc, _M_spec);
 	}
 
       static constexpr const _CharT* _S_chars
@@ -2008,6 +2028,8 @@ namespace __format
 	       _FormatContext& __fc) const
 	{
 	  // Convert to __local_time_fmt with abbrev "TAI" and offset 0s.
+	  // We use __local_time_fmt and not sys_time (as the standard implies)
+	  // because %Z for sys_time would print "UTC" and we want "TAI" here.
 
 	  // Offset is 1970y/January/1 - 1958y/January/1
 	  constexpr chrono::days __tai_offset = chrono::days(4383);
@@ -2038,6 +2060,8 @@ namespace __format
 	       _FormatContext& __fc) const
 	{
 	  // Convert to __local_time_fmt with abbrev "GPS" and offset 0s.
+	  // We use __local_time_fmt and not sys_time (as the standard implies)
+	  // because %Z for sys_time would print "UTC" and we want "GPS" here.
 
 	  // Offset is 1980y/January/Sunday[1] - 1970y/January/1
 	  constexpr chrono::days __gps_offset = chrono::days(3657);
@@ -2104,7 +2128,7 @@ namespace __format
 	typename _FormatContext::iterator
 	format(const chrono::__detail::__local_time_fmt<_Duration>& __t,
 	       _FormatContext& __ctx) const
-	{ return _M_f._M_format(__t, __ctx); }
+	{ return _M_f._M_format(__t, __ctx, /* use %Z for {} */ true); }
 
     private:
       __format::__formatter_chrono<_CharT> _M_f;
diff --git a/libstdc++-v3/testsuite/std/time/clock/gps/io.cc b/libstdc++-v3/testsuite/std/time/clock/gps/io.cc
index d5405f61520..e995d9f3d78 100644
--- a/libstdc++-v3/testsuite/std/time/clock/gps/io.cc
+++ b/libstdc++-v3/testsuite/std/time/clock/gps/io.cc
@@ -22,7 +22,6 @@ test_ostream()
   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);
diff --git a/libstdc++-v3/testsuite/std/time/clock/local/io.cc b/libstdc++-v3/testsuite/std/time/clock/local/io.cc
index bb682cd40cf..b6e8b355bd0 100644
--- a/libstdc++-v3/testsuite/std/time/clock/local/io.cc
+++ b/libstdc++-v3/testsuite/std/time/clock/local/io.cc
@@ -7,15 +7,85 @@
 
 void
 test_ostream()
+{
+  using namespace std::chrono;
+
+  auto lt = local_time<seconds>(1722198122s);
+
+  std::ostringstream ss;
+  ss << lt;
+  auto s = ss.str();
+  ss.str("");
+  ss.clear();
+  ss << sys_time<seconds>{lt.time_since_epoch()};
+  auto s2 = ss.str();
+  VERIFY( s == s2 );
+}
+
+void
+test_format()
 {
   using std::format;
   using namespace std::chrono;
 
-  auto st = sys_days{2000y/January/1};
-  auto tt = clock_cast<tai_clock>(st);
+  auto lt = local_seconds(1722198122s);
 
-  auto s = format("{0:%F %T %Z} == {1:%F %T %Z}", st, tt);
-  VERIFY( s == "2000-01-01 00:00:00 UTC == 2000-01-01 00:00:32 TAI" );
+#if __cpp_exceptions
+  auto args = std::make_format_args(lt);
+  try
+  {
+    (void) std::vformat("{:%Z}", args);
+    VERIFY(false);
+  }
+  catch (const std::format_error&)
+  {
+  }
+  try
+  {
+    (void) std::vformat("{:%z}", args);
+    VERIFY(false);
+  }
+  catch (const std::format_error&)
+  {
+  }
+#endif
+
+  auto s = format("{0:%F %T %a}", lt);
+  VERIFY( s == "2024-07-28 20:22:02 Sun" );
+
+  s = format("{}", lt);
+  VERIFY( s == "2024-07-28 20:22:02" );
+
+  // Test formatting for chrono::local_time_format and local-time-format-t too:
+  auto ltf = local_time_format(lt);
+  s = std::format("{:%F %T %a %b}", ltf);
+  VERIFY( s == "2024-07-28 20:22:02 Sun Jul" );
+#if __cpp_exceptions
+  try
+  {
+    (void) std::format("{:%Z}", ltf);
+    VERIFY(false);
+  }
+  catch (const std::format_error&)
+  {
+  }
+  try
+  {
+    (void) std::format("{:%z}", ltf);
+    VERIFY(false);
+  }
+  catch (const std::format_error&)
+  {
+  }
+#endif
+  std::string abbrev = "FOO";
+  seconds off = -3600s;
+  ltf = local_time_format(lt, &abbrev, &off);
+  s = std::format("{}", ltf);
+  VERIFY( s == "2024-07-28 20:22:02 FOO" );
+  s = std::format("{:%Z %T %F %z %Ez}", ltf);
+  __builtin_puts(s.c_str());
+  VERIFY( s == "FOO 20:22:02 2024-07-28 -0100 -01:00" );
 }
 
 void
diff --git a/libstdc++-v3/testsuite/std/time/clock/system/io.cc b/libstdc++-v3/testsuite/std/time/clock/system/io.cc
index 2cc116156f2..5c223a979c2 100644
--- a/libstdc++-v3/testsuite/std/time/clock/system/io.cc
+++ b/libstdc++-v3/testsuite/std/time/clock/system/io.cc
@@ -64,13 +64,19 @@ test_format()
 		 " | 25.708 | 17:26:25.708 | 1 | 51 | 51 | 1 | 51 | 12/19/22"
 		 " | 17:26:25 | 22 | 2022 | +0000 | UTC" );
 
+  auto st = std::chrono::time_point_cast<std::chrono::seconds>(t);
   auto loc = std::locale::classic();
   auto smod = std::format(loc, "{:%Ec %EC %Od %Oe %OH %OI %Om %OM %OS %Ou %OU"
 			       " %Ow %OW %Ex %EX %Oy %Ey %EY %Ez %Oz}", t);
   s = std::format("{:%c %C %d %e %H %I %m %M %S %u %U"
 		  " %w %W %x %X %y %y %Y +00:00 +00:00}",
-		  std::chrono::time_point_cast<std::chrono::seconds>(t));
+		  st);
   VERIFY( smod == s );
+
+  s = std::format("{}", t);
+  VERIFY( s == "2022-12-19 17:26:25.708" );
+  s = std::format("{0:} {0:=<21}", st);
+  VERIFY( s == "2022-12-19 17:26:25 2022-12-19 17:26:25==" );
 }
 
 void
diff --git a/libstdc++-v3/testsuite/std/time/clock/tai/io.cc b/libstdc++-v3/testsuite/std/time/clock/tai/io.cc
index 0fd61c0e612..dc0c3d26477 100644
--- a/libstdc++-v3/testsuite/std/time/clock/tai/io.cc
+++ b/libstdc++-v3/testsuite/std/time/clock/tai/io.cc
@@ -16,6 +16,9 @@ test_ostream()
 
   auto s = format("{0:%F %T %Z} == {1:%F %T %Z}", st, tt);
   VERIFY( s == "2000-01-01 00:00:00 UTC == 2000-01-01 00:00:32 TAI" );
+
+  s = std::format("{:=>21}", tt);
+  VERIFY( s == "==2000-01-01 00:00:32" );
 }
 
 void
diff --git a/libstdc++-v3/testsuite/std/time/zoned_time/io.cc b/libstdc++-v3/testsuite/std/time/zoned_time/io.cc
index 376b2734f19..ee3b9edba81 100644
--- a/libstdc++-v3/testsuite/std/time/zoned_time/io.cc
+++ b/libstdc++-v3/testsuite/std/time/zoned_time/io.cc
@@ -37,6 +37,7 @@ test_format()
 	       " | 25.708 | 12:26:25.708 | 1 | 51 | 51 | 1 | 51 | 12/19/22"
 	       " | 12:26:25 | 22 | 2022 | -0500 | EST" );
 
+#ifdef _GLIBCXX_USE_WCHAR_T
   std::wstring ws = std::format(L"{:%a | %A | %b | %B | %c"
 				 " | %C | %d | %D | %e | %F | %g | %G | %h"
 				 " | %H | %I | %j | %m | %M | %p | %r | %R"
@@ -47,6 +48,7 @@ test_format()
 		 " | 12 | 12 | 353 | 12 | 26 | PM | 12:26:25 PM | 12:26"
 		 " | 25.708 | 12:26:25.708 | 1 | 51 | 51 | 1 | 51 | 12/19/22"
 		 " | 12:26:25 | 22 | 2022 | -0500 | EST" );
+#endif
 
   auto loc = std::locale::classic();
   auto smod = std::format(loc, "{:%Ec %EC %Od %Oe %OH %OI %Om %OM %OS %Ou %OU"
@@ -55,6 +57,15 @@ test_format()
 		  " %w %W %x %X %y %y %Y -05:00 -05:00}",
 		  zoned_time<seconds>(zone, time_point_cast<seconds>(t)));
   VERIFY( smod == s );
+
+  s = std::format("{}", zt);
+  VERIFY( s == "2022-12-19 12:26:25.708 EST" );
+  s = std::format("{1:=>30}", 1, zt);
+  VERIFY( s == "===2022-12-19 12:26:25.708 EST" );
+#ifdef _GLIBCXX_USE_WCHAR_T
+  ws = std::format(L"{:+^34}", zoned_time<microseconds>(zone, t));
+  VERIFY( ws == L"++2022-12-19 12:26:25.708000 EST++" );
+#endif
 }
 
 int main()
-- 
2.45.2

[PATCH 2/2] libstdc++: Fix formatter for low-resolution chrono::zoned_time (LWG 4124)

[Jonathan Wakely]

Tested x86_64-linux.

-- >8 --

This implements the proposed resolution of LWG 4124, so that
low-resolution chrono::zoned_time objects can be formatted. The
formatter for zoned_time<D, P> needs to account for get_local_time
returning local_time<common_type_t<D, seconds>> not local_time<D>.

libstdc++-v3/ChangeLog:

	* include/bits/chrono_io.h (__local_time_fmt_for): New alias
	template.
	(formatter<zoned_time<D, P>>): Use __local_time_fmt_for.
	* testsuite/std/time/zoned_time/io.cc: Check zoned_time<minutes>
	can be formatted.
---
 libstdc++-v3/include/bits/chrono_io.h            | 12 +++++++++---
 libstdc++-v3/testsuite/std/time/zoned_time/io.cc |  4 ++++
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/libstdc++-v3/include/bits/chrono_io.h b/libstdc++-v3/include/bits/chrono_io.h
index e7e7deb2cde..d8a4a121113 100644
--- a/libstdc++-v3/include/bits/chrono_io.h
+++ b/libstdc++-v3/include/bits/chrono_io.h
@@ -164,6 +164,12 @@ namespace __detail
       const string* _M_abbrev;
       const seconds* _M_offset_sec;
     };
+
+  // _GLIBCXX_RESOLVE_LIB_DEFECTS
+  // 4124. Cannot format zoned_time with resolution coarser than seconds
+  template<typename _Duration>
+    using __local_time_fmt_for
+      = __local_time_fmt<common_type_t<_Duration, seconds>>;
 }
 /// @endcond
 
@@ -2137,15 +2143,15 @@ namespace __format
 #if _GLIBCXX_USE_CXX11_ABI || ! _GLIBCXX_USE_DUAL_ABI
   template<typename _Duration, typename _TimeZonePtr, typename _CharT>
     struct formatter<chrono::zoned_time<_Duration, _TimeZonePtr>, _CharT>
-    : formatter<chrono::__detail::__local_time_fmt<_Duration>, _CharT>
+    : formatter<chrono::__detail::__local_time_fmt_for<_Duration>, _CharT>
     {
       template<typename _FormatContext>
 	typename _FormatContext::iterator
 	format(const chrono::zoned_time<_Duration, _TimeZonePtr>& __tp,
 	       _FormatContext& __ctx) const
 	{
-	  using chrono::__detail::__local_time_fmt;
-	  using _Base = formatter<__local_time_fmt<_Duration>, _CharT>;
+	  using _Ltf = chrono::__detail::__local_time_fmt_for<_Duration>;
+	  using _Base = formatter<_Ltf, _CharT>;
 	  const chrono::sys_info __info = __tp.get_info();
 	  const auto __lf = chrono::local_time_format(__tp.get_local_time(),
 						      &__info.abbrev,
diff --git a/libstdc++-v3/testsuite/std/time/zoned_time/io.cc b/libstdc++-v3/testsuite/std/time/zoned_time/io.cc
index ee3b9edba81..c113eea6d3f 100644
--- a/libstdc++-v3/testsuite/std/time/zoned_time/io.cc
+++ b/libstdc++-v3/testsuite/std/time/zoned_time/io.cc
@@ -66,6 +66,10 @@ test_format()
   ws = std::format(L"{:+^34}", zoned_time<microseconds>(zone, t));
   VERIFY( ws == L"++2022-12-19 12:26:25.708000 EST++" );
 #endif
+
+  // LWG 4124. Cannot format zoned_time with resolution coarser than seconds
+  s = std::format("{}", zoned_time<minutes>(zone, time_point_cast<minutes>(t)));
+  VERIFY( s == "2022-12-19 12:26:00 EST" );
 }
 
 int main()
-- 
2.45.2