From 43b8153c26655a7a00f1584fcb4f511dc5e45fab Mon Sep 17 00:00:00 2001 From: Jonathan Wakely Date: Wed, 31 Jul 2024 16:32:44 +0100 Subject: [PATCH] libstdc++: Only use std::time_put in std::format for non-C locales MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When testing on Solaris I noticed that std/time/year/io.cc was FAILing because the year 1642 was being formatted as "+(" by %Ey. This turns out to be because we defer to std::time_put for modified conversion specs, and std::time_put uses std::strftime, and that's undefined for years before 1970. In particular, years before 1900 mean that the tm_year field is negative, which then causes incorrect results from strftime on at least Solaris and AIX. I've raised the general problem with LWG, but we can fix the FAILing test case (and probably improve performance slightly) by ignoring the E and O modifiers when the formatting locale is the "C" locale. The modifiers have no effect for the C locale, so we can just treat %Ey as %y and format it directly. This doesn't fix anything when the formatting locale isn't the C locale, but that case is not adequately tested, so doesn't cause any FAIL right now! The naïve fix would be simply: if (__mod) if (auto __loc = _M_locale(__ctx); __loc != locale::classic()) // ... However when the format string doesn't use the 'L' option, _M_locale always returns locale::classic(). In that case, we make a copy of the classic locale (which calls the non-inline copy constructor in the library), then make another copy of the classic locale, then compare the two. We can avoid all that by checking for the 'L' option first, instead of letting _M_locale do that: if (__mod && _M_spec._M_localized) if (auto __loc = __ctx.locale(); __loc != locale::classic()) // ... We could optimize this further if we had a __is_classic(__loc) function that would do the __loc == locale::classic() check without making any copies or non-inline calls. That would require examining the locale's _M_impl member, and probably require checking its name, because the locale::_S_classic singleton is not exported from the library. For _M_S the change is slightly different from the other functions, because if we skip using std::time_put for %OS then we fall through to the code that potentially prints fractional seconds, but the %OS format only prints whole seconds. So we need to format whole seconds directly when not using std::time_put, instead of falling through to the code below. libstdc++-v3/ChangeLog: * include/bits/chrono_io.h (__formatter_chrono::_M_C_y_Y): Ignore modifiers unless the formatting locale is not the C locale. (__formatter_chrono::_M_d_e): Likewise. (__formatter_chrono::_M_H_I): Likewise. (__formatter_chrono::_M_m): Likewise. (__formatter_chrono::_M_M): Likewise. (__formatter_chrono::_M_S): Likewise. (__formatter_chrono::_M_u_w): Likewise. (__formatter_chrono::_M_U_V_W): Likewise. --- libstdc++-v3/include/bits/chrono_io.h | 133 ++++++++++++++------------ 1 file changed, 74 insertions(+), 59 deletions(-) diff --git a/libstdc++-v3/include/bits/chrono_io.h b/libstdc++-v3/include/bits/chrono_io.h index a449ffdc558..38a0b002c81 100644 --- a/libstdc++-v3/include/bits/chrono_io.h +++ b/libstdc++-v3/include/bits/chrono_io.h @@ -917,13 +917,14 @@ namespace __format chrono::year __y = _S_year(__t); - if (__mod) [[unlikely]] - { - struct tm __tm{}; - __tm.tm_year = (int)__y - 1900; - return _M_locale_fmt(std::move(__out), _M_locale(__ctx), __tm, - __conv, __mod); - } + 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); + } basic_string<_CharT> __s; int __yi = (int)__y; @@ -985,13 +986,14 @@ namespace __format chrono::day __d = _S_day(__t); unsigned __i = (unsigned)__d; - if (__mod) [[unlikely]] - { - struct tm __tm{}; - __tm.tm_mday = __i; - return _M_locale_fmt(std::move(__out), _M_locale(__ctx), __tm, - (char)__conv, 'O'); - } + 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'); + } auto __sv = _S_two_digits(__i); _CharT __buf[2]; @@ -1051,13 +1053,14 @@ namespace __format const auto __hms = _S_hms(__t); int __i = __hms.hours().count(); - if (__mod) [[unlikely]] - { - struct tm __tm{}; - __tm.tm_hour = __i; - return _M_locale_fmt(std::move(__out), _M_locale(__ctx), __tm, - (char)__conv, 'O'); - } + 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')) { @@ -1109,13 +1112,14 @@ namespace __format auto __m = _S_month(__t); auto __i = (unsigned)__m; - if (__mod) [[unlikely]] // %Om - { - struct tm __tm{}; - __tm.tm_mon = __i - 1; - return _M_locale_fmt(std::move(__out), _M_locale(__ctx), __tm, - 'm', 'O'); - } + 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'); + } return __format::__write(std::move(__out), _S_two_digits(__i)); } @@ -1131,13 +1135,14 @@ namespace __format auto __m = _S_hms(__t).minutes(); auto __i = __m.count(); - if (__mod) [[unlikely]] // %OM - { - struct tm __tm{}; - __tm.tm_min = __i; - return _M_locale_fmt(std::move(__out), _M_locale(__ctx), __tm, - 'M', 'O'); - } + 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)); } @@ -1226,22 +1231,30 @@ namespace __format // %S Seconds as a decimal number. // %OS The locale's alternative representation. auto __hms = _S_hms(__t); + auto __s = __hms.seconds(); if (__mod) [[unlikely]] // %OS { - struct tm __tm{}; - __tm.tm_sec = (int)__hms.seconds().count(); - return _M_locale_fmt(std::move(__out), _M_locale(__ctx), __tm, - 'S', 'O'); + if (_M_spec._M_localized) + if (auto __loc = __ctx.locale(); __loc != locale::classic()) + { + struct tm __tm{}; + __tm.tm_sec = (int)__s.count(); + return _M_locale_fmt(std::move(__out), __loc, __tm, + 'S', 'O'); + } + + // %OS formats don't include subseconds, so just format that: + return __format::__write(std::move(__out), + _S_two_digits(__s.count())); } if constexpr (__hms.fractional_width == 0) __out = __format::__write(std::move(__out), - _S_two_digits(__hms.seconds().count())); + _S_two_digits(__s.count())); else { locale __loc = _M_locale(__ctx); - auto __s = __hms.seconds(); auto __ss = __hms.subseconds(); using rep = typename decltype(__ss)::rep; if constexpr (is_floating_point_v) @@ -1291,13 +1304,14 @@ namespace __format chrono::weekday __wd = _S_weekday(__t); - if (__mod) [[unlikely]] - { - struct tm __tm{}; - __tm.tm_wday = __wd.c_encoding(); - return _M_locale_fmt(std::move(__out), _M_locale(__ctx), __tm, - (char)__conv, 'O'); - } + if (__mod && _M_spec._M_localized) [[unlikely]] + if (auto __loc = __ctx.locale(); __loc != locale::classic()) + { + struct tm __tm{}; + __tm.tm_wday = __wd.c_encoding(); + return _M_locale_fmt(std::move(__out), __loc, __tm, + (char)__conv, 'O'); + } unsigned __wdi = __conv == 'u' ? __wd.iso_encoding() : __wd.c_encoding(); @@ -1320,17 +1334,18 @@ namespace __format auto __d = _S_days(__t); using _TDays = decltype(__d); // Either sys_days or local_days. - if (__mod) [[unlikely]] - { - const year_month_day __ymd(__d); - const year __y = __ymd.year(); - struct tm __tm{}; - __tm.tm_year = (int)__y - 1900; - __tm.tm_yday = (__d - _TDays(__y/January/1)).count(); - __tm.tm_wday = weekday(__d).c_encoding(); - return _M_locale_fmt(std::move(__out), _M_locale(__ctx), __tm, - (char)__conv, 'O'); - } + if (__mod && _M_spec._M_localized) [[unlikely]] + if (auto __loc = __ctx.locale(); __loc != locale::classic()) + { + const year_month_day __ymd(__d); + const year __y = __ymd.year(); + struct tm __tm{}; + __tm.tm_year = (int)__y - 1900; + __tm.tm_yday = (__d - _TDays(__y/January/1)).count(); + __tm.tm_wday = weekday(__d).c_encoding(); + return _M_locale_fmt(std::move(__out), __loc, __tm, + (char)__conv, 'O'); + } _TDays __first; // First day of week 1. if (__conv == 'V') // W01 begins on Monday before first Thursday.