[PATCH] libstdc++: Make chrono::parse support valid leap seconds [PR126543]
Tomasz Kaminski
tkaminsk@redhat.com
Tue Oct 6 08:14:06 GMT 2026
On Tue, Oct 6, 2026 at 10:11 AM Tomasz Kaminski <tkaminsk@redhat.com> wrote:
>
>
> On Tue, Oct 6, 2026 at 10:03 AM Jonathan Wakely <jwakely.gcc@gmail.com>
> wrote:
>
>>
>>
>> On Tue, 6 Oct 2026, 08:23 Tomasz Kaminski, <tkaminsk@redhat.com> wrote:
>>
>>>
>>>
>>> On Tue, Oct 6, 2026 at 8:54 AM Tomasz Kaminski <tkaminsk@redhat.com>
>>> wrote:
>>>
>>>>
>>>>
>>>> On Mon, Oct 5, 2026 at 11:02 PM Jonathan Wakely <jwakely@redhat.com>
>>>> 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()
>>>>
>>> After re-reading the standard, I do not think it requires that:
>>> > We need to delay writing to the *abbrev and *offset variables until
>>> after we know if the parse succeeded
>>>
>>> The pagraph https://eel.is/c++draft/time.clock.system#nonmembers-6 says:
>>> > If %Z is used and successfully parsed, that value will be assigned to
>>> *abbrev if abbrev is non-null.
>>> <https://eel.is/c++draft/time.clock.system#nonmembers-6.sentence-3>
>>> Which for me reads us, if %Z was successfully parsed, the abbrev is
>>> changed, not if the whole thing
>>> was successfully parsed.
>>>
>>
>> I don't agree with that interpretation.
>> https://eel.is/c++draft/time.parse#17.sentence-1 says to me that parsing
>> all succeeds or all fails, there is no partial success for one part and
>> failure for another.
>>
> The sentence says that failbit is set, it does not mention whether the
> input values are modified.
> That does not contradict my claim that the wording says that abbrev is
> modified as soon as %Z is successfully
> parsed, the failbit (indicating whole parsing failure), may still be set.
>
And I think that the ability to pre-allocate/re-use allocation for parsing
many values is more useful than preserving,
input value of *abbrev.
>
>
>>
>> I agree we should get that clarified.
>>
> We should still fix the original issues, as they seem to be independed.
>
>>
>>
>>> I think this distinction is important, as it could potentially allow no
>>> allocations here
>>> std::string abbrev;
>>> while (in)
>>> from_stream("%T %Z", time, &abbrev);
>>> The second and later iterations can use the memory allocated for
>>> abbrev. This is meaningful if we
>>> use named zones, like Europe/Warsaw, and allows preallocation before.
>>>
>>> But, even my interpretation of %Z seem insufficient, as we cannot parse
>>> directly to *abbrev, because
>>> if the stream fails when reading additional characters, we cannot roll
>>> that back. I think we should clarify
>>> that wording, and either make *abbrev unspecified in case if parsing of
>>> %Z fails, all change the wording
>>> to make the changes atomic (either all are modified or none), as we
>>> require buffering anyway. In this
>>> case I would use my _M_fill_tz suggestion.
>>>
>>> In light of above could you change the patch to just
>>> add __p._M_is_leap_second checks, without changes
>>> to handling of abbrev/offset.
>>>
>>> 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
>>>>>
>>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://gcc.gnu.org/pipermail/libstdc++/attachments/20261006/dc76b1ce/attachment-0001.htm>
More information about the Libstdc++
mailing list