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. 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. > > > > > 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. --- 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>
