On Tue, Oct 6, 2026 at 12:01 PM Jonathan Wakely <[email protected]> wrote:
> This delays storing a result to *abbrev and *offset until we know that
> the entire parse was successful, i.e. after examining the
> _M_is_leap_second flag.
>
> It's not currently clear if this is the correct behaviour, but if not
> then we will need to change _Parser::operator() so that it makes the
> result of %z and %Z available even if the rest of the parse fails.
>
> libstdc++-v3/ChangeLog:
>
> * include/bits/chrono_io.h (from_stream): Do not set results of
> parsing %z and %Z unless the entire parse is successful.
> * testsuite/std/time/parse/114279.cc: Check that offset is
> unchanged when parsing fails.
> ---
> Tested x86_64-linux.
>
> v2: Split out the changes to only set *abbrev and *offset if the entire
> parse succeeds in to this patch, 2/2.
>
> I originally coupled the two changes because I only noticed that *abbrev
> and *offset were being updated on a failed parse when I was implementing
> the additional failure case for utc_time.
>
> This needs clarification from LWG whether it's the intended behaviour.
>
If we go with this direciton, I would do the change suggested below:
pass local variables to _Parser::operator() and always update them, and
then handle conditional copy in form_stream.
>
> libstdc++-v3/include/bits/chrono_io.h | 64 +++++++++++--------
> .../testsuite/std/time/parse/114279.cc | 5 +-
> 2 files changed, 43 insertions(+), 26 deletions(-)
>
> diff --git a/libstdc++-v3/include/bits/chrono_io.h
> b/libstdc++-v3/include/bits/chrono_io.h
> index 5cb9c159e5b5..0d6da0eb1bd7 100644
> --- a/libstdc++-v3/include/bits/chrono_io.h
> +++ b/libstdc++-v3/include/bits/chrono_io.h
> @@ -3911,24 +3911,27 @@ namespace __detail
> basic_istream<_CharT, _Traits>&
> from_stream(basic_istream<_CharT, _Traits>& __is, const _CharT* __fmt,
> sys_time<_Duration>& __tp,
> - basic_string<_CharT, _Traits, _Alloc>* __abbrev = nullptr,
> - minutes* __offset = nullptr)
> + basic_string<_CharT, _Traits, _Alloc>* __abbrev_p =
> nullptr,
> + minutes* __offset_p = nullptr)
> {
> + basic_string<_CharT, _Traits, _Alloc> __abbr;
> minutes __off{};
> - if (!__offset)
> - __offset = &__off;
> using __format::_ChronoParts;
> auto __need = _ChronoParts::_Year | _ChronoParts::_Month
> | _ChronoParts::_Day | _ChronoParts::_TimeOfDay;
> __detail::_Parser_t<_Duration> __p(__need);
> - if (__p(__is, __fmt, __abbrev, __offset))
> + if (__p(__is, __fmt, __abbrev_p ? &__abbr : nullptr, &__off))
>
As we have local variables defined, we could change operator() to accept
references to them, and always update them.
> {
> if (__p._M_is_leap_second)
> __is.setstate(ios_base::failbit);
> else
> {
> - auto __st = __p._M_sys_days + __p._M_time - *__offset;
> + auto __st = __p._M_sys_days + __p._M_time - __off;
> __tp = __detail::__round<_Duration>(__st);
>
And then below, check if they were set, and copy them to input pointers.
> + if (__offset_p)
> + *__offset_p = __off;
> + if (__abbrev_p)
> + *__abbrev_p = std::move(__abbr);
> }
> }
> return __is;
> @@ -3945,27 +3948,32 @@ namespace __detail
> inline basic_istream<_CharT, _Traits>&
> from_stream(basic_istream<_CharT, _Traits>& __is, const _CharT* __fmt,
> utc_time<_Duration>& __tp,
> - basic_string<_CharT, _Traits, _Alloc>* __abbrev = nullptr,
> - minutes* __offset = nullptr)
> + basic_string<_CharT, _Traits, _Alloc>* __abbrev_p =
> nullptr,
> + minutes* __offset_p = nullptr)
> {
> + basic_string<_CharT, _Traits, _Alloc> __abbr;
> minutes __off{};
> - if (!__offset)
> - __offset = &__off;
> using __format::_ChronoParts;
> auto __need = _ChronoParts::_Year | _ChronoParts::_Month
> | _ChronoParts::_Day | _ChronoParts::_TimeOfDay;
> __detail::_Parser_t<_Duration> __p(__need);
> - if (__p(__is, __fmt, __abbrev, __offset))
> + if (__p(__is, __fmt, __abbrev_p ? &__abbr : nullptr, &__off))
> {
> // Converting to utc_time before adding _M_time is necessary for
> // "23:59:60" to correctly produce a time within a leap second.
> auto __ut = utc_clock::from_sys(__p._M_sys_days) + __p._M_time
> - - *__offset;
> + - __off;
> if (__p._M_is_leap_second
> && !chrono::get_leap_second_info(__ut).is_leap_second)
> __is.setstate(ios_base::failbit);
> else
> - __tp = __detail::__round<_Duration>(__ut);
> + {
> + __tp = __detail::__round<_Duration>(__ut);
> + if (__offset_p)
> + *__offset_p = __off;
> + if (__abbrev_p)
> + *__abbrev_p = std::move(__abbr);
> + }
> }
> return __is;
> }
> @@ -3981,26 +3989,29 @@ namespace __detail
> inline basic_istream<_CharT, _Traits>&
> from_stream(basic_istream<_CharT, _Traits>& __is, const _CharT* __fmt,
> tai_time<_Duration>& __tp,
> - basic_string<_CharT, _Traits, _Alloc>* __abbrev = nullptr,
> - minutes* __offset = nullptr)
> + basic_string<_CharT, _Traits, _Alloc>* __abbrev_p =
> nullptr,
> + minutes* __offset_p = nullptr)
> {
> + basic_string<_CharT, _Traits, _Alloc> __abbr;
> minutes __off{};
> - if (!__offset)
> - __offset = &__off;
> using __format::_ChronoParts;
> auto __need = _ChronoParts::_Year | _ChronoParts::_Month
> | _ChronoParts::_Day | _ChronoParts::_TimeOfDay;
> __detail::_Parser_t<_Duration> __p(__need);
> - if (__p(__is, __fmt, __abbrev, __offset))
> + if (__p(__is, __fmt, __abbrev_p ? &__abbr : nullptr, &__off))
> {
> if (__p._M_is_leap_second)
> __is.setstate(ios_base::failbit);
> else
> {
> constexpr sys_days __epoch(-days(4383)); // 1958y/1/1
> - auto __d = __p._M_sys_days - __epoch + __p._M_time -
> *__offset;
> + auto __d = __p._M_sys_days - __epoch + __p._M_time - __off;
> tai_time<common_type_t<_Duration, seconds>> __tt(__d);
> __tp = __detail::__round<_Duration>(__tt);
> + if (__offset_p)
> + *__offset_p = __off;
> + if (__abbrev_p)
> + *__abbrev_p = std::move(__abbr);
> }
> }
> return __is;
> @@ -4017,25 +4028,28 @@ namespace __detail
> inline basic_istream<_CharT, _Traits>&
> from_stream(basic_istream<_CharT, _Traits>& __is, const _CharT* __fmt,
> gps_time<_Duration>& __tp,
> - basic_string<_CharT, _Traits, _Alloc>* __abbrev = nullptr,
> - minutes* __offset = nullptr)
> + basic_string<_CharT, _Traits, _Alloc>* __abbrev_p =
> nullptr,
> + minutes* __offset_p = nullptr)
> {
> + basic_string<_CharT, _Traits, _Alloc> __abbr;
> minutes __off{};
> - if (!__offset)
> - __offset = &__off;
> using __format::_ChronoParts;
> auto __need = _ChronoParts::_YearMonthDay |
> _ChronoParts::_TimeOfDay;
> __detail::_Parser_t<_Duration> __p(__need);
> - if (__p(__is, __fmt, __abbrev, __offset))
> + if (__p(__is, __fmt, __abbrev_p ? &__abbr : nullptr, &__off))
> {
> if (__p._M_is_leap_second)
> __is.setstate(ios_base::failbit);
> else
> {
> constexpr sys_days __epoch(days(3657)); // 1980y/1/Sunday[1]
> - auto __d = __p._M_sys_days - __epoch + __p._M_time -
> *__offset;
> + auto __d = __p._M_sys_days - __epoch + __p._M_time - __off;
> gps_time<common_type_t<_Duration, seconds>> __gt(__d);
> __tp = __detail::__round<_Duration>(__gt);
> + if (__offset_p)
> + *__offset_p = __off;
> + if (__abbrev_p)
> + *__abbrev_p = std::move(__abbr);
> }
> }
> return __is;
> diff --git a/libstdc++-v3/testsuite/std/time/parse/114279.cc
> b/libstdc++-v3/testsuite/std/time/parse/114279.cc
> index ff6f8888d9b8..ff9a1f18112a 100644
> --- a/libstdc++-v3/testsuite/std/time/parse/114279.cc
> +++ b/libstdc++-v3/testsuite/std/time/parse/114279.cc
> @@ -44,7 +44,10 @@ do_test_leap_second_parsing()
> VERIFY( offset == std::chrono::minutes(-60) );
> }
> else
> - VERIFY( !ss );
> + {
> + VERIFY( !ss );
> + VERIFY( offset == std::chrono::minutes(9999) ); // unchanged on
> fail
> + }
> }
> }
>
> --
> 2.55.0
>
>
