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() > > 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. > operator() should also clear them (assing __bad_min to _M_offset) on start, not only when we are just going to parse them. > > 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 >> >>
