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

[Jonathan Wakely]

On Fri, 27 Jun 2025 at 11:01, Tomasz Kaminski <tkaminsk@redhat.com> wrote:
>
>
>
> On Fri, Jun 27, 2025 at 11:23 AM Jonathan Wakely <jwakely@redhat.com> wrote:
>>
>> 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? :-)
>
> Yes, when I say tested on x86_64-linux locally, I mean that check-target-libstdc++-v3 passed,
> but not with extended config (used on farms). And then list additional runs I checked beyond that.

Got it, thanks - just wanted to be sure :-)


>
>
>>
>>
>> > 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
>> >>
>>