On Tue, Oct 6, 2026 at 1:09 PM Jonathan Wakely <[email protected]> wrote:
> 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. > Yes, make sense. Other clocks reject any input that set seconds to 60, regardless if that happens to be at the end of the day. If we make _M_is_leap_second true only for offset 23:59:60, we will need to check some other field for sys_clock. We could rename the field to _M_60th_second to cleary convey that if we want, but I am fine either way. > > > 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> > >
