On 12/06/25 14:22 +0100, Jonathan Wakely wrote:
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!
Of course this shows that my tests for formatting these types were
inadequate ... in fact I don't think there are *any* tests, except the
empty_spec.cc ones you added recently.
