Fix leap second validation when parsing `time_point`s (#2705)

Co-authored-by: Stephan T. Lavavej <stl@nuwen.net>
Co-authored-by: Nicole Mazzuca <mazzucan@outlook.com>

stl/inc/chrono

@@ -2675,11 +2675,11 @@ namespace filesystem {
         }
 
 #if _HAS_CXX20
-        // Assumes that FILETIME counts leap seconds only after the first 27 (i.e., after 1 January 2017), even though
+        // Assumes that FILETIME counts leap seconds since 2018-06-01 (i.e., after the first 27 leap seconds), even
-        // systems can opt out of this behavior.
+        // though systems can opt out of this behavior.
         static constexpr chrono::seconds _Skipped_filetime_leap_seconds{27};
         static constexpr chrono::sys_days _Cutoff{
-            chrono::year_month_day{chrono::year{2017}, chrono::January, chrono::day{1}}};
+            chrono::year_month_day{chrono::year{2018}, chrono::June, chrono::day{1}}};
 
         template <class _Duration>
         _NODISCARD static chrono::utc_time<common_type_t<_Duration, chrono::seconds>> to_utc(
@@ -3454,83 +3454,70 @@ namespace chrono {
             return false;
         }
 
-        enum class _LeapSecondRep : unsigned int {
+        enum class _Leap_second_rep : unsigned int {
-            _None, // tai_clock, gps_clock; oblivious to leap seconds
+            _None, // system_clock, tai_clock, gps_clock; oblivious to leap seconds
-            _Negative, // system_clock, observes negative, but not positive, leap seconds
             _All, // utc_clock
-            _File_time // _Negative before 1 January 2017, _All afterwards.
+            _File_time // _None before 2018-06-01, _All afterwards.
         };
 
         template <class _DurationType>
-        _NODISCARD bool _Make_time_point(_DurationType& _Dur, _LeapSecondRep _Leap) {
+        _NODISCARD bool _Make_time_point(_DurationType& _Dur, _Leap_second_rep _Leap) {
 
             const bool _Consistent{_Calculate_hour24() && _Calculate_year_fields() && _Calculate_ymd()};
             if (!_Consistent || !_Apply_duration_fields<_Parse_tp_or_duration::_Time_point>(_Dur)) {
                 return false;
             }
 
-            const _LeapSecondRep _Original_leap{_Leap};
+            const _Leap_second_rep _Original_leap{_Leap};
-            if (_Leap == _LeapSecondRep::_File_time) {
+            if (_Leap == _Leap_second_rep::_File_time) {
-                const int _Year{*_Century + *_Two_dig_year};
+                _Leap =
-                _Leap = _Year <= 2016 ? _LeapSecondRep::_Negative : _LeapSecondRep::_All;
+                    _Dur < file_clock::_Cutoff.time_since_epoch() ? _Leap_second_rep::_None : _Leap_second_rep::_All;
             }
 
-            if (_Second > (_Leap == _LeapSecondRep::_All ? 60 : 59)) {
+            if (_Second > (_Leap == _Leap_second_rep::_All ? 60 : 59)) {
                 return false;
             }
 
-            // A distinction has to be made here between clocks that tick monotonically, even around a positive leap
+            // A distinction has to be made here between clocks that always count 86,400 seconds/day (system_clock,
-            // second (everything except system_clock) and ones that have a special representation for leap seconds
+            // tai_clock, and gps_clock) and ones that count leap seconds (utc_clock and, since 2018-06-01, file_clock).
-            // (utc_clock and, for negative leap seconds, system_clock). gps_clock and tai_clock, in particular, always
+            // The correct tick count for the clocks that always count 86,400 seconds/day can be determined without
-            // monotonically count 86,400 seconds/day. The correct tick count, therefore, can be determined without
             // knowing whether any leap seconds have occurred, and there aren't any invalid times due to negative leap
             // second deletions.
-            //
-            // system_clock also has 86,400 seconds/day, but observes negative leap seconds by skipping them in its tick
-            // count. So leap second data is needed to check for a valid time, but not to calculate the tick count.
 
-            if (_Leap != _LeapSecondRep::_None) {
+            if (_Leap == _Leap_second_rep::_All) {
-                if (_Hour_24 == 23 && _Minute == 59 && _Second >= 59) {
+                if constexpr (!_Can_represent<_DurationType, seconds>()) {
+                    // Error if _Dur can't represent the adjustment below.
+                    return false;
+                } else {
                     // It's possible that the parsed time doesn't exist because (a) _Seconds == 60 and there *isn't* a
-                    // leap second insertion or (b) _Seconds == 59 and there *is* a leap second subtraction.
+                    // leap second insertion or (b) _Seconds >= 59 and there *is* a leap second subtraction.
-
-                    // Check for quick exit.
                     const auto& _Tzdb{_CHRONO get_tzdb()};
-                    if (_Leap == _LeapSecondRep::_Negative && _Tzdb._All_ls_positive) {
-                        return true;
-                    }
 
                     const bool _Possible_insertion{_Second == 60};
                     const sys_seconds _Target_sys_time{
-                        _CHRONO floor<seconds>(_Dur) + (_Possible_insertion ? seconds{0} : seconds{1})};
+                        _CHRONO floor<seconds>(_Dur) - (_Possible_insertion ? seconds{1} : seconds{0})};
                     const auto& _Ls_vector{_Tzdb.leap_seconds};
-                    const auto _It{_STD lower_bound(_Ls_vector.begin(), _Ls_vector.end(), _Target_sys_time)};
+                    const auto _It{_STD upper_bound(_Ls_vector.begin(), _Ls_vector.end(), _Target_sys_time)};
-                    const bool _Match_leap{_It != _Ls_vector.end() && *_It == _Target_sys_time};
+                    const bool _Match_leap{_It != _Ls_vector.end() && *_It == _Target_sys_time + seconds{1}};
 
-                    // Condition for a good parse: (_Seconds == 59 && !_Match_leap) || (_Match_leap &&
+                    if (_Match_leap) {
-                    // _It->_Is_positive()). Below is the inverse of this.
+                        if (!_It->_Positive()) { // negative leap second
-                    if (!(_Match_leap ? _It->_Positive() : !_Possible_insertion)) {
+                            if (_Second >= 59) {
-                        return false;
+                                return false;
+                            }
+                        }
+                    } else { // no leap second
+                        if (_Possible_insertion) {
+                            return false;
+                        }
                     }
-                }
 
-                if (_Leap == _LeapSecondRep::_All) {
+                    const auto _Offset = _It == _Ls_vector.begin() ? seconds{0} : _Prev_iter(_It)->_Elapsed();
-                    constexpr bool _Can_rep_sec = _Can_represent<_DurationType, seconds>();
+                    _Dur += _CHRONO duration_cast<_DurationType>(_Offset);
-                    if constexpr (_Can_rep_sec) {
-                        _Dur = utc_clock::from_sys(sys_time<_DurationType>{_Dur}).time_since_epoch();
-                        // If we parsed a positive leap second, then _Dur, interpreted as a system time, refers to the
-                        // second *after* insertion. Need to back up one second in this case.
-                        if (_Second == 60) {
-                            _Dur -= _CHRONO duration_cast<_DurationType>(seconds{1});
-                        }
 
-                        if (_Original_leap == _LeapSecondRep::_File_time) {
+                    if (_Original_leap == _Leap_second_rep::_File_time) {
-                            _Dur -= _CHRONO duration_cast<_DurationType>(
+                        _Dur -= _CHRONO duration_cast<_DurationType>(
-                                filesystem::_File_time_clock::_Skipped_filetime_leap_seconds);
+                            filesystem::_File_time_clock::_Skipped_filetime_leap_seconds);
-                        }
-                    } else {
-                        // Error if _Dur can't represent the above adjustment.
-                        return false;
                     }
                 }
             }
@@ -4398,7 +4385,7 @@ namespace chrono {
         constexpr auto _Subsecond_precision{hh_mm_ss<_Duration>::fractional_width};
         _Time_parse_fields _Time{_Istr, _Fmt, _Abbrev, _Offset, _Subsecond_precision};
         _Duration _Dur;
-        if (_Istr && _Time._Make_time_point(_Dur, _Time_parse_fields::_LeapSecondRep::_All)) {
+        if (_Istr && _Time._Make_time_point(_Dur, _Time_parse_fields::_Leap_second_rep::_All)) {
             _Tp = utc_time<_Duration>{_Dur};
         } else {
             _Istr.setstate(ios_base::failbit);
@@ -4413,7 +4400,7 @@ namespace chrono {
         constexpr auto _Subsecond_precision{hh_mm_ss<_Duration>::fractional_width};
         _Time_parse_fields _Time{_Istr, _Fmt, _Abbrev, _Offset, _Subsecond_precision};
         _Duration _Dur;
-        if (_Istr && _Time._Make_time_point(_Dur, _Time_parse_fields::_LeapSecondRep::_Negative)) {
+        if (_Istr && _Time._Make_time_point(_Dur, _Time_parse_fields::_Leap_second_rep::_None)) {
             _Tp = sys_time<_Duration>{_Dur};
         } else {
             _Istr.setstate(ios_base::failbit);
@@ -4428,7 +4415,7 @@ namespace chrono {
         constexpr auto _Subsecond_precision{hh_mm_ss<_Duration>::fractional_width};
         _Time_parse_fields _Time{_Istr, _Fmt, _Abbrev, _Offset, _Subsecond_precision};
         _Duration _Dur;
-        if (_Istr && _Time._Make_time_point(_Dur, _Time_parse_fields::_LeapSecondRep::_None)) {
+        if (_Istr && _Time._Make_time_point(_Dur, _Time_parse_fields::_Leap_second_rep::_None)) {
             _Tp = tai_time<_Duration>{_Dur + _CHRONO duration_cast<_Duration>(days{4383})};
         } else {
             _Istr.setstate(ios_base::failbit);
@@ -4443,7 +4430,7 @@ namespace chrono {
         constexpr auto _Subsecond_precision{hh_mm_ss<_Duration>::fractional_width};
         _Time_parse_fields _Time{_Istr, _Fmt, _Abbrev, _Offset, _Subsecond_precision};
         _Duration _Dur;
-        if (_Istr && _Time._Make_time_point(_Dur, _Time_parse_fields::_LeapSecondRep::_None)) {
+        if (_Istr && _Time._Make_time_point(_Dur, _Time_parse_fields::_Leap_second_rep::_None)) {
             _Tp = gps_time<_Duration>{_Dur - _CHRONO duration_cast<_Duration>(days{3657})};
         } else {
             _Istr.setstate(ios_base::failbit);
@@ -4458,7 +4445,7 @@ namespace chrono {
         constexpr auto _Subsecond_precision{hh_mm_ss<_Duration>::fractional_width};
         _Time_parse_fields _Time{_Istr, _Fmt, _Abbrev, _Offset, _Subsecond_precision};
         _Duration _Dur;
-        if (_Istr && _Time._Make_time_point(_Dur, _Time_parse_fields::_LeapSecondRep::_File_time)) {
+        if (_Istr && _Time._Make_time_point(_Dur, _Time_parse_fields::_Leap_second_rep::_File_time)) {
             constexpr auto _File_clock_adj{_CHRONO duration_cast<_Duration>(
                 filesystem::_File_time_clock::duration{filesystem::__std_fs_file_time_epoch_adjustment})};
             _Tp = file_time<_Duration>{_Dur} + _File_clock_adj;
@@ -4477,7 +4464,7 @@ namespace chrono {
         // *_Offset is not subtracted from local_time, see N4885 [time.clock.local]/4.
         _Time._Utc_offset.reset();
         _Duration _Dur;
-        if (_Istr && _Time._Make_time_point(_Dur, _Time_parse_fields::_LeapSecondRep::_None)) {
+        if (_Istr && _Time._Make_time_point(_Dur, _Time_parse_fields::_Leap_second_rep::_None)) {
             _Tp = local_time<_Duration>{_Dur};
         } else {
             _Istr.setstate(ios_base::failbit);

tests/std/tests/P0355R7_calendars_and_time_zones_io/test.cpp

@@ -907,7 +907,7 @@ void parse_whitespace() {
     fail_parse("", "%n", time);
 }
 
-void insert_leap_second(const sys_days& date, const seconds& value) {
+tzdb copy_tzdb() {
     const auto& my_tzdb = get_tzdb_list().front();
     vector<time_zone> zones;
     vector<time_zone_link> links;
@@ -917,10 +917,7 @@ void insert_leap_second(const sys_days& date, const seconds& value) {
         return time_zone_link{link.name(), link.target()};
     });
 
-    auto leap_vec = my_tzdb.leap_seconds;
+    return {my_tzdb.version, move(zones), move(links), my_tzdb.leap_seconds, my_tzdb._All_ls_positive};
-    leap_vec.emplace_back(date, value == 1s, leap_vec.back()._Elapsed());
-    get_tzdb_list()._Emplace_front(
-        tzdb{my_tzdb.version, move(zones), move(links), move(leap_vec), my_tzdb._All_ls_positive && (value == 1s)});
 }
 
 void test_gh_1952() {
@@ -1055,8 +1052,15 @@ void parse_timepoints() {
 
     // Historical leap seconds don't allow complete testing, because they've all been positive and there haven't been
     // any since 2016 (as of 2021).
-    insert_leap_second(1d / January / 2020y, -1s);
+    {
-    insert_leap_second(1d / January / 2022y, 1s);
+        auto my_tzdb   = copy_tzdb();
+        auto& leap_vec = my_tzdb.leap_seconds;
+        leap_vec.erase(leap_vec.begin() + 27, leap_vec.end());
+        leap_vec.emplace_back(sys_days{1d / January / 2020y}, false, leap_vec.back()._Elapsed());
+        leap_vec.emplace_back(sys_days{1d / January / 2022y}, true, leap_vec.back()._Elapsed());
+        my_tzdb._All_ls_positive = false;
+        get_tzdb_list()._Emplace_front(move(my_tzdb));
+    }
 
     utc_seconds ut_ref = utc_clock::from_sys(sys_days{1d / July / 1972y}) - 1s; // leap second insertion
     test_parse("june 30 23:59:60 1972", "%c", ut);
@@ -1075,11 +1079,25 @@ void parse_timepoints() {
 
     fail_parse("june 30 23:59:60 1973", "%c", ut); // not a leap second insertion
 
+    // the last leap second insertion that file_clock is not aware of
+    test_parse("dec 31 23:59:59 2016", "%c", ut);
+    test_parse("dec 31 23:59:59 2016", "%c", ft);
+    assert(ft == clock_cast<file_clock>(ut));
+
+    test_parse("dec 31 23:59:60 2016", "%c", ut);
+    fail_parse("dec 31 23:59:60 2016", "%c", ft);
+
+    test_parse("jan 01 00:00:00 2017", "%c", ut);
+    test_parse("jan 01 00:00:00 2017", "%c", ft);
+    assert(ft == clock_cast<file_clock>(ut));
+
     ref = sys_days{1d / January / 2020y} - 1s; // negative leap second, UTC time doesn't exist
     fail_parse("dec 31 23:59:59 2019", "%c", ut);
-    fail_parse("dec 31 23:59:59 2019", "%c", st);
     fail_parse("dec 31 23:59:59 2019", "%c", ft);
 
+    test_parse("dec 31 23:59:59 2019", "%c", st);
+    assert(st == ref);
+
     test_parse("dec 31 23:59:59 2019", "%c", lt); // Not UTC, might be valid depending on the time zone.
     assert(lt.time_since_epoch() == ref.time_since_epoch());
 
@@ -1177,6 +1195,13 @@ void parse_timepoints() {
     assert(st == sys_days{23d / March / 1882y});
 
     test_gh_1952();
+
+    // GH-2698: 00:00:60 incorrectly parsed
+    fail_parse("2021-08-28 00:00:60", "%F %T", ut);
+
+    fail_parse("2017-01-01 05:29:60", "%F %T", ut);
+    test_parse("2017-01-01 05:29:60 +05:30", "%F %T %Ez", ut);
+    assert(ut == clock_cast<utc_clock>(sys_days{1d / January / 2017y}) - 1s);
 }
 
 template <class CharT, class CStringOrStdString>