On Mon, 7 Jul 2025 at 14:21, Tomasz Kamiński <tkami...@redhat.com> wrote:
>
> From: XU Kailiang <xu2k...@outlook.com>
>
> C++ formatting locale could have a custom time_put that performs
> differently from the C locale, so do not use __timepunct directly,
> instead all of above specifiers use _M_locale_fmt.
>
> For %a/%A/%b/%h/%B, the code handling the exception is now moved
> to the _M_check_ok function, that is invoked before handling of the
> conversion specifier. For time_points the values of months/weekday
> are computed, and thus are always ok(), this information is indicated
> by new _M_time_point member of the _ChronoSpec.
>
> The different behavior of j specifier for durations and time_points/calendar
> types, is now handled using only _ChronoParts, and _M_time_only in _ChronoSpec
> is no longer needed, thus it was removed.
>
> PR libstdc++/117214
>
> libstdc++-v3/ChangeLog:
>
> * include/bits/chrono_io.h (_ChronoSpec::_M_time_only): Remove.
> (_ChronoSpec::_M_time_point): Define.
> (__formatter_chrono::_M_parse): Use __parts to determine
> interpretation of j.
> (__formatter_chrono::_M_check_ok): Define.
> (__formatter_chrono::_M_format_to): Invoke _M_check_ok.
> (__formatter_chrono::_M_a_A, __formatter_chrono::_M_b_B): Move
> exception throwing to _M_check_ok.
> (__formatter_chrono::_M_j): Use _M_needs to define interpretation.
> (__formatter_duration::_S_spec_for): Set _M_time_point.
> * testsuite/std/time/format/format.cc: Test for exception for !ok()
> months/weekday.
> * testsuite/std/time/format/pr117214_custom_timeput.cc: New
> test.
>
> Co-authored-by: Tomasz Kaminski <tkami...@redhat.com>
> Reviewed-by: Jonathan Wakely <jwak...@redhat.com>
> Signed-off-by: XU Kailiang <xu2k...@outlook.com>
> Signed-off-by: Tomasz Kaminski <tkami...@redhat.com>
> ---
> Posting v3 as I have realized, there is no test checking if we throw
> exception for above spec and !ok() values, so added one.
>
> Changes in v3:
> - adds test for exception being thrown for !ok() months and weekdays
> - fixes typos in commit description
> - changes comment for 'j'.
>
> Tested on x86_64-linux locally, cfarm is not reachable again.
It's still just the VMs hosted in Japan that are unreachable. Other
x86 hosts like cfarm186 - 188 are reachable.
> Additionally tested `std/time/format/*` with
> -target_board=unix\{,-D_GLIBCXX_USE_CXX11_ABI=0/-D_GLIBCXX_DEBUG,-D_GLIBCXX_DEBUG\}.
> OK for trunk? (mostly additional tests).
OK
>
>
> libstdc++-v3/include/bits/chrono_io.h | 61 ++++++++++++++-----
> .../testsuite/std/time/format/format.cc | 7 +++
> .../time/format/pr117214_custom_timeput.cc | 37 +++++++++++
> 3 files changed, 90 insertions(+), 15 deletions(-)
> create mode 100644
> libstdc++-v3/testsuite/std/time/format/pr117214_custom_timeput.cc
>
> diff --git a/libstdc++-v3/include/bits/chrono_io.h
> b/libstdc++-v3/include/bits/chrono_io.h
> index 72cd569ccd6..75ee7e818b2 100644
> --- a/libstdc++-v3/include/bits/chrono_io.h
> +++ b/libstdc++-v3/include/bits/chrono_io.h
> @@ -280,8 +280,8 @@ namespace __format
> // in the format-spec, e.g. "{:L%a}" is localized and locale-specific,
> // but "{:L}" is only localized and "{:%a}" is only locale-specific.
> unsigned _M_locale_specific : 1;
> - // Indicates that we are handling duration.
> - unsigned _M_time_only : 1;
> + // Indicates that we are handling time_point.
> + unsigned _M_time_point : 1;
> // Indicates that duration should be treated as floating point.
> unsigned _M_floating_point_rep : 1;
> // Indicate that duration uses user-defined representation.
> @@ -693,8 +693,11 @@ namespace __format
> __allowed_mods = _Mod_O;
> break;
> case 'j':
> - __needed = __spec._M_time_only ? _HoursMinutesSeconds
> - : _DayOfYear;
> + __needed = __parts & _DayOfYear;
> + // If we do not know day-of-year then we must have a
> duration,
> + // which is to be formatted as decimal number of days.
> + if (__needed == _None)
> + __needed = _HoursMinutesSeconds;
> break;
> case 'm':
> __needed = _Month;
> @@ -919,7 +922,13 @@ namespace __format
> {
> switch (__conv)
> {
> + case 'a':
> + case 'A':
> + case 'b':
> + case 'B':
> case 'c':
> + case 'h':
> + case 'p':
> case 'r':
> case 'x':
> case 'X':
> @@ -947,6 +956,32 @@ namespace __format
> return __out;
> }
>
> + void
> + _M_check_ok(const _ChronoData<_CharT>& __t, _CharT __conv) const
> + {
> + // n.b. for time point all date parts are computed, so
> + // they are always ok.
> + if (_M_spec._M_time_point || _M_spec._M_debug)
> + return;
> +
> + switch (__conv)
> + {
> + case 'a':
> + case 'A':
> + if (!__t._M_weekday.ok()) [[unlikely]]
> + __throw_format_error("format error: invalid weekday");
> + return;
> + case 'b':
> + case 'h':
> + case 'B':
> + if (!__t._M_month.ok()) [[unlikely]]
> + __throw_format_error("format error: invalid month");
> + return;
> + default:
> + return;
> + }
> + }
> +
> template<typename _OutIter, typename _FormatContext>
> _OutIter
> _M_format_to(const _ChronoData<_CharT>& __t, _OutIter __out,
> @@ -1003,6 +1038,8 @@ namespace __format
> do
> {
> _CharT __c = *__first++;
> + _M_check_ok(__t, __c);
> +
> if (__use_locale_fmt && _S_localized_spec(__c, __mod))
> [[unlikely]]
> __out = _M_locale_fmt(std::move(__out), __fc.locale(),
> __tm, __c, __mod);
> @@ -1153,11 +1190,8 @@ namespace __format
> {
> // %a Locale's abbreviated weekday name.
> // %A Locale's full weekday name.
> - if (!__wd.ok())
> + if (_M_spec._M_debug && !__wd.ok())
> {
> - if (!_M_spec._M_debug)
> - __throw_format_error("format error: invalid weekday");
> -
> _CharT __buf[3];
> __out = __format::__write(std::move(__out),
> _S_str_d1(__buf, __wd.c_encoding()));
> @@ -1183,11 +1217,8 @@ namespace __format
> {
> // %b Locale's abbreviated month name.
> // %B Locale's full month name.
> - if (!__m.ok())
> + if (_M_spec._M_debug && !__m.ok())
> {
> - if (!_M_spec._M_debug)
> - __throw_format_error("format error: invalid month");
> -
> _CharT __buf[3];
> __out = __format::__write(std::move(__out),
> _S_str_d1(__buf, (unsigned)__m));
> @@ -1419,7 +1450,7 @@ namespace __format
> _OutIter
> _M_j(const _ChronoData<_CharT>& __t, _OutIter __out) const
> {
> - if (_M_spec._M_time_only)
> + if (!_M_spec._M_needs(_ChronoParts::_DayOfYear))
> {
> // Decimal number of days, without padding.
> auto __d = chrono::floor<chrono::days>(__t._M_hours).count();
> @@ -1766,7 +1797,7 @@ namespace __format
> {
> if (__n < 100) [[likely]]
> return _S_two_digits(__n);
> - return _S_str_d3(__buf, __n);
> + return _S_str_d3(__buf, __n);
> }
>
> // Returns decimal representation of __n, padded to 3 digits.
> @@ -1811,7 +1842,7 @@ namespace __format
> using enum _ChronoParts;
>
> _ChronoSpec<_CharT> __res{};
> - __res._M_time_only = (__parts & _Date) == 0;
> + __res._M_time_point = (__parts & _DateTime) == _DateTime;
> __res._M_floating_point_rep =
> chrono::treat_as_floating_point_v<_Rep>;
> __res._M_custom_rep = !is_arithmetic_v<_Rep>;
> __res._M_prec = chrono::hh_mm_ss<_Duration>::fractional_width;
> diff --git a/libstdc++-v3/testsuite/std/time/format/format.cc
> b/libstdc++-v3/testsuite/std/time/format/format.cc
> index d6e35832cb5..00affb9e11a 100644
> --- a/libstdc++-v3/testsuite/std/time/format/format.cc
> +++ b/libstdc++-v3/testsuite/std/time/format/format.cc
> @@ -78,6 +78,13 @@ test_bad_format_strings()
> VERIFY( not is_format_string_for("{:%OOy}", t) );
> VERIFY( not is_format_string_for("{:%OEy}", t) );
> VERIFY( not is_format_string_for("{:%EOy}", t) );
> +
> + // weekday and month values for which ok() is false
> + VERIFY( not is_format_string_for("{:%a}", std::chrono::weekday(8)) );
> + VERIFY( not is_format_string_for("{:%A}", std::chrono::weekday(8)) );
> + VERIFY( not is_format_string_for("{:%b}", std::chrono::month(13)) );
> + VERIFY( not is_format_string_for("{:%h}", std::chrono::month(13)) );
> + VERIFY( not is_format_string_for("{:%B}", std::chrono::month(13)) );
> }
>
> template<typename I>
> diff --git
> a/libstdc++-v3/testsuite/std/time/format/pr117214_custom_timeput.cc
> b/libstdc++-v3/testsuite/std/time/format/pr117214_custom_timeput.cc
> new file mode 100644
> index 00000000000..03b94961096
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/std/time/format/pr117214_custom_timeput.cc
> @@ -0,0 +1,37 @@
> +// { dg-do run { target c++20 } }
> +
> +#include <chrono>
> +#include <format>
> +#include <locale>
> +#include <testsuite_hooks.h>
> +
> +struct custom_time_put : std::time_put<char>
> +{
> + iter_type
> + do_put(iter_type out, std::ios_base& io, char_type fill, const tm* t,
> + char format, char modifier) const override
> + {
> + using Base = std::time_put<char>;
> +
> + switch (format) {
> + case 'a': case 'A': case 'b': case 'h': case 'B': case 'p':
> + *out++ = '[';
> + *out++ = format;
> + *out++ = ']';
> + }
> + return Base::do_put(out, io, fill, t, format, modifier);
> + }
> +};
> +
> +int main()
> +{
> + using namespace std::chrono;
> + std::locale loc(std::locale::classic(), new custom_time_put);
> +#define test(t, fmt, exp) VERIFY( std::format(loc, fmt, t) == exp )
> + test(Monday, "{:L%a}", "[a]Mon");
> + test(Monday, "{:L%A}", "[A]Monday");
> + test(January, "{:L%b}", "[b]Jan");
> + test(January, "{:L%h}", "[h]Jan");
> + test(January, "{:L%B}", "[B]January");
> + test(1h, "{:L%p}", "[p]AM");
> +}
> --
> 2.49.0
>
