On Tue, 6 Oct 2026 at 11:24, Tomasz Kaminski <[email protected]> wrote: > > > > On Tue, Oct 6, 2026 at 12:18 PM Jonathan Wakely <[email protected]> wrote: >> >> On Tue, 6 Oct 2026 at 11:06, Jonathan Wakely <[email protected]> wrote: >> > >> > On Tue, 6 Oct 2026 at 11:01, 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. >> > > >> > > libstdc++-v3/ChangeLog: >> > > >> > > PR libstdc++/126543 >> > > * include/bits/chrono_io.h (from_stream(..., 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. >> > > * testsuite/std/time/parse/126543.cc: New test. >> > > --- >> > > >> > > Tested x86_64-linux. >> > > >> > > v2: Split out the changes to only set *abbrev and *offset if the entire >> > > parse succeeds in to PATCH v2 2/2. So 1/2 only implements the fixes to >> > > properly parse and validate leap seconds. >> > > >> > > libstdc++-v3/include/bits/chrono_io.h | 12 ++++-- >> > > .../testsuite/std/time/parse/114279.cc | 28 +++++++++---- >> > > .../testsuite/std/time/parse/126543.cc | 40 +++++++++++++++++++ >> > > 3 files changed, 70 insertions(+), 10 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..5cb9c159e5b5 100644 >> > > --- a/libstdc++-v3/include/bits/chrono_io.h >> > > +++ b/libstdc++-v3/include/bits/chrono_io.h >> > > @@ -3961,7 +3961,11 @@ namespace __detail >> > > // "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); >> > > + if (__p._M_is_leap_second >> > > + && !chrono::get_leap_second_info(__ut).is_leap_second) >> > >> > We could optimize this slightly by checking if: >> > >> > (__p._M_time - *__offset) == 23h + 59min + 60s >> > >> > and only doing the get_leap_second_info call if that's true. Because >> > it must be true for valid leap seconds, so we avoid the lookup (and >> > potentially slow call into the library to read a new leapseconds file) >> > on input like 01:23:60 which can never be a valid leap second. >> >> i.e. like this >> >> if (__p(__is, __fmt, __abbrev, __offset)) >> { >> // 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; >> if (__p._M_is_leap_second >> && (chrono::floor<seconds>(__p._M_time - *__offset) != >> hours(24) >> || !chrono::get_leap_second_info(__ut).is_leap_second)) >> __is.setstate(ios_base::failbit); >> else >> __tp = __detail::__round<_Duration>(__ut); > > Yes, looks good to me.
OK, I'll add that. I started implementing the `t - offset == hours(24)` check in Parser::operator() but that means doing additional work for types that are not utc_time. So I'll leave the parser being responsible only for setting _M_is_leap_second and the from_stream caller responsible for deciding what to do when _M_is_leap_second is true. > We could in theorry check (__p._M_is_leap_second && > (chrono::floor<seconds>(__p._M_time - *__offset) != hours(24)) > and set failbit, before computing __ut, and then check only > __p._M_is_leap_second before calling > !chrono::get_leap_second_info(__ut).is_leap_second), > but that seems like overkill for invalid input. Yes, hh:mm:60 where hh != 23 || mm != 59 should not be a common failure case, so not important to optimize. If we really wanted to optimize the leap second case (for valid and invalid leap seconds), we would need to combine the get_leap_second_info call done by utc_clock::from_sys and the one to check for a valid leap second, because they both do a lookup in the same list of leap seconds. We could do in initial lookup using a sys_time then see if the next leap second is within __inf.elapsed, and adjust by +1s if it is, and then check if we land exactly on that next leap second. But we would need changes to __detail::__get_leap_second_info (or direct access to the list of leap seconds) to do that. Even the valid leap second case is going to be rare, so I don't think this is important to optimize more than the hours(24) check above. >> >> >> >> >> > Or we could do that inside _Parser::operator() where we set >> > _M_is_leap_second and set failbit. >> > Below there was extra H in #include, that I assume was accidentally adding > when sending the patch. Huh, it's not in my git tree so I must have added it while editing the email. It won't be in the commit that gets pushed! > --- a/libstdc++-v3/testsuite/std/time/parse/114279.cc > +++ b/libstdc++-v3/testsuite/std/time/parse/114279.cc > @@ -4,14 +4,18 @@ > #include <sstream> > #incluHde <testsuite_hooks.h>
