On Fri, 18 Jul 2025 at 11:04, Tomasz Kamiński <tkami...@redhat.com> wrote:
>
> Previously for localized output, if _M_debug option was send, the _M_check_ok
> completed succesfully and _M_locale_fmt was called for months/weekdays that
> are !ok().
>
> This patch lifts debug checks from each conversion function into _M_check_ok,
> that in case of !ok() values return a string_view containing the kind of
> calendar data, to be included after "is not a valid" string. The localized
> output (_M_locale_fmt) is not used if string is non-empty.
> The emitting of this message is now handled in _M_format_to, further reducing
> each specifier function.
Nice, I like how they've got simpler and simpler, with more logic
moved to common functions that are reused for most localized formats
or invalid dates.
>
> To handle weekday (%a,%A) and month (%b,%B), _M_check_ok now accepts a
> mutable reference to conversion specifier, and updates it to corresponding
> numeric value (%w, %m). Extra care needs to be taken to handle a month(0)
> that needs to be printed as single digit in debug format.
>
> Finally, the _M_check_ok is no longer invoked, if _M_needed does not include
> chrono specifiers that are checked..
>
> PR libstdc++/PR121154
>
> libstdc++-v3/ChangeLog:
>
> * include/bits/chrono_io.h (__formatter_chrono::_M_check_ok):
> Check values also for debug mode, and return __string_view.
> (__formatter_chrono::_M_format_to): Handle results of _M_check_ok.
> (__formatter_chrono::_M_wi, __formatter_chrono::_M_a_A)
> (__formatter_chrono::_M_b_B, __formatter_chrono::_M_C_y_Y)
> (__formatter_chrono::_M_d_e, __formatter_chrono::_M_F):
> Removed handling of _M_debug.
> (__formatter_chrono::__M_m): Print zero unpadded in _M_debug mode.
> * testsuite/std/time/month/io.cc: Test for localized !ok() values.
> * testsuite/std/time/weekday/io.cc: Test for localized !ok() values.
> ---
> You should now see, why I wanted operator<< to print month(0) padded to
> 2 digits.
>
> Testing on x86_64-linux. OK for trunk when all tests passes?
OK, thanks.
>
> libstdc++-v3/include/bits/chrono_io.h | 124 +++++++++++-------
> libstdc++-v3/testsuite/std/time/month/io.cc | 7 +
> libstdc++-v3/testsuite/std/time/weekday/io.cc | 2 +
> 3 files changed, 88 insertions(+), 45 deletions(-)
>
> diff --git a/libstdc++-v3/include/bits/chrono_io.h
> b/libstdc++-v3/include/bits/chrono_io.h
> index fe3c9126749..631a373d120 100644
> --- a/libstdc++-v3/include/bits/chrono_io.h
> +++ b/libstdc++-v3/include/bits/chrono_io.h
> @@ -972,30 +972,71 @@ namespace __format
> return __out;
> }
>
> - void
> - _M_check_ok(const _ChronoData<_CharT>& __t, _CharT __conv) const
> + __string_view
> + _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;
> + if (!_M_spec._M_debug)
> + {
> + switch (__conv)
> + {
> + case 'a':
> + case 'A':
> + if (!__t._M_weekday.ok()) [[unlikely]]
> + __throw_format_error("format error: invalid weekday");
> + break;
> + case 'b':
> + case 'h':
> + case 'B':
> + if (!__t._M_month.ok()) [[unlikely]]
> + __throw_format_error("format error: invalid month");
> + break;
> + default:
> + break;
> + }
> + return __string_view();
> + }
>
> switch (__conv)
> {
> + // %\0 is extension for handling weekday index
> + case '\0':
> + if (__t._M_weekday_index < 1 || __t._M_weekday_index > 5)
> [[unlikely]]
> + return _GLIBCXX_WIDEN("index");
> + break;
> case 'a':
> case 'A':
> if (!__t._M_weekday.ok()) [[unlikely]]
> - __throw_format_error("format error: invalid weekday");
> - return;
> + {
> + __conv = 'w'; // print as decimal number
> + return _GLIBCXX_WIDEN("weekday");
> + }
> + break;
> case 'b':
> case 'h':
> case 'B':
> if (!__t._M_month.ok()) [[unlikely]]
> - __throw_format_error("format error: invalid month");
> - return;
> + {
> + __conv = 'm'; // print as decimal number
> + return _GLIBCXX_WIDEN("month");
> + }
> + break;
> + case 'd':
> + case 'e':
> + if (!__t._M_day.ok()) [[unlikely]]
> + return _GLIBCXX_WIDEN("day");
> + break;
> + case 'F':
> + if (!(__t._M_year/__t._M_month/__t._M_day).ok()) [[unlikely]]
> + return _GLIBCXX_WIDEN("date");
> + break;
> + case 'Y':
> + if (!__t._M_year.ok()) [[unlikely]]
> + return _GLIBCXX_WIDEN("year");
> + break;
> default:
> - return;
> + break;
> }
> + return __string_view();
> }
>
> template<typename _OutIter, typename _FormatContext>
> @@ -1045,6 +1086,17 @@ namespace __format
> #endif
> }
>
> + bool __needs_check_ok = false;
> + // n.b. for time point all date parts are computed, they are always
> ok.
> + if (!_M_spec._M_time_point)
> + {
> + using enum _ChronoParts;
> + if (_M_spec._M_debug)
> + __needs_check_ok =
> _M_spec._M_needs(_YearMonthDay|_IndexedWeekday);
> + else
> + __needs_check_ok = _M_spec._M_needs(_Month|_Weekday);
> + }
> +
> // Characters to output for "%n", "%t" and "%%" specifiers.
> constexpr const _CharT* __literals = _GLIBCXX_WIDEN("\n\t%");
>
> @@ -1054,9 +1106,12 @@ namespace __format
> do
> {
> _CharT __c = *__first++;
> - _M_check_ok(__t, __c);
> + __string_view __invalid;
> + if (__needs_check_ok)
> + __invalid = _M_check_ok(__t, __c);
>
> - if (__use_locale_fmt && _S_localized_spec(__c, __mod))
> [[unlikely]]
> + if (__invalid.empty() &&__use_locale_fmt
> + && _S_localized_spec(__c, __mod)) [[unlikely]]
> __out = _M_locale_fmt(std::move(__out), __fc.locale(),
> __tm, __c, __mod);
> else switch (__c)
> @@ -1164,6 +1219,14 @@ namespace __format
> __first = __last;
> break;
> }
> +
> + if (!__invalid.empty())
> + {
> + constexpr __string_view __pref = _GLIBCXX_WIDEN(" is not a
> valid ");
> + __out = __format::__write(std::move(__out), __pref);
> + __out = __format::__write(std::move(__out), __invalid);
> + }
> +
> __mod = _CharT();
> // Scan for next '%' and write out everything before it.
> __string_view __str(__first, __last - __first);
> @@ -1193,9 +1256,6 @@ namespace __format
> // %\0 Extension to format weekday index, used only by empty format
> spec
> _CharT __buf[3];
> __out = __format::__write(std::move(__out), _S_str_d1(__buf, __wi));
> - if (_M_spec._M_debug && (__wi < 1 || __wi > 5))
> - __out = __format::__write(std::move(__out),
> - __string_view(_GLIBCXX_WIDEN(" is not a valid index")));
> return std::move(__out);
> }
>
> @@ -1205,15 +1265,6 @@ namespace __format
> {
> // %a Locale's abbreviated weekday name.
> // %A Locale's full weekday name.
> - if (_M_spec._M_debug && !__wd.ok())
> - {
> - _CharT __buf[3];
> - __out = __format::__write(std::move(__out),
> - _S_str_d1(__buf, __wd.c_encoding()));
> - return __format::__write(std::move(__out),
> - __string_view(_GLIBCXX_WIDEN(" is not a valid
> weekday")));
> - }
> -
> __string_view __str = _S_weekdays[__wd.c_encoding()];
> if (!__full)
> __str = __str.substr(0, 3);
> @@ -1226,15 +1277,6 @@ namespace __format
> {
> // %b Locale's abbreviated month name.
> // %B Locale's full month name.
> - if (_M_spec._M_debug && !__m.ok())
> - {
> - _CharT __buf[3];
> - __out = __format::__write(std::move(__out),
> - _S_str_d1(__buf, (unsigned)__m));
> - return __format::__write(std::move(__out),
> - __string_view(_GLIBCXX_WIDEN(" is not a valid
> month")));
> - }
> -
> __string_view __str = _S_months[(unsigned)__m - 1];
> if (!__full)
> __str = __str.substr(0, 3);
> @@ -1303,10 +1345,6 @@ namespace __format
> }
> __out = __format::__write(std::move(__out), __sv);
> }
> -
> - if (_M_spec._M_debug && __conv == 'Y' && !__y.ok()) [[unlikely]]
> - __out = __format::__write(std::move(__out),
> - __string_view(_GLIBCXX_WIDEN(" is not a valid year")));
> return __out;
> }
>
> @@ -1365,9 +1403,6 @@ namespace __format
> }
>
> __out = __format::__write(std::move(__out), __sv);
> - if (_M_spec._M_debug && !__d.ok()) [[unlikely]]
> - __out = __format::__write(std::move(__out),
> - __string_view(_GLIBCXX_WIDEN(" is not a valid day")));
> return std::move(__out);
> }
>
> @@ -1404,9 +1439,6 @@ namespace __format
> __out = __format::__write(std::move(__out), __sv);
> }
>
> - if (_M_spec._M_debug && !(__t._M_year/__t._M_month/__t._M_day).ok())
> - __out = __format::__write(std::move(__out),
> - __string_view(_GLIBCXX_WIDEN(" is not a valid date")));
> return std::move(__out);
> }
>
> @@ -1474,8 +1506,10 @@ namespace __format
> {
> // %m month as a decimal number.
> // %Om Locale's alternative representation.
> -
> auto __i = (unsigned)__m;
> + if (__i == 0 && _M_spec._M_debug) [[unlikely]]
> + // 0 should not be padded to two digits
> + return __format::__write(std::move(__out), _S_digit(0));
>
> _CharT __buf[3];
> return __format::__write(std::move(__out), _S_str_d2(__buf, __i));
> diff --git a/libstdc++-v3/testsuite/std/time/month/io.cc
> b/libstdc++-v3/testsuite/std/time/month/io.cc
> index 99ec0737305..edfa196b71c 100644
> --- a/libstdc++-v3/testsuite/std/time/month/io.cc
> +++ b/libstdc++-v3/testsuite/std/time/month/io.cc
> @@ -24,6 +24,9 @@ test_ostream()
> ss.imbue(std::locale(ISO_8859(15,fr_FR)));
> ss << month(1);
> VERIFY( ss.str() == "janv." );
> + ss.str("");
> + ss << month(0) << '|' << month(13);
> + VERIFY( ss.str() == "0 is not a valid month|13 is not a valid month" );
> }
>
> void
> @@ -66,6 +69,10 @@ test_format()
> VERIFY( s == "Jan" );
> s = std::format(loc_fr, "{:L%b}", month(1));
> VERIFY( s == "janv." );
> + s = std::format(loc_fr, "{:L}", month(0));
> + VERIFY( s == "0 is not a valid month" );
> + s = std::format(loc_fr, "{:L}", month(13));
> + VERIFY( s == "13 is not a valid month" );
>
> std::string_view specs = "aAbBcCdDeFgGhHIjmMpqQrRSTuUVwWxXyYzZ";
> std::string_view my_specs = "bBhm";
> diff --git a/libstdc++-v3/testsuite/std/time/weekday/io.cc
> b/libstdc++-v3/testsuite/std/time/weekday/io.cc
> index a56cdaef88c..90a9bcbbb62 100644
> --- a/libstdc++-v3/testsuite/std/time/weekday/io.cc
> +++ b/libstdc++-v3/testsuite/std/time/weekday/io.cc
> @@ -69,6 +69,8 @@ test_format()
> VERIFY( s == "Mon" );
> s = std::format(loc_fr, "{:L%a}", weekday(1));
> VERIFY( s == "lun." );
> + s = std::format(loc_fr, "{:L}", weekday(25));
> + VERIFY( s == "25 is not a valid weekday" );
>
> std::string_view specs = "aAbBcCdDeFgGhHIjmMpqQrRSTuUVwWxXyYzZ";
> std::string_view my_specs = "aAuw";
> --
> 2.49.0
>
