On Fri, 13 Jun 2025 at 14:17, Tomasz Kamiński <tkami...@redhat.com> wrote:
>
> Similarly to issue reported for %c in PR117214, the format string for locale
> specific time (%r, %X) and date (%x) representations may contain specifiers
> not accepted by chrono-spec, leading to exception being thrown. This
> happened for following conversion specifier and locale combinations:
> * %r, %X for aa_DJ.UTF-8, ar_SA.UTF-8
> * %x for ca_AD.UTF-8, my_MM.UTF-8
>
> This fix follows approach from r15-8490-gc24a1d5, and uses time_put to emit
> localized date format. The existing _M_c is reworked to handle all locale
> dependent conversion specifies, by accepting them as argument. It is also
> renamed to _M_c_r_x_X.
>
> PR libstdc++/120648
>
> libstdc++-v3/ChangeLog:
>
> * include/bits/chrono_io.h (__formatter_chrono::_M_format_to):
> Handle %c, %r, %x and %X by passing them to _M_c_r_x_X.
> (__formatter_chrono::_M_c_r_x_X): Reworked from _M_c.
> (__formatter_chrono::_M_c): Renamed into above.
> (__formatter_chrono::_M_r, __formatter_chrono::_M_x)
> (__formatter_chrono::_M_X): Removed.
> * testsuite/std/time/format/pr117214.cc: New tests for %r, %x,
> %X with date, time and durations.
> ---
> Testing on x86_64-linux. Alll std/time* tests passed.
> OK for trunk if all tests passes?
Consolidating all these locale-specific formats into one function
makes sense, thanks.
OK for trunk, and we should consider backporting too (but maybe not
immediately).
>
> libstdc++-v3/include/bits/chrono_io.h | 128 +++++-------------
> .../testsuite/std/time/format/pr117214.cc | 43 +++++-
> 2 files changed, 72 insertions(+), 99 deletions(-)
>
> diff --git a/libstdc++-v3/include/bits/chrono_io.h
> b/libstdc++-v3/include/bits/chrono_io.h
> index 81ec4414f48..abbf4efcc3b 100644
> --- a/libstdc++-v3/include/bits/chrono_io.h
> +++ b/libstdc++-v3/include/bits/chrono_io.h
> @@ -688,7 +688,10 @@ namespace __format
> __out = _M_b_B(__t, std::move(__out), __fc, __c == 'B');
> break;
> case 'c':
> - __out = _M_c(__t, std::move(__out), __fc, __mod == 'E');
> + case 'r':
> + case 'x':
> + case 'X':
> + __out = _M_c_r_x_X(__t, std::move(__out), __fc, __c, __mod);
> break;
> case 'C':
> case 'y':
> @@ -739,9 +742,6 @@ namespace __format
> __throw_format_error("chrono format error: argument is "
> "not a duration");
> break;
> - case 'r':
> - __out = _M_r(__t, __print_sign(), __fc);
> - break;
> case 'R':
> case 'T':
> __out = _M_R_T(__t, __print_sign(), __fc, __c == 'T');
> @@ -759,12 +759,6 @@ namespace __format
> __out = _M_U_V_W(__t, std::move(__out), __fc, __c,
> __mod == 'O');
> break;
> - case 'x':
> - __out = _M_x(__t, std::move(__out), __fc, __mod == 'E');
> - break;
> - case 'X':
> - __out = _M_X(__t, __print_sign(), __fc, __mod == 'E');
> - break;
> case 'z':
> __out = _M_z(__t, std::move(__out), __fc, (bool)__mod);
> break;
> @@ -954,11 +948,16 @@ namespace __format
>
> template<typename _Tp, typename _FormatContext>
> typename _FormatContext::iterator
> - _M_c(const _Tp& __t, typename _FormatContext::iterator __out,
> - _FormatContext& __ctx, bool __mod = false) const
> + _M_c_r_x_X(const _Tp& __t, typename _FormatContext::iterator __out,
> + _FormatContext& __ctx, _CharT __conv, _CharT __mod) 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;
> using ::std::chrono::__detail::__utc_leap_second;
> @@ -992,23 +991,30 @@ namespace __format
> __tm.tm_zone = const_cast<char*>("UTC");
> #endif
>
> - auto __d = _S_days(__t); // Either sys_days or local_days.
> - using _TDays = decltype(__d);
> - const year_month_day __ymd(__d);
> - const auto __y = __ymd.year();
> - const auto __hms = _S_hms(__t);
> + if (__conv == 'c' || __conv == 'x')
> + {
> + auto __d = _S_days(__t); // Either sys_days or local_days.
> + using _TDays = decltype(__d);
> + const year_month_day __ymd(__d);
> + const auto __y = __ymd.year();
> +
> + __tm.tm_year = (int)__y - 1900;
> + __tm.tm_yday = (__d - _TDays(__y/January/1)).count();
> + __tm.tm_mon = (unsigned)__ymd.month() - 1;
> + __tm.tm_mday = (unsigned)__ymd.day();
> + __tm.tm_wday = weekday(__d).c_encoding();
> + }
> +
> + if (__conv != 'x')
> + {
> + const auto __hms = _S_hms(__t);
> + __tm.tm_hour = __hms.hours().count();
> + __tm.tm_min = __hms.minutes().count();
> + __tm.tm_sec = __hms.seconds().count();
> + }
>
> - __tm.tm_year = (int)__y - 1900;
> - __tm.tm_yday = (__d - _TDays(__y/January/1)).count();
> - __tm.tm_mon = (unsigned)__ymd.month() - 1;
> - __tm.tm_mday = (unsigned)__ymd.day();
> - __tm.tm_wday = weekday(__d).c_encoding();
> - __tm.tm_hour = __hms.hours().count();
> - __tm.tm_min = __hms.minutes().count();
> - __tm.tm_sec = __hms.seconds().count();
> -
> - return _M_locale_fmt(std::move(__out), _M_locale(__ctx), __tm, 'c',
> - __mod ? 'E' : '\0');
> + return _M_locale_fmt(std::move(__out), _M_locale(__ctx), __tm,
> + __conv, __mod);
> }
>
> template<typename _Tp, typename _FormatContext>
> @@ -1347,25 +1353,6 @@ namespace __format
>
> // %Q handled in _M_format
>
> - template<typename _Tp, typename _FormatContext>
> - typename _FormatContext::iterator
> - _M_r(const _Tp& __tt, typename _FormatContext::iterator __out,
> - _FormatContext& __ctx) const
> - {
> - // %r locale's 12-hour clock time.
> - auto __t = _S_floor_seconds(__tt);
> - locale __loc = _M_locale(__ctx);
> - const auto& __tp = use_facet<__timepunct<_CharT>>(__loc);
> - const _CharT* __ampm_fmt;
> - __tp._M_am_pm_format(&__ampm_fmt);
> - basic_string<_CharT> __fmt(_S_empty_spec);
> - __fmt.insert(1u, 1u, _S_colon);
> - __fmt.insert(2u, __ampm_fmt);
> - using _FmtStr = _Runtime_format_string<_CharT>;
> - return _M_write(std::move(__out), __loc,
> - std::format(__loc, _FmtStr(__fmt), __t));
> - }
> -
> template<typename _Tp, typename _FormatContext>
> typename _FormatContext::iterator
> _M_R_T(const _Tp& __t, typename _FormatContext::iterator __out,
> @@ -1545,53 +1532,6 @@ namespace __format
> return __format::__write(std::move(__out), __sv);
> }
>
> - template<typename _Tp, typename _FormatContext>
> - typename _FormatContext::iterator
> - _M_x(const _Tp& __t, typename _FormatContext::iterator __out,
> - _FormatContext& __ctx, bool __mod = false) const
> - {
> - // %x Locale's date rep
> - // %Ex Locale's alternative date representation.
> - locale __loc = _M_locale(__ctx);
> - const auto& __tp = use_facet<__timepunct<_CharT>>(__loc);
> - const _CharT* __date_reps[2];
> - __tp._M_date_formats(__date_reps);
> - const _CharT* __rep = __date_reps[__mod];
> - if (!*__rep)
> - return _M_D(__t, std::move(__out), __ctx);
> -
> - basic_string<_CharT> __fmt(_S_empty_spec);
> - __fmt.insert(1u, 1u, _S_colon);
> - __fmt.insert(2u, __rep);
> - using _FmtStr = _Runtime_format_string<_CharT>;
> - return _M_write(std::move(__out), __loc,
> - std::format(__loc, _FmtStr(__fmt), __t));
> - }
> -
> - template<typename _Tp, typename _FormatContext>
> - typename _FormatContext::iterator
> - _M_X(const _Tp& __tt, typename _FormatContext::iterator __out,
> - _FormatContext& __ctx, bool __mod = false) const
> - {
> - // %X Locale's time rep
> - // %EX Locale's alternative time representation.
> - auto __t = _S_floor_seconds(__tt);
> - locale __loc = _M_locale(__ctx);
> - const auto& __tp = use_facet<__timepunct<_CharT>>(__loc);
> - const _CharT* __time_reps[2];
> - __tp._M_time_formats(__time_reps);
> - const _CharT* __rep = __time_reps[__mod];
> - if (!*__rep)
> - return _M_R_T(__t, std::move(__out), __ctx, true);
> -
> - basic_string<_CharT> __fmt(_S_empty_spec);
> - __fmt.insert(1u, 1u, _S_colon);
> - __fmt.insert(2u, __rep);
> - using _FmtStr = _Runtime_format_string<_CharT>;
> - return _M_write(std::move(__out), __loc,
> - std::format(__loc, _FmtStr(__fmt), __t));
> - }
> -
> template<typename _Tp, typename _FormatContext>
> typename _FormatContext::iterator
> _M_z(const _Tp& __t, typename _FormatContext::iterator __out,
> diff --git a/libstdc++-v3/testsuite/std/time/format/pr117214.cc
> b/libstdc++-v3/testsuite/std/time/format/pr117214.cc
> index 87c703dd255..79109c39151 100644
> --- a/libstdc++-v3/testsuite/std/time/format/pr117214.cc
> +++ b/libstdc++-v3/testsuite/std/time/format/pr117214.cc
> @@ -7,10 +7,14 @@
>
> #include <chrono>
> #include <locale>
> +#include <span>
> #include <testsuite_hooks.h>
>
> +
> +template<typename ChronoType>
> void
> -test_c()
> +test_locale_formats(const ChronoType& t,
> + std::span<const char* const> test_specifiers)
> {
> const char *test_locales[] = {
> "aa_DJ.UTF-8",
> @@ -19,15 +23,44 @@ test_c()
> "az_IR.UTF-8",
> "my_MM.UTF-8",
> };
> - std::chrono::sys_seconds t{std::chrono::seconds{1}};
>
> + auto format_args = std::make_format_args(t);
> for (auto locale_name : test_locales)
> {
> - auto s = std::format(std::locale(locale_name), "{:L%c}", t);
> - VERIFY( !s.empty() );
> + std::locale loc(locale_name);
> + for (auto specifier : test_specifiers)
> + {
> + auto s = std::vformat(loc, specifier, format_args);
> + VERIFY( !s.empty() );
> + }
> }
> }
>
> +void
> +test_locale_formats()
> +{
> + using namespace std::chrono;
> +
> + const char* test_specifiers[] = {
> + "{:L%x}", "{:L%Ex}",
> + "{:L%c}", "{:L%Ec}",
> + "{:L%X}", "{:L%EX}",
> + "{:L%r}",
> + };
> + auto date_time_specifiers = std::span(test_specifiers);
> + auto date_specifiers = date_time_specifiers.subspan(0, 2);
> + auto time_specifiers = date_time_specifiers.subspan(4);
> +
> + auto ymd = 2020y/November/12d;
> + test_locale_formats(ymd, date_specifiers);
> +
> + auto tod = 25h + 10min + 12s;
> + test_locale_formats(tod, time_specifiers);
> +
> + auto tp = sys_days(ymd) + tod;
> + test_locale_formats(tp, date_time_specifiers);
> +}
> +
> #include <stdlib.h>
> #include <time.h>
>
> @@ -93,7 +126,7 @@ test_c_local()
>
> int main()
> {
> - test_c();
> + test_locale_formats();
> test_c_zoned();
> test_c_local();
> }
> --
> 2.49.0
>
