On Thu, Jun 26, 2025 at 2:52 PM Jonathan Wakely <jwak...@redhat.com> wrote:
> On Thu, 26 Jun 2025 at 13:30, Tomasz Kaminski <tkami...@redhat.com> wrote: > > > > > > > > On Thu, Jun 26, 2025 at 2:13 PM Tomasz Kaminski <tkami...@redhat.com> > wrote: > >> > >> > >> > >> On Thu, Jun 26, 2025 at 2:09 PM Jonathan Wakely <jwak...@redhat.com> > wrote: > >>> > >>> On 26/06/25 11:39 +0200, Tomasz Kamiński wrote: > >>> >This patch extract calls to _M_locale_fmt and construction of the > struct tm, > >>> >from the functions dedicated to each specifier, to main format loop in > >>> >_M_format_to functions. This removes duplicated code repeated for > specifiers. > >>> > >>> Great, this is exactly what I wanted to do. Removing all the branches > >>> to call _M_locale_fmt from each of the _M_xxx member functions makes > >>> them smaller and potentially faster. > >>> > >>> >To allow _M_locale_fmt to only be called if localized formatting is > enabled > >>> >('L' is present in chrono-format-spec), we provide a implementations > for > >>> >locale specific specifiers (%c, %r, %x, %X) that produces the same > result > >>> >as locale::classic(): > >>> > * %c is implemented as separate _M_c method > >>> > * %r is implemented as separate _M_r method > >>> > * %x is implemented together with %D, as they provide same behavior, > >>> > * %X is implemented together with %R as _M_R_X, as both of them do > not include > >>> > subseconds. > >>> > >>> Nice. > >>> > >>> >The handling of subseconds was also extracted to _M_subsecs function > that is > >>> >used by _M_S and _M_T specifier. The _M_T is now implemented in terms > of > >>> >_M_R_X (printing time without subseconds) and _M_subs. > >>> > > >>> >The __mod responsible for triggering localized formatting was removed > from > >>> >method handling most of specifiers, except: > >>> > * _M_S (for %S) for which it determines if subseconds should be > included, > >>> > * _M_z (for %z) for which it determines if ':' is used as separator. > >>> > > >>> > PR libstdc++/110739 > >>> > > >>> >libstdc++-v3/ChangeLog: > >>> > > >>> > * include/bits/chrono_io.h > (__formatter_chrono::_M_use_locale_fmt): > >>> > Define. > >>> > (__formatter_chrono::_M_locale_fmt): Moved to front of the > class. > >>> > (__formatter_chrono::_M_format_to): Construct and initialize > >>> > struct tm and call _M_locale_fmt if needed. > >>> > (__formatter_chrono::_M_c_r_x_X): Split into separate methods. > >>> > (__formatter_chrono::_M_c, __formatter_chrono::_M_r): Define. > >>> > (__formatter_chrono::_M_D): Renamed to _M_D_x. > >>> > (__formatter_chrono::_M_D_x): Renamed from _M_D. > >>> > (__formatter_chrono::_M_R_T): Split into _M_R_X and _M_T. > >>> > (__formatter_chrono::_M_R_X): Extracted from _M_R_T. > >>> > (__formatter_chrono::_M_T): Define in terms of _M_R_X and > _M_subsecs. > >>> > (__formatter_chrono::_M_subsecs): Extracted from _M_S. > >>> > (__formatter_chrono::_M_S): Replaced __mod with __subs > argument, > >>> > removed _M_locale_fmt call, and delegate to _M_subsecs. > >>> > (__formatter_chrono::_M_C_y_Y, __formatter_chrono::_M_d_e) > >>> > (__formatter_chrono::_M_H_I, __formatter_chrono::_M_m) > >>> > (__formatter_chrono::_M_u_w, __formatter_chrono::_M_U_V_W): > Remove > >>> > __mod argument and call to _M_locale_fmt. > >>> >--- > >>> > libstdc++-v3/include/bits/chrono_io.h | 340 > +++++++++++++------------- > >>> > 1 file changed, 172 insertions(+), 168 deletions(-) > >>> > > >>> >diff --git a/libstdc++-v3/include/bits/chrono_io.h > b/libstdc++-v3/include/bits/chrono_io.h > >>> >index 35e95906e6a..d451bde722d 100644 > >>> >--- a/libstdc++-v3/include/bits/chrono_io.h > >>> >+++ b/libstdc++-v3/include/bits/chrono_io.h > >>> >@@ -906,6 +906,40 @@ namespace __format > >>> > return __format::__write(std::move(__out), __s); > >>> > } > >>> > > >>> >+ [[__gnu__::__always_inline__]] > >>> >+ static bool > >>> >+ _S_localized_spec(_CharT __conv, _CharT __mod) > >>> >+ { > >>> >+ switch (__conv) > >>> >+ { > >>> >+ case 'c': > >>> >+ case 'r': > >>> >+ case 'x': > >>> >+ case 'X': > >>> >+ return true; > >>> >+ case 'z': > >>> >+ return false; > >>> >+ default: > >>> >+ return (bool)__mod; > >>> >+ }; > >>> >+ } > >>> >+ > >>> >+ // Use the formatting locale's std::time_put facet to produce > >>> >+ // a locale-specific representation. > >>> >+ template<typename _Iter> > >>> >+ _Iter > >>> >+ _M_locale_fmt(_Iter __out, const locale& __loc, const struct > tm& __tm, > >>> >+ char __fmt, char __mod) const > >>> >+ { > >>> >+ basic_ostringstream<_CharT> __os; > >>> >+ __os.imbue(__loc); > >>> >+ const auto& __tp = use_facet<time_put<_CharT>>(__loc); > >>> >+ __tp.put(__os, __os, _S_space, &__tm, __fmt, __mod); > >>> >+ if (__os) > >>> >+ __out = _M_write(std::move(__out), __loc, __os.view()); > >>> >+ return __out; > >>> >+ } > >>> >+ > >>> > template<typename _Out, typename _FormatContext> > >>> > _Out > >>> > _M_format_to(const _ChronoData<_CharT>& __t, _Out __out, > >>> >@@ -923,6 +957,36 @@ namespace __format > >>> > return std::move(__out); > >>> > }; > >>> > > >>> >+ struct tm __tm{}; > >>> >+ bool __use_locale_fmt = false; > >>> >+ if (_M_spec._M_localized && _M_spec._M_locale_specific) > >>> >+ if (__fc.locale() != locale::classic()) > >>> >+ { > >>> >+ __use_locale_fmt = true; > >>> >+ > >>> >+ __tm.tm_year = (int)__t._M_year - 1900; > >>> >+ __tm.tm_yday = __t._M_day_of_year.count(); > >>> >+ __tm.tm_mon = (unsigned)__t._M_month - 1; > >>> >+ __tm.tm_mday = (unsigned)__t._M_day; > >>> >+ __tm.tm_wday = __t._M_weekday.c_encoding(); > >>> >+ __tm.tm_hour = __t._M_hours.count(); > >>> >+ __tm.tm_min = __t._M_minutes.count(); > >>> >+ __tm.tm_sec = __t._M_seconds.count(); > >>> >+ > >>> >+ // Some locales use %Z in their %c format but we don't > want strftime > >>> >+ // to use the system's local time zone (from > /etc/localtime or $TZ) > >>> >+ // as the output for %Z. Setting tm_isdst to -1 says > there is no > >>> >+ // time zone info available for the time in __tm. > >>> >+ __tm.tm_isdst = -1; > >>> >+ > >>> >+#ifdef _GLIBCXX_USE_STRUCT_TM_TM_ZONE > >>> >+ // POSIX.1-2024 adds tm.tm_zone which will be used for > %Z. > >>> >+ // BSD has had tm_zone since 1987 but as char* so cast > away const. > >>> >+ if (__t._M_zone_cstr) > >>> >+ __tm.tm_zone = const_cast<char*>(__t._M_zone_cstr); > >>> >+#endif > >>> >+ } > >>> >+ > >>> > // Characters to output for "%n", "%t" and "%%" specifiers. > >>> > constexpr const _CharT* __literals = _GLIBCXX_WIDEN("\n\t%"); > >>> > > >>> >@@ -932,7 +996,10 @@ namespace __format > >>> > do > >>> > { > >>> > _CharT __c = *__first++; > >>> >- switch (__c) > >>> >+ if (__use_locale_fmt && _S_localized_spec(__c, __mod)) > [[unlikely]] > >>> >+ __out = _M_locale_fmt(std::move(__out), __fc.locale(), > >>> >+ __tm, __c, __mod); > >>> >+ else switch (__c) > >>> > { > >>> > // %\0 is extension for handling weekday index > >>> > case '\0': > >>> >@@ -948,22 +1015,20 @@ namespace __format > >>> > __out = _M_b_B(__t._M_month, std::move(__out), __fc, > __c == 'B'); > >>> > break; > >>> > case 'c': > >>> >- case 'r': > >>> >- case 'x': > >>> >- case 'X': > >>> >- __out = _M_c_r_x_X(__t, std::move(__out), __fc, __c, > __mod); > >>> >+ __out = _M_c(__t, std::move(__out), __fc); > >>> > break; > >>> > case 'C': > >>> > case 'y': > >>> > case 'Y': > >>> >- __out = _M_C_y_Y(__t._M_year, std::move(__out), > __fc, __c, __mod); > >>> >+ __out = _M_C_y_Y(__t._M_year, std::move(__out), > __fc, __c); > >>> > break; > >>> > case 'd': > >>> > case 'e': > >>> >- __out = _M_d_e(__t._M_day, std::move(__out), __fc, > __c, __mod == 'O'); > >>> >+ __out = _M_d_e(__t._M_day, std::move(__out), __fc, > __c); > >>> > break; > >>> > case 'D': > >>> >- __out = _M_D(__t, std::move(__out), __fc); > >>> >+ case 'x': > >>> >+ __out = _M_D_x(__t, std::move(__out), __fc); > >>> > break; > >>> > case 'F': > >>> > __out = _M_F(__t, std::move(__out), __fc); > >>> >@@ -974,16 +1039,16 @@ namespace __format > >>> > break; > >>> > case 'H': > >>> > case 'I': > >>> >- __out = _M_H_I(__t._M_hours, __print_sign(), __fc, > __c, __mod == 'O'); > >>> >+ __out = _M_H_I(__t._M_hours, __print_sign(), __fc, > __c); > >>> > break; > >>> > case 'j': > >>> > __out = _M_j(__t, __print_sign(), __fc); > >>> > break; > >>> > case 'm': > >>> >- __out = _M_m(__t._M_month, std::move(__out), __fc, > __mod == 'O'); > >>> >+ __out = _M_m(__t._M_month, std::move(__out), __fc); > >>> > break; > >>> > case 'M': > >>> >- __out = _M_M(__t._M_minutes, __print_sign(), __fc, > __mod == 'O'); > >>> >+ __out = _M_M(__t._M_minutes, __print_sign(), __fc); > >>> > break; > >>> > case 'p': > >>> > __out = _M_p(__t._M_hours, std::move(__out), __fc); > >>> >@@ -994,22 +1059,28 @@ namespace __format > >>> > case 'Q': > >>> > __out = _M_Q(__t, __print_sign(), __fc); > >>> > break; > >>> >+ case 'r': > >>> >+ __out = _M_r(__t, __print_sign(), __fc); > >>> >+ break; > >>> > case 'R': > >>> >+ case 'X': > >>> >+ __out = _M_R_X(__t, __print_sign(), __fc, __c != > 'R'); > >>> >+ break; > >>> > case 'T': > >>> >- __out = _M_R_T(__t, __print_sign(), __fc, __c == > 'T'); > >>> >+ __out = _M_T(__t, __print_sign(), __fc); > >>> > break; > >>> >+ > >>> > case 'S': > >>> >- __out = _M_S(__t, __print_sign(), __fc, __mod == > 'O'); > >>> >+ __out = _M_S(__t, __print_sign(), __fc, __mod != > 'O'); > >>> > break; > >>> > case 'u': > >>> > case 'w': > >>> >- __out = _M_u_w(__t._M_weekday, std::move(__out), > __fc, __c, __mod == 'O'); > >>> >+ __out = _M_u_w(__t._M_weekday, std::move(__out), > __fc, __c); > >>> > break; > >>> > case 'U': > >>> > case 'V': > >>> > case 'W': > >>> >- __out = _M_U_V_W(__t, std::move(__out), __fc, __c, > >>> >- __mod == 'O'); > >>> >+ __out = _M_U_V_W(__t, std::move(__out), __fc, __c); > >>> > break; > >>> > case 'z': > >>> > __out = _M_z(__t._M_zone_offset, std::move(__out), > __fc, (bool)__mod); > >>> >@@ -1132,50 +1203,27 @@ namespace __format > >>> > > >>> > template<typename _FormatContext> > >>> > typename _FormatContext::iterator > >>> >- _M_c_r_x_X(const _ChronoData<_CharT>& __t, typename > _FormatContext::iterator __out, > >>> >- _FormatContext& __ctx, _CharT __conv, _CharT __mod) > const > >>> >+ _M_c(const _ChronoData<_CharT>& __t, typename > _FormatContext::iterator __out, > >>> >+ _FormatContext& __ctx) const > >>> > { > >>> >- // %c Locale's date and time representation. > >>> >- // %Ec Locale's alternate date and time representation. > >>> >- // %r Locale's 12-hour clock time. > >>> >- // %x Locale's date rep > >>> >- // %Ex Locale's alternative date representation. > >>> >- // %X Locale's time rep > >>> >- // %EX Locale's alternative time representation. > >>> >- > >>> >- using namespace chrono; > >>> >- struct tm __tm{}; > >>> >- > >>> >- // Some locales use %Z in their %c format but we don't want > strftime > >>> >- // to use the system's local time zone (from /etc/localtime > or $TZ) > >>> >- // as the output for %Z. Setting tm_isdst to -1 says there > is no > >>> >- // time zone info available for the time in __tm. > >>> >- __tm.tm_isdst = -1; > >>> >- > >>> >-#ifdef _GLIBCXX_USE_STRUCT_TM_TM_ZONE > >>> >- // POSIX.1-2024 adds tm.tm_zone which will be used for %Z. > >>> >- // BSD has had tm_zone since 1987 but as char* so cast away > const. > >>> >- if (__t._M_zone_cstr) > >>> >- __tm.tm_zone = const_cast<char*>(__t._M_zone_cstr); > >>> >-#endif > >>> >- > >>> >- __tm.tm_year = (int)__t._M_year - 1900; > >>> >- __tm.tm_yday = __t._M_day_of_year.count(); > >>> >- __tm.tm_mon = (unsigned)__t._M_month - 1; > >>> >- __tm.tm_mday = (unsigned)__t._M_day; > >>> >- __tm.tm_wday = __t._M_weekday.c_encoding(); > >>> >- __tm.tm_hour = __t._M_hours.count(); > >>> >- __tm.tm_min = __t._M_minutes.count(); > >>> >- __tm.tm_sec = __t._M_seconds.count(); > >>> >- > >>> >- return _M_locale_fmt(std::move(__out), _M_locale(__ctx), > __tm, > >>> >- __conv, __mod); > >>> >+ // %c Locale's date and time representation, for C-locale: > %a %b %e %T %Y > >>> >+ // %Ec Locale's alternate date and time representation, for > C-locale same as above > >>> >+ > >>> >+ __out = _M_a_A(__t._M_weekday, std::move(__out), __ctx, > false); > >>> >+ *__out = _S_space; > >>> >+ __out = _M_b_B(__t._M_month, std::move(++__out), __ctx, > false); > >>> >+ *__out = _S_space; > >>> >+ __out = _M_d_e(__t._M_day, std::move(++__out), __ctx, 'e'); > >>> >+ *__out = _S_space; > >>> >+ __out = _M_R_X(__t, std::move(++__out), __ctx, true); > >>> >+ *__out = _S_space; > >>> >+ return _M_C_y_Y(__t._M_year, std::move(__out), __ctx, 'Y'); > >>> > } > >>> > > >>> > template<typename _FormatContext> > >>> > typename _FormatContext::iterator > >>> > _M_C_y_Y(chrono::year __y, typename _FormatContext::iterator > __out, > >>> >- _FormatContext& __ctx, _CharT __conv, _CharT __mod = > 0) const > >>> >+ _FormatContext& __ctx, _CharT __conv) const > >>> > { > >>> > // %C Year divided by 100 using floored division. > >>> > // %EC Locale's alternative preresentation of the century > (era name). > >>> >@@ -1185,15 +1233,6 @@ namespace __format > >>> > // %Y Year as a decimal number. > >>> > // %EY Locale's alternative full year representation. > >>> > > >>> >- if (__mod && _M_spec._M_localized) [[unlikely]] > >>> >- if (auto __loc = __ctx.locale(); __loc != > locale::classic()) > >>> >- { > >>> >- struct tm __tm{}; > >>> >- __tm.tm_year = (int)__y - 1900; > >>> >- return _M_locale_fmt(std::move(__out), __loc, __tm, > >>> >- __conv, __mod); > >>> >- } > >>> >- > >>> > int __yi = (int)__y; > >>> > const bool __is_neg = __yi < 0; > >>> > __yi = __builtin_abs(__yi); > >>> >@@ -1235,9 +1274,13 @@ namespace __format > >>> > > >>> > template<typename _FormatContext> > >>> > typename _FormatContext::iterator > >>> >- _M_D(const _ChronoData<_CharT>& __t, typename > _FormatContext::iterator __out, > >>> >- _FormatContext&) const > >>> >+ _M_D_x(const _ChronoData<_CharT>& __t, typename > _FormatContext::iterator __out, > >>> >+ _FormatContext&) const > >>> > { > >>> >+ // %D Equivalent to %m/%d/%y > >>> >+ // %x Locale's date rep, for C-locale: %m/%d/%y > >>> >+ // %Ex Locale's alternative date representation, for > C-locale same as above > >>> >+ > >>> > auto __di = (unsigned)__t._M_day; > >>> > auto __mi = (unsigned)__t._M_month; > >>> > auto __yi = __builtin_abs((int)__t._M_year) % 100; > >>> >@@ -1267,7 +1310,7 @@ namespace __format > >>> > template<typename _FormatContext> > >>> > typename _FormatContext::iterator > >>> > _M_d_e(chrono::day __d, typename _FormatContext::iterator > __out, > >>> >- _FormatContext& __ctx, _CharT __conv, bool __mod = > false) const > >>> >+ _FormatContext& __ctx, _CharT __conv) const > >>> > { > >>> > // %d The day of month as a decimal number. > >>> > // %Od Locale's alternative representation. > >>> >@@ -1276,15 +1319,6 @@ namespace __format > >>> > > >>> > unsigned __i = (unsigned)__d; > >>> > > >>> >- if (__mod && _M_spec._M_localized) [[unlikely]] > >>> >- if (auto __loc = __ctx.locale(); __loc != > locale::classic()) > >>> >- { > >>> >- struct tm __tm{}; > >>> >- __tm.tm_mday = __i; > >>> >- return _M_locale_fmt(std::move(__out), __loc, __tm, > >>> >- (char)__conv, 'O'); > >>> >- } > >>> >- > >>> > _CharT __buf[3]; > >>> > auto __sv = _S_str_d2(__buf, __i); > >>> > if (__conv == _CharT('e') && __i < 10) > >>> >@@ -1360,7 +1394,7 @@ namespace __format > >>> > template<typename _FormatContext> > >>> > typename _FormatContext::iterator > >>> > _M_H_I(chrono::hours __h, typename _FormatContext::iterator > __out, > >>> >- _FormatContext& __ctx, _CharT __conv, bool __mod = > false) const > >>> >+ _FormatContext& __ctx, _CharT __conv) const > >>> > { > >>> > // %H The hour (24-hour clock) as a decimal number. > >>> > // %OH Locale's alternative representation. > >>> >@@ -1369,15 +1403,6 @@ namespace __format > >>> > > >>> > int __i = __h.count(); > >>> > > >>> >- if (__mod && _M_spec._M_localized) [[unlikely]] > >>> >- if (auto __loc = __ctx.locale(); __loc != > locale::classic()) > >>> >- { > >>> >- struct tm __tm{}; > >>> >- __tm.tm_hour = __i; > >>> >- return _M_locale_fmt(std::move(__out), __loc, __tm, > >>> >- (char)__conv, 'O'); > >>> >- } > >>> >- > >>> > if (__conv == _CharT('I')) > >>> > { > >>> > __i %= 12; > >>> >@@ -1409,22 +1434,13 @@ namespace __format > >>> > template<typename _FormatContext> > >>> > typename _FormatContext::iterator > >>> > _M_m(chrono::month __m, typename _FormatContext::iterator > __out, > >>> >- _FormatContext& __ctx, bool __mod) const > >>> >+ _FormatContext& __ctx) const > >>> > { > >>> > // %m month as a decimal number. > >>> > // %Om Locale's alternative representation. > >>> > > >>> > auto __i = (unsigned)__m; > >>> > > >>> >- if (__mod && _M_spec._M_localized) [[unlikely]] // %Om > >>> >- if (auto __loc = __ctx.locale(); __loc != > locale::classic()) > >>> >- { > >>> >- struct tm __tm{}; > >>> >- __tm.tm_mon = __i - 1; > >>> >- return _M_locale_fmt(std::move(__out), __loc, __tm, > >>> >- 'm', 'O'); > >>> >- } > >>> >- > >>> > _CharT __buf[3]; > >>> > return __format::__write(std::move(__out), _S_str_d2(__buf, > __i)); > >>> > } > >>> >@@ -1432,22 +1448,12 @@ namespace __format > >>> > template<typename _FormatContext> > >>> > typename _FormatContext::iterator > >>> > _M_M(chrono::minutes __m, typename _FormatContext::iterator > __out, > >>> >- _FormatContext& __ctx, bool __mod) const > >>> >+ _FormatContext& __ctx) const > >>> > { > >>> > // %M The minute as a decimal number. > >>> > // %OM Locale's alternative representation. > >>> > > >>> > auto __i = __m.count(); > >>> >- > >>> >- if (__mod && _M_spec._M_localized) [[unlikely]] // %OM > >>> >- if (auto __loc = __ctx.locale(); __loc != > locale::classic()) > >>> >- { > >>> >- struct tm __tm{}; > >>> >- __tm.tm_min = __i; > >>> >- return _M_locale_fmt(std::move(__out), __loc, __tm, > >>> >- 'M', 'O'); > >>> >- } > >>> >- > >>> > return __format::__write(std::move(__out), > _S_two_digits(__i)); > >>> > } > >>> > > >>> >@@ -1488,17 +1494,42 @@ namespace __format > >>> > > >>> > template<typename _FormatContext> > >>> > typename _FormatContext::iterator > >>> >- _M_R_T(const _ChronoData<_CharT>& __t, typename > _FormatContext::iterator __out, > >>> >+ _M_r(const _ChronoData<_CharT>& __t, typename > _FormatContext::iterator __out, > >>> >+ _FormatContext& __ctx) const > >>> >+ { > >>> >+ // %r Locale's 12-hour clock time, for C-locale: %I:%M:%S %p > >>> >+ auto __hi = __t._M_hours.count() % 12; > >>> >+ if (__hi == 0) > >>> >+ __hi = 12; > >>> >+ > >>> >+ _CharT __buf[9]; > >>> >+ __buf[2] = _S_colon; > >>> >+ __buf[5] = _S_colon; > >>> >+ __buf[8] = _S_space; > >>> >+ _S_fill_two_digits(__buf, __hi); > >>> >+ _S_fill_two_digits(__buf + 3, __t._M_minutes.count()); > >>> >+ _S_fill_two_digits(__buf + 6, __t._M_seconds.count()); > >>> >+ > >>> >+ __string_view __sv(__buf, 9); > >>> >+ __out = __format::__write(std::move(__out), __sv); > >>> >+ return _M_p(__t._M_hours, std::move(__out), __ctx); > >>> >+ } > >>> >+ > >>> >+ template<typename _FormatContext> > >>> >+ typename _FormatContext::iterator > >>> >+ _M_R_X(const _ChronoData<_CharT>& __t, typename > _FormatContext::iterator __out, > >>> > _FormatContext& __ctx, bool __secs) const > >>> > { > >>> >- // %R Equivalent to %H:%M > >>> >- // %T Equivalent to %H:%M:%S > >>> >+ // %R Equivalent to %H:%M > >>> >+ // %X Locale's time rep, for C-locale: %H:%M:%S (without > subseconds) > >>> >+ // %EX Locale's alternative time representation, for > C-locale same as above > >>> >+ > >>> > auto __hi = __t._M_hours.count(); > >>> > > >>> >- _CharT __buf[6]; > >>> >+ _CharT __buf[8]; > >>> > __buf[2] = _S_colon; > >>> > __buf[5] = _S_colon; > >>> >- __string_view __sv(__buf, 5 + __secs); > >>> >+ __string_view __sv(__buf, 8); > >>> > > >>> > if (__hi >= 100) [[unlikely]] > >>> > { > >>> >@@ -1509,39 +1540,39 @@ namespace __format > >>> > _S_fill_two_digits(__buf, __hi); > >>> > > >>> > _S_fill_two_digits(__buf + 3, __t._M_minutes.count()); > >>> >- __out = __format::__write(std::move(__out), __sv); > >>> > if (__secs) > >>> >- __out = _M_S(__t, std::move(__out), __ctx); > >>> >- return __out; > >>> >+ _S_fill_two_digits(__buf + 6, __t._M_seconds.count()); > >>> >+ else > >>> >+ __sv.remove_suffix(3); > >>> >+ > >>> >+ return __format::__write(std::move(__out), __sv); > >>> > } > >>> > > >>> > template<typename _FormatContext> > >>> > typename _FormatContext::iterator > >>> > _M_S(const _ChronoData<_CharT>& __t, typename > _FormatContext::iterator __out, > >>> >- _FormatContext& __ctx, bool __mod = false) const > >>> >+ _FormatContext& __ctx, bool __subs = true) const > >>> > { > >>> >- // %S Seconds as a decimal number. > >>> > >>> Doesn't this function still handle %S formats? Should this comment be > kept? > >> > >> Yes, it gets called to handle '%S' in the big switch. It is no longer > called by _M_T, which calls _M_subsecs > >> directly. > > > > Ah, you are referring to 'OS'. Yes, it is still-handled here, if the > format is not localized, or the locale is the same > > as classic. The effect is that we do not print subseconds. > > What I mean is that the _M_S function handles both %S and %OS, but you > remove the comment about %S which suggests to me that it only handles > %OS now (but that's not correct). > > Should the first comment line be restored? > Yes, I did that already locally, and forgot to mention it. I removed it by accident.