From bd8e675c167031d559a8b4d4ccb255bd62b97d22 Mon Sep 17 00:00:00 2001 From: Elnar D Date: Tue, 20 Apr 2021 11:18:40 -0700 Subject: [PATCH 1/5] : Fix utc_clock seconds formatting Seconds were being formatted incorrectly because we incorrectly assumed that a day is 24*60*60 seconds (which is what flooring does) when in reality they aren't always (like if there was a leap second). This error compounds over time as leap seconds are inserted, so we need to add the elapsed leap seconds at any given time point to get the correct seconds. This issue doesn't affect the `tm` structure because it is constructed out of a system time (`utc_clock::to_sys`), which has 24*60*60 second days. --- stl/inc/chrono | 5 ++++- .../P0355R7_calendars_and_time_zones_formatting/test.cpp | 4 ++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/stl/inc/chrono b/stl/inc/chrono index 0035ee71026..c6a52266b39 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -5485,8 +5485,11 @@ namespace chrono { if constexpr (is_same_v<_Clock, utc_clock>) { if (_CHRONO get_leap_second_info(_Val).is_leap_second) { _Os << _STATICALLY_WIDEN(_CharT, "60"); - return; + } else { + const auto _Dp = _CHRONO floor(_Val) + get_leap_second_info(_Val).elapsed; + _Write_seconds(_Os, hh_mm_ss{_Val - _Dp}); } + return; } const auto _Dp = _CHRONO floor(_Val); _Write_seconds(_Os, hh_mm_ss{_Val - _Dp}); diff --git a/tests/std/tests/P0355R7_calendars_and_time_zones_formatting/test.cpp b/tests/std/tests/P0355R7_calendars_and_time_zones_formatting/test.cpp index 4cdaac2844f..6a8e9d55dbf 100644 --- a/tests/std/tests/P0355R7_calendars_and_time_zones_formatting/test.cpp +++ b/tests/std/tests/P0355R7_calendars_and_time_zones_formatting/test.cpp @@ -250,6 +250,10 @@ void test_clock_formatter() { throw_helper(STR("{:%Z %z %Oz %Ez}"), local_seconds{}); assert(format(STR("{:%S}"), utc_clock::from_sys(get_tzdb().leap_seconds.front().date()) - 1s) == STR("60")); + assert(format(STR("{:%F %T}"), utc_clock::from_sys(get_tzdb().leap_seconds.front().date())) + == STR("1972-07-01 00:00:00")); + assert(format(STR("{:%F %T}"), utc_clock::from_sys(sys_days{January / 9 / 2014} + 12h + 35min + 34s)) + == STR("2014-01-09 12:35:34")); } template From d433ff7ab8308aaa86cd5d2e37d68c0abe0a3320 Mon Sep 17 00:00:00 2001 From: Elnar D Date: Tue, 20 Apr 2021 13:56:41 -0700 Subject: [PATCH 2/5] Fix subseconds on leap second --- stl/inc/chrono | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/stl/inc/chrono b/stl/inc/chrono index c6a52266b39..54b02d3d526 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -5465,28 +5465,33 @@ namespace chrono { _STL_INTERNAL_CHECK(false); } - template - void _Write_seconds(basic_ostream<_CharT, _Traits>& _Os, const hh_mm_ss<_Duration>& _Val) { - _Os << _STD format(_STATICALLY_WIDEN(_CharT, "{:02}"), _Val.seconds().count()); - if constexpr (hh_mm_ss<_Duration>::fractional_width > 0) { + template + void _Write_seconds(basic_ostream<_CharT, _Traits>& _Os, const seconds& _Seconds, const _Precision& _Subseconds) { + _Os << _STD format(_STATICALLY_WIDEN(_CharT, "{:02}"), _Seconds.count()); + if constexpr (_Fractional_width > 0) { _Os << _STD use_facet>(_Os.getloc()).decimal_point(); - if constexpr (treat_as_floating_point_v::precision::rep>) { - _Os << _STD format(_STATICALLY_WIDEN(_CharT, "{:0{}.0f}"), _STD floor(_Val.subseconds().count()), - _Val.fractional_width); - } else { + if constexpr (treat_as_floating_point_v<_Precision::rep>) { _Os << _STD format( - _STATICALLY_WIDEN(_CharT, "{:0{}}"), _Val.subseconds().count(), _Val.fractional_width); + _STATICALLY_WIDEN(_CharT, "{:0{}.0f}"), _STD floor(_Subseconds.count()), _Fractional_width); + } else { + _Os << _STD format(_STATICALLY_WIDEN(_CharT, "{:0{}}"), _Subseconds.count(), _Fractional_width); } } } + template + void _Write_seconds(basic_ostream<_CharT, _Traits>& _Os, const hh_mm_ss<_Duration>& _Val) { + _Write_seconds<_Val.fractional_width>(_Os, _Val.seconds(), _Val.subseconds()); + } + template void _Write_seconds(basic_ostream<_CharT, _Traits>& _Os, const time_point<_Clock, _Duration>& _Val) { if constexpr (is_same_v<_Clock, utc_clock>) { + const auto _Dp = _CHRONO floor(_Val) + get_leap_second_info(_Val).elapsed; + const auto _Hms = hh_mm_ss{_Val - _Dp}; if (_CHRONO get_leap_second_info(_Val).is_leap_second) { - _Os << _STATICALLY_WIDEN(_CharT, "60"); + _Write_seconds<_Hms.fractional_width>(_Os, _Hms.seconds() + 60s, _Hms.subseconds()); } else { - const auto _Dp = _CHRONO floor(_Val) + get_leap_second_info(_Val).elapsed; _Write_seconds(_Os, hh_mm_ss{_Val - _Dp}); } return; From c4593da140db42b15a7341682f53ebac24fd56bd Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 20 Apr 2021 16:55:19 -0700 Subject: [PATCH 3/5] Code review feedback. --- stl/inc/chrono | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/stl/inc/chrono b/stl/inc/chrono index 54b02d3d526..ad69725daaf 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -5466,11 +5466,12 @@ namespace chrono { } template - void _Write_seconds(basic_ostream<_CharT, _Traits>& _Os, const seconds& _Seconds, const _Precision& _Subseconds) { + void _Write_fractional_seconds( + basic_ostream<_CharT, _Traits>& _Os, const seconds& _Seconds, const _Precision& _Subseconds) { _Os << _STD format(_STATICALLY_WIDEN(_CharT, "{:02}"), _Seconds.count()); if constexpr (_Fractional_width > 0) { _Os << _STD use_facet>(_Os.getloc()).decimal_point(); - if constexpr (treat_as_floating_point_v<_Precision::rep>) { + if constexpr (treat_as_floating_point_v) { _Os << _STD format( _STATICALLY_WIDEN(_CharT, "{:0{}.0f}"), _STD floor(_Subseconds.count()), _Fractional_width); } else { @@ -5481,18 +5482,19 @@ namespace chrono { template void _Write_seconds(basic_ostream<_CharT, _Traits>& _Os, const hh_mm_ss<_Duration>& _Val) { - _Write_seconds<_Val.fractional_width>(_Os, _Val.seconds(), _Val.subseconds()); + _Write_fractional_seconds<_Val.fractional_width>(_Os, _Val.seconds(), _Val.subseconds()); } template void _Write_seconds(basic_ostream<_CharT, _Traits>& _Os, const time_point<_Clock, _Duration>& _Val) { if constexpr (is_same_v<_Clock, utc_clock>) { - const auto _Dp = _CHRONO floor(_Val) + get_leap_second_info(_Val).elapsed; - const auto _Hms = hh_mm_ss{_Val - _Dp}; - if (_CHRONO get_leap_second_info(_Val).is_leap_second) { - _Write_seconds<_Hms.fractional_width>(_Os, _Hms.seconds() + 60s, _Hms.subseconds()); + const auto _Lsi = _CHRONO get_leap_second_info(_Val); + const auto _Dp = _CHRONO floor(_Val) + _Lsi.elapsed; + const hh_mm_ss _Hms{_Val - _Dp}; + if (_Lsi.is_leap_second) { + _Write_fractional_seconds<_Hms.fractional_width>(_Os, _Hms.seconds() + 60s, _Hms.subseconds()); } else { - _Write_seconds(_Os, hh_mm_ss{_Val - _Dp}); + _Write_seconds(_Os, _Hms); } return; } From fe2210f0245c298337c29a9e77e562b2f8532435 Mon Sep 17 00:00:00 2001 From: Elnar D Date: Tue, 20 Apr 2021 17:21:38 -0700 Subject: [PATCH 4/5] Leap second insertion accounted for --- stl/inc/chrono | 6 ++++-- .../P0355R7_calendars_and_time_zones_formatting/test.cpp | 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/stl/inc/chrono b/stl/inc/chrono index ad69725daaf..320a2356e39 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -5489,12 +5489,14 @@ namespace chrono { void _Write_seconds(basic_ostream<_CharT, _Traits>& _Os, const time_point<_Clock, _Duration>& _Val) { if constexpr (is_same_v<_Clock, utc_clock>) { const auto _Lsi = _CHRONO get_leap_second_info(_Val); - const auto _Dp = _CHRONO floor(_Val) + _Lsi.elapsed; + // If leap second is being inserter, subtract it from the start of the day. + const auto _Dp = _CHRONO floor(_Val) + _Lsi.elapsed - 1s; const hh_mm_ss _Hms{_Val - _Dp}; if (_Lsi.is_leap_second) { _Write_fractional_seconds<_Hms.fractional_width>(_Os, _Hms.seconds() + 60s, _Hms.subseconds()); } else { - _Write_seconds(_Os, _Hms); + // A leap second was not inserted, so reduce the distance from the start of day until now by one. + _Write_fractional_seconds<_Hms.fractional_width>(_Os, _Hms.seconds() - 1s, _Hms.subseconds()); } return; } diff --git a/tests/std/tests/P0355R7_calendars_and_time_zones_formatting/test.cpp b/tests/std/tests/P0355R7_calendars_and_time_zones_formatting/test.cpp index 6a8e9d55dbf..ec34fe57c85 100644 --- a/tests/std/tests/P0355R7_calendars_and_time_zones_formatting/test.cpp +++ b/tests/std/tests/P0355R7_calendars_and_time_zones_formatting/test.cpp @@ -254,6 +254,8 @@ void test_clock_formatter() { == STR("1972-07-01 00:00:00")); assert(format(STR("{:%F %T}"), utc_clock::from_sys(sys_days{January / 9 / 2014} + 12h + 35min + 34s)) == STR("2014-01-09 12:35:34")); + assert(format(STR("{:%F %T}"), utc_clock::from_sys(get_tzdb().leap_seconds.front().date()) - 500ms) + == STR("1972-06-30 23:59:60.500")); } template From 7546b49d36965644d1f9def7871f86825b6cfcee Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 20 Apr 2021 18:59:27 -0700 Subject: [PATCH 5/5] Fix and test UTC arithmetic. Also fixes Clang compiler errors. Co-authored-by: MattStephanson <68978048+MattStephanson@users.noreply.github.com> --- stl/inc/chrono | 12 +++++----- .../test.cpp | 22 +++++++++++++++++++ 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/stl/inc/chrono b/stl/inc/chrono index 320a2356e39..7e49b82e144 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -5482,21 +5482,21 @@ namespace chrono { template void _Write_seconds(basic_ostream<_CharT, _Traits>& _Os, const hh_mm_ss<_Duration>& _Val) { - _Write_fractional_seconds<_Val.fractional_width>(_Os, _Val.seconds(), _Val.subseconds()); + _Write_fractional_seconds::fractional_width>(_Os, _Val.seconds(), _Val.subseconds()); } template void _Write_seconds(basic_ostream<_CharT, _Traits>& _Os, const time_point<_Clock, _Duration>& _Val) { if constexpr (is_same_v<_Clock, utc_clock>) { const auto _Lsi = _CHRONO get_leap_second_info(_Val); - // If leap second is being inserter, subtract it from the start of the day. - const auto _Dp = _CHRONO floor(_Val) + _Lsi.elapsed - 1s; + const auto _Dp = + _CHRONO floor(_Val - _Lsi.elapsed) + _Lsi.elapsed - seconds{_Lsi.is_leap_second ? 1 : 0}; const hh_mm_ss _Hms{_Val - _Dp}; + constexpr auto _Fractional_width = decltype(_Hms)::fractional_width; if (_Lsi.is_leap_second) { - _Write_fractional_seconds<_Hms.fractional_width>(_Os, _Hms.seconds() + 60s, _Hms.subseconds()); + _Write_fractional_seconds<_Fractional_width>(_Os, _Hms.seconds() + seconds{60}, _Hms.subseconds()); } else { - // A leap second was not inserted, so reduce the distance from the start of day until now by one. - _Write_fractional_seconds<_Hms.fractional_width>(_Os, _Hms.seconds() - 1s, _Hms.subseconds()); + _Write_fractional_seconds<_Fractional_width>(_Os, _Hms.seconds(), _Hms.subseconds()); } return; } diff --git a/tests/std/tests/P0355R7_calendars_and_time_zones_formatting/test.cpp b/tests/std/tests/P0355R7_calendars_and_time_zones_formatting/test.cpp index ec34fe57c85..3c14b7c28f5 100644 --- a/tests/std/tests/P0355R7_calendars_and_time_zones_formatting/test.cpp +++ b/tests/std/tests/P0355R7_calendars_and_time_zones_formatting/test.cpp @@ -256,6 +256,28 @@ void test_clock_formatter() { == STR("2014-01-09 12:35:34")); assert(format(STR("{:%F %T}"), utc_clock::from_sys(get_tzdb().leap_seconds.front().date()) - 500ms) == STR("1972-06-30 23:59:60.500")); + + // Test an ordinary day. + const auto utc_2021_05_04 = utc_clock::from_sys(sys_days{2021y / May / 4}); + + // This is both the last day of a leap year (366th day) and the day of a leap second insertion. + const auto utc_2016_12_31 = utc_clock::from_sys(sys_days{2016y / December / 31}); + + for (const auto& h : {0h, 1h, 7h, 22h, 23h}) { // Accelerate testing; 24 * 60 * 61 iterations would be a lot. + for (const auto& m : {0min, 1min, 7min, 58min, 59min}) { + for (const auto& s : {0s, 1s, 7s, 58s, 59s, 60s}) { + if (s != 60s) { + assert(format(STR("{:%F %T}"), utc_2021_05_04 + h + m + s) + == format(STR("2021-05-04 {:02}:{:02}:{:02}"), h.count(), m.count(), s.count())); + } + + if ((h == 23h && m == 59min) || s != 60s) { + assert(format(STR("{:%F %T}"), utc_2016_12_31 + h + m + s) + == format(STR("2016-12-31 {:02}:{:02}:{:02}"), h.count(), m.count(), s.count())); + } + } + } + } } template