[libc++][chrono] implements UTC clock. #90393
Open
+2,459
−6
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Mark de Wever (mordante) ChangesWhile implementing this feature and its associated LWG issues it turns out
Implements parts of:
Patch is 115.96 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/90393.diff 26 Files Affected:
diff --git a/libcxx/benchmarks/CMakeLists.txt b/libcxx/benchmarks/CMakeLists.txt
index 527a2acf2d3b36..ea7ef6902f7266 100644
--- a/libcxx/benchmarks/CMakeLists.txt
+++ b/libcxx/benchmarks/CMakeLists.txt
@@ -229,6 +229,7 @@ set(BENCHMARK_TESTS
system_error.bench.cpp
to_chars.bench.cpp
unordered_set_operations.bench.cpp
+ utc_clock.bench.cpp
util_smartptr.bench.cpp
variant_visit_1.bench.cpp
variant_visit_2.bench.cpp
diff --git a/libcxx/benchmarks/utc_clock.bench.cpp b/libcxx/benchmarks/utc_clock.bench.cpp
new file mode 100644
index 00000000000000..255b1dde52aa67
--- /dev/null
+++ b/libcxx/benchmarks/utc_clock.bench.cpp
@@ -0,0 +1,54 @@
+//===----------------------------------------------------------------------===//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include <chrono>
+
+#include "benchmark/benchmark.h"
+
+// Benchmarks the performance of the UTC <-> system time conversions. These
+// operations determine the sum of leap second insertions at a specific time.
+
+static void BM_from_sys(benchmark::State& state) {
+ std::chrono::sys_days date{std::chrono::July / 1 / state.range(0)};
+ for (auto _ : state)
+ benchmark::DoNotOptimize(std::chrono::utc_clock::from_sys(date));
+}
+
+BENCHMARK(BM_from_sys)
+ ->Arg(1970) // before the first leap seconds
+ ->Arg(1979) // in the first half of inserted leap seconds
+ ->Arg(1993) // in the second half of inserted leap seconds
+ ->Arg(2100); // after the last leap second
+
+BENCHMARK(BM_from_sys)->Arg(1970)->Arg(1979)->Arg(1993)->Arg(2100)->Threads(4);
+BENCHMARK(BM_from_sys)->Arg(1970)->Arg(1979)->Arg(1993)->Arg(2100)->Threads(16);
+
+static void BM_to_sys(benchmark::State& state) {
+ // 59 sec offset means we pass th UTC offset for the leap second; assuming
+ // there won't be more than 59 leap seconds ever.
+ std::chrono::utc_seconds date{
+ std::chrono::sys_days{std::chrono::July / 1 / state.range(0)}.time_since_epoch() + std::chrono::seconds{59}};
+ for (auto _ : state)
+ benchmark::DoNotOptimize(std::chrono::utc_clock::to_sys(date));
+}
+
+BENCHMARK(BM_to_sys)
+ ->Arg(1970) // before the first leap seconds
+ ->Arg(1979) // in the first half of inserted leap seconds
+ ->Arg(1993) // in the second half of inserted leap seconds
+ ->Arg(2100); // after the last leap second
+
+BENCHMARK(BM_to_sys)->Arg(1970)->Arg(1979)->Arg(1993)->Arg(2100)->Threads(4);
+BENCHMARK(BM_to_sys)->Arg(1970)->Arg(1979)->Arg(1993)->Arg(2100)->Threads(16);
+
+int main(int argc, char** argv) {
+ benchmark::Initialize(&argc, argv);
+ if (benchmark::ReportUnrecognizedArguments(argc, argv))
+ return 1;
+
+ benchmark::RunSpecifiedBenchmarks();
+}
diff --git a/libcxx/docs/Status/Cxx20.rst b/libcxx/docs/Status/Cxx20.rst
index 23289dc6e5961b..baa1dc6d613826 100644
--- a/libcxx/docs/Status/Cxx20.rst
+++ b/libcxx/docs/Status/Cxx20.rst
@@ -55,9 +55,9 @@ Paper Status
* ``Input parsers`` not done
* ``Stream output`` Obsolete due to `P1361R2 <https://wg21.link/P1361R2>`_ "Integration of chrono with text formatting"
* ``Time zone and leap seconds`` In Progress
+ * ``UTC clock`` Clang 19
* ``TAI clock`` not done
* ``GPS clock`` not done
- * ``UTC clock`` not done
.. [#note-P0718] P0718: Implemented deprecation of ``shared_ptr`` atomic access APIs only.
diff --git a/libcxx/docs/Status/Cxx20Issues.csv b/libcxx/docs/Status/Cxx20Issues.csv
index fed1c7e736248f..556da4fc72f83a 100644
--- a/libcxx/docs/Status/Cxx20Issues.csv
+++ b/libcxx/docs/Status/Cxx20Issues.csv
@@ -238,7 +238,7 @@
"`3313 <https://wg21.link/LWG3313>`__","``join_view::iterator::operator--``\ is incorrectly constrained","Prague","|Complete|","14.0","|ranges|"
"`3314 <https://wg21.link/LWG3314>`__","Is stream insertion behavior locale dependent when ``Period::type``\ is ``micro``\ ?","Prague","|Complete|","16.0","|chrono|"
"`3315 <https://wg21.link/LWG3315>`__","Correct Allocator Default Behavior","Prague","",""
-"`3316 <https://wg21.link/LWG3316>`__","Correctly define epoch for ``utc_clock``\ / ``utc_timepoint``\ ","Prague","","","|chrono|"
+"`3316 <https://wg21.link/LWG3316>`__","Correctly define epoch for ``utc_clock``\ / ``utc_timepoint``\ ","Prague","|Nothing To Do|","","|chrono|"
"`3317 <https://wg21.link/LWG3317>`__","Incorrect ``operator<<``\ for floating-point durations","Prague","","","|chrono|"
"`3318 <https://wg21.link/LWG3318>`__","Clarify whether clocks can represent time before their epoch","Prague","","","|chrono|"
"`3319 <https://wg21.link/LWG3319>`__","Properly reference specification of IANA time zone database","Prague","|Nothing To Do|","","|chrono|"
diff --git a/libcxx/docs/Status/FormatPaper.csv b/libcxx/docs/Status/FormatPaper.csv
index f29f1f7ca74875..bf733e264cde22 100644
--- a/libcxx/docs/Status/FormatPaper.csv
+++ b/libcxx/docs/Status/FormatPaper.csv
@@ -2,7 +2,7 @@ Section,Description,Dependencies,Assignee,Status,First released version
`P1361 <https://wg21.link/P1361>`__ `P2372 <https://wg21.link/P2372>`__,"Formatting chrono"
`[time.syn] <https://wg21.link/time.syn>`_,"Formatter ``chrono::duration<Rep, Period>``",,Mark de Wever,|Complete|,16.0
`[time.syn] <https://wg21.link/time.syn>`_,"Formatter ``chrono::sys_time<Duration>``",,Mark de Wever,|Complete|,17.0
-`[time.syn] <https://wg21.link/time.syn>`_,"Formatter ``chrono::utc_time<Duration>``",A ``<chrono>`` implementation,Mark de Wever,,,
+`[time.syn] <https://wg21.link/time.syn>`_,"Formatter ``chrono::utc_time<Duration>``",,Mark de Wever,|Complete|,19.0
`[time.syn] <https://wg21.link/time.syn>`_,"Formatter ``chrono::tai_time<Duration>``",A ``<chrono>`` implementation,Mark de Wever,,,
`[time.syn] <https://wg21.link/time.syn>`_,"Formatter ``chrono::gps_time<Duration>``",A ``<chrono>`` implementation,Mark de Wever,,,
`[time.syn] <https://wg21.link/time.syn>`_,"Formatter ``chrono::file_time<Duration>``",,Mark de Wever,|Complete|,17.0
diff --git a/libcxx/include/CMakeLists.txt b/libcxx/include/CMakeLists.txt
index 1296c536bc882c..4aff1487030500 100644
--- a/libcxx/include/CMakeLists.txt
+++ b/libcxx/include/CMakeLists.txt
@@ -285,6 +285,7 @@ set(files
__chrono/time_zone_link.h
__chrono/tzdb.h
__chrono/tzdb_list.h
+ __chrono/utc_clock.h
__chrono/weekday.h
__chrono/year.h
__chrono/year_month.h
diff --git a/libcxx/include/__chrono/convert_to_tm.h b/libcxx/include/__chrono/convert_to_tm.h
index 881a4970822d8e..8e9310da2ce94a 100644
--- a/libcxx/include/__chrono/convert_to_tm.h
+++ b/libcxx/include/__chrono/convert_to_tm.h
@@ -24,6 +24,7 @@
#include <__chrono/sys_info.h>
#include <__chrono/system_clock.h>
#include <__chrono/time_point.h>
+#include <__chrono/utc_clock.h>
#include <__chrono/weekday.h>
#include <__chrono/year.h>
#include <__chrono/year_month.h>
@@ -96,6 +97,21 @@ _LIBCPP_HIDE_FROM_ABI _Tm __convert_to_tm(const chrono::sys_time<_Duration> __tp
return __result;
}
+# if !defined(_LIBCPP_HAS_NO_TIME_ZONE_DATABASE) && !defined(_LIBCPP_HAS_NO_FILESYSTEM) && \
+ !defined(_LIBCPP_HAS_NO_LOCALIZATION)
+template <class _Tm, class _Duration>
+_LIBCPP_HIDE_FROM_ABI _Tm __convert_to_tm(chrono::utc_time<_Duration> __tp) {
+ _Tm __result = std::__convert_to_tm<_Tm>(chrono::utc_clock::to_sys(__tp));
+
+ if (chrono::get_leap_second_info(__tp).is_leap_second)
+ ++__result.tm_sec;
+
+ return __result;
+}
+
+# endif // !defined(_LIBCPP_HAS_NO_TIME_ZONE_DATABASE) && !defined(_LIBCPP_HAS_NO_FILESYSTEM) &&
+ // !defined(_LIBCPP_HAS_NO_LOCALIZATION)
+
// Convert a chrono (calendar) time point, or dururation to the given _Tm type,
// which must have the same properties as std::tm.
template <class _Tm, class _ChronoT>
@@ -108,6 +124,8 @@ _LIBCPP_HIDE_FROM_ABI _Tm __convert_to_tm(const _ChronoT& __value) {
if constexpr (__is_time_point<_ChronoT>) {
if constexpr (same_as<typename _ChronoT::clock, chrono::system_clock>)
return std::__convert_to_tm<_Tm>(__value);
+ else if constexpr (same_as<typename _ChronoT::clock, chrono::utc_clock>)
+ return std::__convert_to_tm<_Tm>(__value);
else if constexpr (same_as<typename _ChronoT::clock, chrono::file_clock>)
return std::__convert_to_tm<_Tm>(_ChronoT::clock::to_sys(__value));
else if constexpr (same_as<typename _ChronoT::clock, chrono::local_t>)
diff --git a/libcxx/include/__chrono/formatter.h b/libcxx/include/__chrono/formatter.h
index e9b81c3de8a700..6a6f60341c1394 100644
--- a/libcxx/include/__chrono/formatter.h
+++ b/libcxx/include/__chrono/formatter.h
@@ -28,6 +28,7 @@
#include <__chrono/sys_info.h>
#include <__chrono/system_clock.h>
#include <__chrono/time_point.h>
+#include <__chrono/utc_clock.h>
#include <__chrono/weekday.h>
#include <__chrono/year.h>
#include <__chrono/year_month.h>
@@ -673,6 +674,23 @@ struct _LIBCPP_TEMPLATE_VIS formatter<chrono::sys_time<_Duration>, _CharT> : pub
}
};
+# if !defined(_LIBCPP_HAS_NO_TIME_ZONE_DATABASE) && !defined(_LIBCPP_HAS_NO_FILESYSTEM) && \
+ !defined(_LIBCPP_HAS_NO_LOCALIZATION)
+
+template <class _Duration, __fmt_char_type _CharT>
+struct _LIBCPP_TEMPLATE_VIS formatter<chrono::utc_time<_Duration>, _CharT> : public __formatter_chrono<_CharT> {
+public:
+ using _Base = __formatter_chrono<_CharT>;
+
+ template <class _ParseContext>
+ _LIBCPP_HIDE_FROM_ABI constexpr typename _ParseContext::iterator parse(_ParseContext& __ctx) {
+ return _Base::__parse(__ctx, __format_spec::__fields_chrono, __format_spec::__flags::__clock);
+ }
+};
+
+# endif // !defined(_LIBCPP_HAS_NO_TIME_ZONE_DATABASE) && !defined(_LIBCPP_HAS_NO_FILESYSTEM) &&
+ // !defined(_LIBCPP_HAS_NO_LOCALIZATION)
+
template <class _Duration, __fmt_char_type _CharT>
struct _LIBCPP_TEMPLATE_VIS formatter<chrono::file_time<_Duration>, _CharT> : public __formatter_chrono<_CharT> {
public:
diff --git a/libcxx/include/__chrono/ostream.h b/libcxx/include/__chrono/ostream.h
index ecf07a320c8b94..4862a8ae326a3e 100644
--- a/libcxx/include/__chrono/ostream.h
+++ b/libcxx/include/__chrono/ostream.h
@@ -22,6 +22,7 @@
#include <__chrono/statically_widen.h>
#include <__chrono/sys_info.h>
#include <__chrono/system_clock.h>
+#include <__chrono/utc_clock.h>
#include <__chrono/weekday.h>
#include <__chrono/year.h>
#include <__chrono/year_month.h>
@@ -56,6 +57,17 @@ operator<<(basic_ostream<_CharT, _Traits>& __os, const sys_days& __dp) {
return __os << year_month_day{__dp};
}
+# if !defined(_LIBCPP_HAS_NO_TIME_ZONE_DATABASE) && !defined(_LIBCPP_HAS_NO_FILESYSTEM) && \
+ !defined(_LIBCPP_HAS_NO_LOCALIZATION)
+
+template <class _CharT, class _Traits, class _Duration>
+_LIBCPP_HIDE_FROM_ABI basic_ostream<_CharT, _Traits>&
+operator<<(basic_ostream<_CharT, _Traits>& __os, const utc_time<_Duration>& __tp) {
+ return __os << std::format(__os.getloc(), _LIBCPP_STATICALLY_WIDEN(_CharT, "{:L%F %T}"), __tp);
+}
+# endif // !defined(_LIBCPP_HAS_NO_TIME_ZONE_DATABASE) && !defined(_LIBCPP_HAS_NO_FILESYSTEM) &&
+ // !defined(_LIBCPP_HAS_NO_LOCALIZATION)
+
template <class _CharT, class _Traits, class _Duration>
_LIBCPP_HIDE_FROM_ABI basic_ostream<_CharT, _Traits>&
operator<<(basic_ostream<_CharT, _Traits>& __os, const file_time<_Duration> __tp) {
diff --git a/libcxx/include/__chrono/utc_clock.h b/libcxx/include/__chrono/utc_clock.h
new file mode 100644
index 00000000000000..a3db972f26a4e2
--- /dev/null
+++ b/libcxx/include/__chrono/utc_clock.h
@@ -0,0 +1,155 @@
+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef _LIBCPP___CHRONO_UTC_CLOCK_H
+#define _LIBCPP___CHRONO_UTC_CLOCK_H
+
+#include <version>
+// Enable the contents of the header only when libc++ was built with experimental features enabled.
+#if !defined(_LIBCPP_HAS_NO_INCOMPLETE_TZDB)
+
+# include <__chrono/duration.h>
+# include <__chrono/system_clock.h>
+# include <__chrono/time_point.h>
+# include <__chrono/tzdb.h>
+# include <__chrono/tzdb_list.h>
+# include <__config>
+# include <__type_traits/common_type.h>
+
+# if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+# pragma GCC system_header
+# endif
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+# if _LIBCPP_STD_VER >= 20 && !defined(_LIBCPP_HAS_NO_TIME_ZONE_DATABASE) && !defined(_LIBCPP_HAS_NO_FILESYSTEM) && \
+ !defined(_LIBCPP_HAS_NO_LOCALIZATION)
+
+namespace chrono {
+
+class utc_clock;
+
+template <class _Duration>
+using utc_time = time_point<utc_clock, _Duration>;
+using utc_seconds = utc_time<seconds>;
+
+class utc_clock {
+public:
+ using rep = system_clock::rep;
+ using period = system_clock::period;
+ using duration = chrono::duration<rep, period>;
+ using time_point = chrono::time_point<utc_clock>;
+ static constexpr bool is_steady = false; // The system_clock is not steady.
+
+ [[nodiscard]] _LIBCPP_HIDE_FROM_ABI static time_point now() { return from_sys(system_clock::now()); }
+
+ template <class _Duration>
+ [[nodiscard]] _LIBCPP_HIDE_FROM_ABI static sys_time<common_type_t<_Duration, seconds>>
+ to_sys(const utc_time<_Duration>& __time);
+
+ template <class _Duration>
+ [[nodiscard]] _LIBCPP_HIDE_FROM_ABI static utc_time<common_type_t<_Duration, seconds>>
+ from_sys(const sys_time<_Duration>& __time) {
+ using _Rp = utc_time<common_type_t<_Duration, seconds>>;
+ // TODO TZDB investigate optimizations.
+ //
+ // The leap second database stores all transitions, this mean to calculate
+ // the current number of leap seconds the code needs to iterate over all
+ // leap seconds to accumulate the sum. Then the sum can be used to determine
+ // the sys_time. Accessing the database involves acquiring a mutex.
+ //
+ // The historic entries in the database are immutable. Hard-coding these
+ // values in a table would allow:
+ // - To store the sum, allowing a binary search on the data.
+ // - Avoid acquiring a mutex.
+ // The disadvantage are:
+ // - A slightly larger code size.
+ //
+ // There are two optimization directions
+ // - hard-code the database and do a linear search for future entries. This
+ // search can start at the back, and should probably contain very few
+ // entries. (Adding leap seconds is quite rare and new release of libc++
+ // can add the new entries; they are announced half a year before they are
+ // added.)
+ // - During parsing the leap seconds store an additional database in the
+ // dylib with the list of the sum of the leap seconds. In that case there
+ // can be a private function __get_utc_to_sys_table that returns the
+ // table.
+ //
+ // Note for to_sys there are no optimizations to be done; it uses
+ // get_leap_second_info. The function get_leap_second_info could benefit
+ // from optimizations as described above; again both options apply.
+
+ // Both UTC and the system clock use the same epoch. The Standard
+ // specifies from 1970-01-01 even when UTC starts at
+ // 1972-01-01 00:00:10 TAI. So when the sys_time is before epoch we can be
+ // sure there both clocks return the same value.
+
+ const tzdb& __tzdb = chrono::get_tzdb();
+ _Rp __result{__time.time_since_epoch()};
+ for (const auto& __leap_second : __tzdb.leap_seconds) {
+ if (__time < __leap_second)
+ return __result;
+
+ __result += __leap_second.value();
+ }
+ return __result;
+ }
+};
+
+struct leap_second_info {
+ bool is_leap_second;
+ seconds elapsed;
+};
+
+template <class _Duration>
+[[nodiscard]] _LIBCPP_HIDE_FROM_ABI leap_second_info get_leap_second_info(const utc_time<_Duration>& __time) {
+ const tzdb& __tzdb = chrono::get_tzdb();
+ if (__tzdb.leap_seconds.empty())
+ return {false, chrono::seconds{0}};
+
+ sys_seconds __sys{chrono::floor<seconds>(__time).time_since_epoch()};
+ seconds __elapsed{0};
+ for (const auto& __leap_second : __tzdb.leap_seconds) {
+ if (__sys == __leap_second.date() + __elapsed)
+ return {__leap_second.value() > 0s, // only positive leap seconds are considered leap seconds
+ __elapsed + __leap_second.value()};
+
+ if (__sys < __leap_second.date() + __elapsed)
+ return {false, __elapsed};
+
+ __elapsed += __leap_second.value();
+ }
+
+ return {false, __elapsed};
+}
+
+template <class _Duration>
+[[nodiscard]] _LIBCPP_HIDE_FROM_ABI sys_time<common_type_t<_Duration, seconds>>
+utc_clock::to_sys(const utc_time<_Duration>& __time) {
+ using _Dp = common_type_t<_Duration, seconds>;
+ leap_second_info __info = get_leap_second_info(__time);
+
+ sys_time<common_type_t<_Duration, seconds>> __result{__time.time_since_epoch() - __info.elapsed};
+ if (__info.is_leap_second)
+ return chrono::floor<seconds>(__result) + chrono::seconds{1} - _Dp{1};
+
+ return __result;
+}
+
+} // namespace chrono
+
+# endif // _LIBCPP_STD_VER >= 20 && !defined(_LIBCPP_HAS_NO_TIME_ZONE_DATABASE) && !defined(_LIBCPP_HAS_NO_FILESYSTEM)
+ // && !defined(_LIBCPP_HAS_NO_LOCALIZATION)
+
+_LIBCPP_END_NAMESPACE_STD
+
+#endif // !defined(_LIBCPP_HAS_NO_INCOMPLETE_TZDB)
+
+#endif // _LIBCPP___CHRONO_UTC_CLOCK_H
diff --git a/libcxx/include/chrono b/libcxx/include/chrono
index 4d8398af1a108f..7ce527f120e9e4 100644
--- a/libcxx/include/chrono
+++ b/libcxx/include/chrono
@@ -300,6 +300,41 @@ template<class charT, class traits> // C++20
basic_ostream<charT, traits>&
operator<<(basic_ostream<charT, traits>& os, const sys_days& dp);
+// [time.clock.utc], class utc_clock
+class utc_clock { // C++20
+public:
+ using rep = a signed arithmetic type;
+ using period = ratio<unspecified, unspecified>;
+ using duration = chrono::duration<rep, period>;
+ using time_point = chrono::time_point<utc_clock>;
+ static constexpr bool is_steady = unspecified;
+
+ static time_point now();
+
+ template<class Duration>
+ static sys_time<common_type_t<Duration, seconds>>
+ to_sys(const utc_time<Duration>& t);
+ template<class Duration>
+ static utc_time<common_type_t<Duration, seconds>>
+ from_sys(const sys_time<Duration>& t);
+};
+
+template<class Duration>
+using utc_time = time_point<utc_clock, Duration>; // C++20
+using utc_seconds = utc_time<seconds>; // C++20
+
+template<class charT, class traits, class Duration> // C++20
+ basic_ostream<charT, traits>&
+ operator<<(basic_ostream<charT, traits>& os, const utc_time<Duration>& t);
+
+struct leap_second_info { // C++20
+ bool is_leap_second;
+ seconds elapsed;
+};
+
+template<class Duration> // C++20
+ leap_second_info get_leap_second_info(const utc_time<Duration>& ut);
+
class file_clock // C++20
{
public:
@@ -827,6 +862,8 @@ strong_ordering operator<=>(const time_zone_link& x, const time_zone_link& y);
namespace std {
template<class Duration, class charT>
struct formatter<chrono::sys_time<Duration>, charT>; // C++20
+ template<class Duration, class charT>
+ struct formatter<chrono::utc_time<Duration>, charT>; // C++20
template<class Duration, class charT>
struct formatter<chrono::filetime<Duration>, charT>; // C++20
template<class Duration, class charT>
@@ -942,6 +979,7 @@ constexpr chrono::year operator ""y(unsigned lo
# include <__chrono/time_zone_link.h>
# include <__chrono/tzdb.h>
# include <__chrono/tzdb_list.h>
+# include <__chrono/utc_clock.h>
# endif
#endif
diff --git a/libcxx/include/module.modulemap b/libcxx/include/module.modulemap
index...
[truncated]
|
While implementing this feature and its associated LWG issues it turns out - LWG3316 Correctly define epoch for utc_clock / utc_timepoint only added non-normative wording to the standard. Implements parts of: - P0355 Extending <chrono> to Calendars and Time Zones - P1361 Integration of chrono with text formatting - LWG3359 <chrono> leap second support should allow for negative leap seconds
mordante
force-pushed
the
users/mordante/implements_utc_clock
branch
from
188e6f4
to
77edef4
Compare
|
|
||
| sys_time<common_type_t<_Duration, seconds>> __result{__time.time_since_epoch() - __info.elapsed}; | ||
| if (__info.is_leap_second) | ||
| return chrono::floor<seconds>(__result) + chrono::seconds{1} - _Dp{1}; |
There was a problem hiding this comment.
I don't think this is right for floating point duration reps, where the ulp can be more or less than _Dp{1}. I think nextafter is needed for reps where treat_as_floating_point_v is true. (Assuming you have such an overload for extended-precision floats. I'm also uncertain if [time.duration.general]/2 allows a user-defined type to "emulate" a floating point type by specializing treat_as_floating_point.)
There was a problem hiding this comment.
Good point. I'll need to look into this.
| // epoch + transition time in the database + leap seconds before the transition. | ||
| test_transition(epoch + 60s + 0s, 0s, true); | ||
| test_transition(epoch + 120s + 1s, 1s, true); | ||
| test_transition(epoch + 180s + 2s, 2s, false); |
There was a problem hiding this comment.
I don't think this test is quite right. But since we don't have any historical practice with negative leap seconds, my reasoning is based on an assumption: the IANA db contains the earliest time after a leap second is fully inserted. I'm basing this on the fact that positive leap seconds are inserted at 11:59:60 PM on 30 June/31 December, but the db contains 12:00:00 AM on 1 July/1 January.
For a positive leap second, the utc_time when insertion begins is this sys_time plus the accumulated offset, because the insertion begins at the time that would otherwise be 12:00:00 AM on the following day. The test is correct for those cases. But negative leap seconds are different. Insertion happens at the time that would otherwise be 11:59:59 PM, i.e., one second earlier, so I think you have to subtract 1s to get the correct transition time for negative leap seconds.
There was a problem hiding this comment.
Not having negative leap seconds indeed makes it hard to determine their proper behaviour. The Standard also is quite silent on them; I can imagine why. I'll consider your point and maybe search for other references.
There was a problem hiding this comment.
Paul Eggert, currently one of the official TZ Coordinators, has this message on the tz mailing list concerning negative leap seconds. It contains a sample leap-seconds.list file. You can see that the NTP timestamps all end in "00", suggesting they refer to 00:00:00, and indeed they are all multiples of 86,400.
| } | ||
|
|
||
| template <class CharT> | ||
| static void test_utc_transitions() { |
There was a problem hiding this comment.
If you can customize the leap second db like in some of the other tests, it might be useful to verify that a negative leap second has the correct behavior, i.e., "23:59:58" is followed by "00:00:00".
There was a problem hiding this comment.
I thought I had such a test, but seems only with my own faked data. I'll look at adding such a test and I want to look at using the fake numbers Paul Eggert generated. Thanks for that link!
mordante
commented
May 1, 2024
|
|
||
| sys_time<common_type_t<_Duration, seconds>> __result{__time.time_since_epoch() - __info.elapsed}; | ||
| if (__info.is_leap_second) | ||
| return chrono::floor<seconds>(__result) + chrono::seconds{1} - _Dp{1}; |
There was a problem hiding this comment.
Good point. I'll need to look into this.
| // epoch + transition time in the database + leap seconds before the transition. | ||
| test_transition(epoch + 60s + 0s, 0s, true); | ||
| test_transition(epoch + 120s + 1s, 1s, true); | ||
| test_transition(epoch + 180s + 2s, 2s, false); |
There was a problem hiding this comment.
Not having negative leap seconds indeed makes it hard to determine their proper behaviour. The Standard also is quite silent on them; I can imagine why. I'll consider your point and maybe search for other references.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While implementing this feature and its associated LWG issues it turns out
Implements parts of: