[PATCH] libstdc++: Use runtime format for internal format calls in chrono [PR110739]

[Jonathan Wakely]

On Fri, 27 Jun 2025 at 10:10, Tomasz Kaminski <tkaminsk@redhat.com> wrote:
>
>
>
> On Fri, Jun 27, 2025 at 10:31 AM Tomasz Kamiński <tkaminsk@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
>>