On Tue, 6 Oct 2026, 08:23 Tomasz Kaminski, <[email protected]> wrote:
> > > On Tue, Oct 6, 2026 at 8:54 AM Tomasz Kaminski <[email protected]> > wrote: > >> >> >> On Mon, Oct 5, 2026 at 11:02 PM Jonathan Wakely <[email protected]> >> wrote: >> >>> Formatting utc_time can produce a time ending with ":60" for a leap >>> second, so we should also be able to read that back using chrono::parse. >>> >>> We should reject ":60" unless the time really is a leap second, so >>> compare to the list of known leap seconds. Continue to reject ":60" for >>> any time point except utc_time. >>> >>> We need to delay writing to the *abbrev and *offset variables until >>> after we know if the parse is successful, so after handling the >>> _M_is_leap_second flag. >>> >>> libstdc++-v3/ChangeLog: >>> >>> PR libstdc++/126543 >>> * include/bits/chrono_io.h (from_stream): Delay writing abbrev >>> and offset until successful or failed parse has been decided. >>> For utc_time use chrono::get_leap_second_info to check for valid >>> leap seconds. >>> (_Parser::operator()) <R>: Do not accept "60" for minutes. >>> <S>: Accept "60" for seconds. >>> * testsuite/std/time/parse/114279.cc: Use a real leap second as >>> the test input. Check parsing whole seconds as well as >>> fractional seconds. Check that offset isn't written to for >>> failed parse. >>> * testsuite/std/time/parse/126543.cc: New test. >>> --- >>> >>> Tested x86_64-linux. >>> >>> libstdc++-v3/include/bits/chrono_io.h | 74 ++++++++++++------- >>> .../testsuite/std/time/parse/114279.cc | 33 +++++++-- >>> .../testsuite/std/time/parse/126543.cc | 40 ++++++++++ >>> 3 files changed, 112 insertions(+), 35 deletions(-) >>> create mode 100644 libstdc++-v3/testsuite/std/time/parse/126543.cc >>> >>> diff --git a/libstdc++-v3/include/bits/chrono_io.h >>> b/libstdc++-v3/include/bits/chrono_io.h >>> index eec12357fdf7..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; >>> >> I do not like how we are tripple buffering the __abber now: >> * one string provided by the user provided by >> * one created here >> * one created in operator() >> > After re-reading the standard, I do not think it requires that: > > We need to delay writing to the *abbrev and *offset variables until > after we know if the parse succeeded > > The pagraph https://eel.is/c++draft/time.clock.system#nonmembers-6 says: > > If %Z is used and successfully parsed, that value will be assigned to * > abbrev if abbrev is non-null. > <https://eel.is/c++draft/time.clock.system#nonmembers-6.sentence-3> > Which for me reads us, if %Z was successfully parsed, the abbrev is > changed, not if the whole thing > was successfully parsed. > I don't agree with that interpretation. https://eel.is/c++draft/time.parse#17.sentence-1 says to me that parsing all succeeds or all fails, there is no partial success for one part and failure for another. I agree we should get that clarified. > I think this distinction is important, as it could potentially allow no > allocations here > std::string abbrev; > while (in) > from_stream("%T %Z", time, &abbrev); > The second and later iterations can use the memory allocated for abbrev. > This is meaningful if we > use named zones, like Europe/Warsaw, and allows preallocation before. > > But, even my interpretation of %Z seem insufficient, as we cannot parse > directly to *abbrev, because > if the stream fails when reading additional characters, we cannot roll > that back. I think we should clarify > that wording, and either make *abbrev unspecified in case if parsing of %Z > fails, all change the wording > to make the changes atomic (either all are modified or none), as we > require buffering anyway. In this > case I would use my _M_fill_tz suggestion. > > In light of above could you change the patch to just > add __p._M_is_leap_second checks, without changes > to handling of abbrev/offset. > > My suggestion in the _Parser, add following members and remove them >> from operator(): >> minutes __M_offset = __bad_min; >> basic_string<_CharT, _Traits> __M_abbr; >> > >> Remove the __abbrev and __offset parameter from the operator() and >> instead simply set __tz_offset and __tz_abbr always. >> >> And finally add _M_fill_tz(basic_string<_CharT, _Traits, _Alloc>* >> __abbrev, >> basic_string<_CharT, _Traits, >> _Alloc>* __offset): >> to the Parser, that will doo if (abbrev && _M_abbrev.empty()) abbrev = >> std::move(_M_abbev). >> And call it only when we are successful. >> >> minutes __off{}; >>> - if (!__offset) >>> - __offset = &__off; >>> >> So all of the above goes away. >> >>> 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 >>> { >>> - 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 we call __p._M_fill_tz(__offset, __abbrev) here. >> >>> + if (__offset_p) >>> + *__offset_p = __off; >>> + if (__abbrev_p) >>> + *__abbrev_p = std::move(__abbr); >>> } >>> } >>> return __is; >>> @@ -3945,23 +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; >>> - __tp = __detail::__round<_Duration>(__ut); >>> + - __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); >>> + if (__offset_p) >>> + *__offset_p = __off; >>> + if (__abbrev_p) >>> + *__abbrev_p = std::move(__abbr); >>> + } >>> } >>> return __is; >>> } >>> @@ -3977,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; >>> @@ -4013,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; >>> @@ -4769,7 +4787,7 @@ namespace __detail >>> __h = hours(__val); >>> >>> __val = __read_unsigned(2); >>> - if (__val == -1 || __val > 60) [[unlikely]] >>> + if (__val == -1 || __val > 59) [[unlikely]] >>> if ((_M_need & _ChronoParts::_TimeOfDay) != 0) >>> { >>> __err |= ios_base::failbit; >>> @@ -4802,10 +4820,12 @@ namespace __detail >>> && !__is_floating) >>> { >>> auto __val = __read_unsigned(__num ? __num : 2); >>> - if (0 <= __val && __val <= 59) [[likely]] >>> + if (0 <= __val && __val <= 60) [[likely]] >>> __s = seconds(__val); >>> else >>> { >>> + // Not a valid second, but we only fail if >>> we're >>> + // parsing into a type that cares about that. >>> if ((_M_need & _ChronoParts::_TimeOfDay) != 0) >>> __err |= ios_base::failbit; >>> break; >>> diff --git a/libstdc++-v3/testsuite/std/time/parse/114279.cc >>> b/libstdc++-v3/testsuite/std/time/parse/114279.cc >>> index 67e5cba23bd8..ff9a1f18112a 100644 >>> --- a/libstdc++-v3/testsuite/std/time/parse/114279.cc >>> +++ b/libstdc++-v3/testsuite/std/time/parse/114279.cc >>> @@ -4,14 +4,18 @@ >>> #include <sstream> >>> #include <testsuite_hooks.h> >>> >>> -template<class Clock> >>> +template<class Clock, class Dur> >>> void >>> -test_leap_second_parsing() >>> +do_test_leap_second_parsing() >>> { >>> - std::chrono::time_point<Clock, std::chrono::milliseconds> tp, tp2; >>> + std::chrono::time_point<Clock, Dur> tp, tp2; >>> >>> - std::istringstream ss("20161231-23:59:60.05"); >>> - ss >> std::chrono::parse("%Y%m%d-%T", tp); >>> + std::string subs; >>> + if constexpr (std::ratio_less_v<typename Dur::period, std::ratio<1, >>> 1>>) >>> + subs = ".05"; >>> + >>> + std::istringstream ss("20081231-23:59:60" + subs + " "); >>> + ss >> std::chrono::parse("%Y%m%d-%T ", tp); >>> >>> if constexpr (std::is_same_v<Clock, std::chrono::local_t>) >>> VERIFY( ss ); // We allow parsing "23:59:60" as local_time. >>> @@ -28,20 +32,33 @@ test_leap_second_parsing() >>> else >>> VERIFY( !ss ); // Other clocks do not allow "HH:MM:60" >>> >>> + std::chrono::minutes offset(9999); >>> ss.clear(); >>> - ss.str("20161231-22:59:60.05 -0100"); // Same time at -1h offset. >>> - ss >> std::chrono::parse("%Y%m%d-%T %z", tp2); >>> + ss.str("20081231-22:59:60" + subs + " -0100"); // Same time at -1h >>> offset. >>> + ss >> std::chrono::parse("%Y%m%d-%T %z", tp2, offset); >>> >>> if constexpr (std::is_same_v<Clock, std::chrono::utc_clock>) >>> { >>> VERIFY( ss ); >>> VERIFY( tp2 == tp ); >>> + VERIFY( offset == std::chrono::minutes(-60) ); >>> } >>> else >>> - VERIFY( !ss ); >>> + { >>> + VERIFY( !ss ); >>> + VERIFY( offset == std::chrono::minutes(9999) ); // unchanged on >>> fail >>> + } >>> } >>> } >>> >>> +template<class Clock> >>> +void >>> +test_leap_second_parsing() >>> +{ >>> + do_test_leap_second_parsing<Clock, std::chrono::milliseconds>(); >>> + do_test_leap_second_parsing<Clock, std::chrono::seconds>(); >>> +} >>> + >>> int main() >>> { >>> test_leap_second_parsing<std::chrono::system_clock>(); >>> diff --git a/libstdc++-v3/testsuite/std/time/parse/126543.cc >>> b/libstdc++-v3/testsuite/std/time/parse/126543.cc >>> new file mode 100644 >>> index 000000000000..2d2f633b4b7a >>> --- /dev/null >>> +++ b/libstdc++-v3/testsuite/std/time/parse/126543.cc >>> @@ -0,0 +1,40 @@ >>> +// { dg-do run { target c++20 } } >>> + >>> +// Bug 126543 - chrono::parse should accept "60" seconds for a utc_time >>> +// that is a valid leap second >>> + >>> +#include <chrono> >>> +#include <sstream> >>> +#include <testsuite_hooks.h> >>> + >>> +int main() >>> +{ >>> + std::istringstream in; >>> + std::chrono::utc_seconds ut; >>> + std::chrono::sys_seconds st; >>> + >>> + in.str("2016-12-31 24:59:59"); // invalid hour >>> + in >> parse("%F %T", ut); >>> + VERIFY(in.fail()); >>> + in.clear(); >>> + >>> + in.str("2016-12-31 23:60:59"); // invalid minute >>> + in >> parse("%F %T", ut); >>> + VERIFY(in.fail()); >>> + in.clear(); >>> + >>> + in.str("2016-12-31 23:59:60"); // valid leap second >>> + in >> parse("%F %T", ut); >>> + VERIFY(in.good()); >>> + in.clear(); >>> + >>> + in.str("2016-12-31 23:58:60"); // invalid leap second >>> + in >> parse("%F %T", ut); >>> + VERIFY(in.fail()); >>> + in.clear(); >>> + >>> + in.str("2016-12-31 23:59:60"); >>> + in >> parse("%F %T", st); >>> + VERIFY(in.fail()); // sys_time doesn't use leap seconds >>> + in.clear(); >>> +} >>> -- >>> 2.55.0 >>> >>>
