[PATCH 3/4] libstdc++: Format empty chrono-spec for the sys_info and local_info directly.

[Jonathan Wakely]

On 06/06/25 12:41 +0200, Tomasz Kamiński wrote:
>This patch change implementation of the formatters for sys_info and local_info,
>so they no longer delegate to operator<< for ostream in case of empty spec.
>As this types may be only formatted with empty chrono-spec (with padding), they now
>use a separate __formatter_chrono_info formatter.
>
>The __formatter_chrono_info formats sys_info using format_to call with format
>specifier extracted from corresponding operator<<, that now delegates to format
>with empty spec. For local_info we replicate functionality of the operator<<.
>The alignment and padding is handled using an _Padding_sink.
>
>libstdc++-v3/ChangeLog:
>
>	* include/bits/chrono_io.h (__format::__formatter_chrono_info):
>	Define.
>	(std::formatter<chrono::sys_info, _CharT>)
>	(std::formatter<chrono::local_inf, _CharT>): Delegate to
>	__format::__formatter_chrono_info.
>	(std::operator<<(basic_ostream<_CharT, _Traits>& const sys_info&)):
>	Use format on sys_info with empty format spec.
>---
> libstdc++-v3/include/bits/chrono_io.h | 132 +++++++++++++++++++++++---
> 1 file changed, 117 insertions(+), 15 deletions(-)
>
>diff --git a/libstdc++-v3/include/bits/chrono_io.h b/libstdc++-v3/include/bits/chrono_io.h
>index 24c7dfb91c9..bbbae3d3064 100644
>--- a/libstdc++-v3/include/bits/chrono_io.h
>+++ b/libstdc++-v3/include/bits/chrono_io.h
>@@ -1948,6 +1948,104 @@ namespace __format
> 	}
>     };
>
>+  template<typename _CharT>
>+    struct __formatter_chrono_info
>+    {
>+      constexpr typename basic_format_parse_context<_CharT>::iterator
>+      _M_parse(basic_format_parse_context<_CharT>& __pc)
>+      {
>+	auto __first = __pc.begin();
>+	auto __last = __pc.end();
>+
>+	_Spec<_CharT> __spec{};
>+
>+	auto __finished = [&] {
>+	  if (__first == __last || *__first == '}')
>+	    {
>+	      _M_spec = __spec;
>+	      return true;
>+	    }
>+	  return false;
>+	};
>+
>+	if (__finished())
>+	  return __first;
>+
>+	__first = __spec._M_parse_fill_and_align(__first, __last);
>+	if (__finished())
>+	  return __first;
>+
>+	__first = __spec._M_parse_width(__first, __last, __pc);
>+	if (__finished())
>+	  return __first;
>+
>+	return __spec._M_parse_locale(__first, __last);

It looks like if we reach this point, i.e. the L option is present,
then we don't do _M_spec = __spec?

And do we want to cause parsing to fail if __finished() == false after
parsing the locale option? Otherwise doesn't it mean we will ignore
any garbage after the L in the format string?

Ah no, I see you handle that in formatter::parse after calling
_M_f._M_parse.

But I don't think it's true that the chrono-specs must be empty for
local_info and sys_info, consider:

std::format("{:%n %t %%}", std::chrono::sys_info{})

Does __formatter_chrono_info need to parse and store the chrono-specs,
and then handle that in its _M_format? Only whitespace characters are
valid, and if there's a non-empty chrono-specs then we don't use the
format arg at all, just loop over the chrono-specs and output the
relevant whitespace chars. And that loop can be [[unlikely]] because
I doubt anybody will ever do this!


>+      }
>+
>+      template<typename _Info, typename _Out>
>+	typename basic_format_context<_Out, _CharT>::iterator
>+	format(const _Info& __i,
>+	       basic_format_context<_Out, _CharT>& __fc) const
>+	{
>+	  const size_t __padwidth = _M_spec._M_get_width(__fc);
>+	  if (__padwidth == 0)
>+	    return _M_format_to(__fc.out(), __i);
>+
>+	  _Padding_sink<_Out, _CharT> __sink(__fc.out(), __padwidth);
>+	  _M_format_to(__sink.out(), __i);
>+	  return __sink._M_finish(_M_spec._M_align, _M_spec._M_fill);
>+	}
>+
>+    private:
>+       template<typename _Out>
>+	 _Out
>+	 _M_format_to(_Out __out, const chrono::sys_info& __si) const
>+	 {
>+	   using _FmtStr = _Runtime_format_string<_CharT>;
>+	   // n.b. only decimal separator is locale dependent for specifiers
>+	   // used below, as sys_info uses seconds and minutes duration, the
>+	   // output is locale-independent.
>+	   constexpr auto* __fs
>+	     = _GLIBCXX_WIDEN("[{0:%F %T},{1:%F %T},{2:%T},{3:%Q%q},{0:%Z}]");
>+	   const chrono::local_seconds __lb(__si.begin.time_since_epoch());
>+	   return std::format_to(std::move(__out), _FmtStr(__fs),
>+				 chrono::local_time_format(__lb, &__si.abbrev),
>+				 __si.end, __si.offset, __si.save);
>+	 }
>+
>+       template<typename _Out>
>+	 _Out
>+	 _M_format_to(_Out __out, const chrono::local_info& __li) const
>+	 {
>+	   *__out = _Separators<_CharT>::_S_squares()[0];
>+	   ++__out;
>+	   if (__li.result == chrono::local_info::unique)
>+	     __out = _M_format_to(std::move(__out), __li.first);
>+	   else
>+	    {
>+	      basic_string_view<_CharT> __sv;
>+	      if (__li.result == chrono::local_info::nonexistent)
>+		__sv =_GLIBCXX_WIDEN("nonexistent");
>+	      else
>+		__sv = _GLIBCXX_WIDEN("ambiguous");
>+	      __out = __format::__write(std::move(__out), __sv);
>+
>+	      __sv = _GLIBCXX_WIDEN(" local time between ");
>+	      __out = __format::__write(std::move(__out), __sv);
>+	      __out = _M_format_to(std::move(__out), __li.first);
>+
>+	      __sv = _GLIBCXX_WIDEN(" and ");
>+	      __out = __format::__write(std::move(__out), __sv);
>+	      __out = _M_format_to(std::move(__out), __li.second);
>+	    }
>+	  *__out = _Separators<_CharT>::_S_squares()[1];
>+	  ++__out;
>+	  return std::move(__out);
>+	}
>+
>+      _Spec<_CharT> _M_spec;
>+    };
>+
> } // namespace __format
> /// @endcond
>
>@@ -2438,16 +2536,22 @@ namespace __format
>     {
>       constexpr typename basic_format_parse_context<_CharT>::iterator
>       parse(basic_format_parse_context<_CharT>& __pc)
>-      { return _M_f._M_parse(__pc, __format::_ChronoParts{}); }
>+      {
>+     	auto __res = _M_f._M_parse(__pc);
>+	if (__res == __pc.end() || *__res == '}')
>+	  return __res;
>+	std::__throw_format_error("format error: invalid format-spec for "
>+				   "std::chrono::sys_info");
>+      }
>
>       template<typename _Out>
> 	typename basic_format_context<_Out, _CharT>::iterator
> 	format(const chrono::sys_info& __i,
> 	       basic_format_context<_Out, _CharT>& __fc) const
>-	{ return _M_f._M_format(__i, __fc); }
>+	{ return _M_f.format(__i, __fc); }
>
>     private:
>-      __format::__formatter_chrono<_CharT> _M_f;
>+      __format::__formatter_chrono_info<_CharT> _M_f;
>     };
>
>   template<__format::__char _CharT>
>@@ -2455,16 +2559,22 @@ namespace __format
>     {
>       constexpr typename basic_format_parse_context<_CharT>::iterator
>       parse(basic_format_parse_context<_CharT>& __pc)
>-      { return _M_f._M_parse(__pc, __format::_ChronoParts{}); }
>+      {
>+     	auto __res = _M_f._M_parse(__pc);
>+	if (__res == __pc.end() || *__res == '}')
>+	  return __res;
>+	std::__throw_format_error("format error: invalid format-spec for "
>+				   "std::chrono::local_info");
>+      }
>
>       template<typename _Out>
> 	typename basic_format_context<_Out, _CharT>::iterator
> 	format(const chrono::local_info& __i,
> 	       basic_format_context<_Out, _CharT>& __fc) const
>-	{ return _M_f._M_format(__i, __fc); }
>+	{ return _M_f.format(__i, __fc); }
>
>     private:
>-      __format::__formatter_chrono<_CharT> _M_f;
>+      __format::__formatter_chrono_info<_CharT> _M_f;
>     };
> #endif
>
>@@ -3326,15 +3436,7 @@ namespace __detail
>     basic_ostream<_CharT, _Traits>&
>     operator<<(basic_ostream<_CharT, _Traits>& __os, const sys_info& __i)
>     {
>-      // n.b. only decimal separator is locale dependent for specifiers
>-      // used below, as sys_info uses seconds and minutes duration, the
>-      // output is locale-independent.
>-      constexpr auto* __fs
>-	= _GLIBCXX_WIDEN("[{0:%F %T},{1:%F %T},{2:%T},{3:%Q%q},{0:%Z}]");
>-      local_seconds __lb(__i.begin.time_since_epoch());
>-      __os << std::format(__fs, local_time_format(__lb, &__i.abbrev),
>-			  __i.end, __i.offset, __i.save);
>-      return __os;
>+      return __os << std::format(__os.getloc(), _GLIBCXX_WIDEN("{}"), __i);
>     }
>
>   /// Writes a local_info object to an ostream in an unspecified format.
>-- 
>2.49.0
>
>