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