On Fri, 27 Jun 2025 at 10:10, Tomasz Kaminski <tkami...@redhat.com> wrote:
>
>
>
> On Fri, Jun 27, 2025 at 10:31 AM Tomasz Kamiński <tkami...@redhat.com> wrote:
>>
>> This patch adjust all internal std::format call inside of __formatter_chrono,
>> to use runtime format stirng and thus avoid compile time checking of validity
>> of the format string. Majority of cases are covered by calling newly
>> introduced
>> _S_empty_fs() function that returns __Runtime_format_string containing
>> _S_empty_spec, instead of passing later directly.
>>
>> In case of _M_j we use _S_str_d3 function (extracted from _S_str_d2),
>> eliminating
>> call to std::format outside of unlikely scenario in which day of year is
>> greater
>> than 1000 (this may happen for year_month_day with month greater than 12). In
>> consequence, outside of handling subseconds, we no longer delegate to
>> std::format
>> or construct temporary strings, when formatting chrono types with ok()
>> values.
>>
>> PR libstdc++/110739
>>
>> libstdc++-v3/ChangeLog:
>>
>> * include/bits/chrono_io.h (__formatter_chrono::_S_empty_fs): Define.
>> (__formatter_chrono::_S_str_d2): Use _S_str_d3 for 3+ digits.
>> (__formatter_chrono::_S_str_d3): Extracted from _S_str_d2.
>> (__formatter_chrono::_M_H_I, __formatter_chrono::_M_R_X): Replace
>> _S_empty_spec with _S_empty_fs().
>> (__formatter_chrono::_M_j): Likewise and use _S_str_d3 in common
>> case.
>> ---
>>
>>
>
> I do not think this buys us much, but I think it is worth doing anyway.
I agree, I've never really liked doing recursive std::format calls
inside formatters (obviously sometimes it's unavoidable, e.g. for
custom duration rep types).
> It also finishes my side goal, of getting rid of temporary strings, and using
> local buffers,
> that I applied to other specifiers in previous commits.
> Tested on x86_64-linux locally. The std/time* test passed with
> -D_GLIBCXX_USE_CXX11_ABI=0 and -D_GLIBCXX_DEBUG.
Just to be sure, it passes without those options too, right? :-)
> OK for trunk?
Please put the always_inline attribute after the comment on _S_str_d3.
OK for trunk with that change, thanks.
>
>>
>> libstdc++-v3/include/bits/chrono_io.h | 27 ++++++++++++++++++++++-----
>> 1 file changed, 22 insertions(+), 5 deletions(-)
>>
>> diff --git a/libstdc++-v3/include/bits/chrono_io.h
>> b/libstdc++-v3/include/bits/chrono_io.h
>> index bcfd51b9866..d6bc6c7cf2a 100644
>> --- a/libstdc++-v3/include/bits/chrono_io.h
>> +++ b/libstdc++-v3/include/bits/chrono_io.h
>> @@ -873,6 +873,11 @@ namespace __format
>> static constexpr const _CharT* _S_minus_empty_spec = _S_chars + 17;
>> static constexpr const _CharT* _S_empty_spec = _S_chars + 18;
>>
>> + [[__gnu__::__always_inline__]]
>> + static _Runtime_format_string<_CharT>
>> + _S_empty_fs()
>> + { return _Runtime_format_string<_CharT>(_S_empty_spec); }
>> +
>> // Return the formatting locale.
>> template<typename _FormatContext>
>> std::locale
>> @@ -1411,7 +1416,7 @@ namespace __format
>> __i = 12;
>> }
>> else if (__i >= 100) [[unlikely]]
>> - return std::format_to(std::move(__out), _S_empty_spec, __i);
>> + return std::format_to(std::move(__out), _S_empty_fs(), __i);
>>
>> return __format::__write(std::move(__out), _S_two_digits(__i));
>> }
>> @@ -1425,11 +1430,15 @@ namespace __format
>> {
>> // Decimal number of days, without padding.
>> auto __d = chrono::floor<chrono::days>(__t._M_hours).count();
>> - return std::format_to(std::move(__out), _S_empty_spec, __d);
>> + return std::format_to(std::move(__out), _S_empty_fs(), __d);
>> }
>>
>> - return std::format_to(std::move(__out), _GLIBCXX_WIDEN("{:03d}"),
>> - __t._M_day_of_year.count());
>> + auto __d = __t._M_day_of_year.count();
>> + if (__d >= 1000) [[unlikely]]
>> + return std::format_to(std::move(__out), _S_empty_fs(), __d);
>> +
>> + _CharT __buf[3];
>> + return __format::__write(std::move(__out), _S_str_d3(__buf, __d));
>> }
>>
>> template<typename _FormatContext>
>> @@ -1534,7 +1543,7 @@ namespace __format
>>
>> if (__hi >= 100) [[unlikely]]
>> {
>> - __out = std::format_to(std::move(__out), _S_empty_spec, __hi);
>> + __out = std::format_to(std::move(__out), _S_empty_fs(), __hi);
>> __sv.remove_prefix(2);
>> }
>> else
>> @@ -1772,7 +1781,15 @@ namespace __format
>> {
>> if (__n < 100) [[likely]]
>> return _S_two_digits(__n);
>> + return _S_str_d3(__buf, __n);
>> + }
>>
>> + [[__gnu__::__always_inline__]]
>> + // Returns decimal representation of __n, padded to 3 digits.
>> + // Returned string_view points to __buf.
>> + static basic_string_view<_CharT>
>> + _S_str_d3(span<_CharT, 3> __buf, unsigned __n)
>> + {
>> _S_fill_two_digits(__buf.data(), __n / 10);
>> __buf[2] = _S_chars[__n % 10];
>> return __string_view(__buf.data(), 3);
>> --
>> 2.49.0
>>
