[libc++][chrono] Loads leap-seconds.list in tzdb. #82113
Conversation
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Mark de Wever (mordante) ChangesThis implements the loading of the leap-seconds.list file and store its contents in the tzdb struct. This adds the required The class leap_seconds is fully implemented including its non-member functions. Implements parts of:
Implements:
Patch is 39.88 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/82113.diff 23 Files Affected:
diff --git a/libcxx/docs/Status/Cxx20Issues.csv b/libcxx/docs/Status/Cxx20Issues.csv
index f0e9c4090f9cf6..db57b15256a62f 100644
--- a/libcxx/docs/Status/Cxx20Issues.csv
+++ b/libcxx/docs/Status/Cxx20Issues.csv
@@ -269,7 +269,7 @@
"`3355 <https://wg21.link/LWG3355>`__","The memory algorithms should support move-only input iterators introduced by P1207","Prague","|Complete|","15.0","|ranges|"
"`3356 <https://wg21.link/LWG3356>`__","``__cpp_lib_nothrow_convertible``\ should be ``__cpp_lib_is_nothrow_convertible``\ ","Prague","|Complete|","12.0"
"`3358 <https://wg21.link/LWG3358>`__","|sect|\ [span.cons] is mistaken that ``to_address``\ can throw","Prague","|Complete|","17.0"
-"`3359 <https://wg21.link/LWG3359>`__","``<chrono>``\ leap second support should allow for negative leap seconds","Prague","","","|chrono|"
+"`3359 <https://wg21.link/LWG3359>`__","``<chrono>``\ leap second support should allow for negative leap seconds","Prague","|Complete|","19.0","|chrono|"
"`3360 <https://wg21.link/LWG3360>`__","``three_way_comparable_with``\ is inconsistent with similar concepts","Prague","|Nothing To Do|","","|spaceship|"
"`3362 <https://wg21.link/LWG3362>`__","Strike ``stop_source``\ 's ``operator!=``\ ","Prague","",""
"`3363 <https://wg21.link/LWG3363>`__","``drop_while_view``\ should opt-out of ``sized_range``\ ","Prague","|Nothing To Do|","","|ranges|"
@@ -286,7 +286,7 @@
"`3380 <https://wg21.link/LWG3380>`__","``common_type``\ and comparison categories","Prague","|Complete|","15.0","|spaceship|"
"`3381 <https://wg21.link/LWG3381>`__","``begin``\ and ``data``\ must agree for ``contiguous_range``\ ","Prague","|Nothing To Do|","","|ranges|"
"`3382 <https://wg21.link/LWG3382>`__","NTTP for ``pair``\ and ``array``\ ","Prague","",""
-"`3383 <https://wg21.link/LWG3383>`__","|sect|\ [time.zone.leap.nonmembers] ``sys_seconds``\ should be replaced with ``seconds``\ ","Prague","","","|chrono|"
+"`3383 <https://wg21.link/LWG3383>`__","|sect|\ [time.zone.leap.nonmembers] ``sys_seconds``\ should be replaced with ``seconds``\ ","Prague","|Complete|","19.0","|chrono|"
"`3384 <https://wg21.link/LWG3384>`__","``transform_view::*sentinel*``\ has an incorrect ``operator-``\ ","Prague","|Complete|","15.0","|ranges|"
"`3385 <https://wg21.link/LWG3385>`__","``common_iterator``\ is not sufficiently constrained for non-copyable iterators","Prague","|Complete|","15.0","|ranges|"
"`3387 <https://wg21.link/LWG3387>`__","|sect|\ [range.reverse.view] ``reverse_view<V>``\ unintentionally requires ``range<const V>``\ ","Prague","|Complete|","15.0","|ranges|"
diff --git a/libcxx/docs/Status/Cxx20Papers.csv b/libcxx/docs/Status/Cxx20Papers.csv
index db6491419a5cc3..77078b11a72780 100644
--- a/libcxx/docs/Status/Cxx20Papers.csv
+++ b/libcxx/docs/Status/Cxx20Papers.csv
@@ -179,7 +179,7 @@
"`P1970R2 <https://wg21.link/P1970R2>`__","LWG","Consistency for size() functions: Add ranges::ssize","Prague","|Complete|","15.0","|ranges|"
"`P1973R1 <https://wg21.link/P1973R1>`__","LWG","Rename ""_default_init"" Functions, Rev1","Prague","|Complete|","16.0"
"`P1976R2 <https://wg21.link/P1976R2>`__","LWG","Fixed-size span construction from dynamic range","Prague","|Complete|","11.0","|ranges|"
-"`P1981R0 <https://wg21.link/P1981R0>`__","LWG","Rename leap to leap_second","Prague","* *",""
+"`P1981R0 <https://wg21.link/P1981R0>`__","LWG","Rename leap to leap_second","Prague","|Complete|","19.0","|chrono|"
"`P1982R0 <https://wg21.link/P1982R0>`__","LWG","Rename link to time_zone_link","Prague","|Complete|","19.0","|chrono|"
"`P1983R0 <https://wg21.link/P1983R0>`__","LWG","Wording for GB301, US296, US292, US291, and US283","Prague","|Complete|","15.0","|ranges|"
"`P1994R1 <https://wg21.link/P1994R1>`__","LWG","elements_view needs its own sentinel","Prague","|Complete|","16.0","|ranges|"
diff --git a/libcxx/docs/Status/SpaceshipProjects.csv b/libcxx/docs/Status/SpaceshipProjects.csv
index c8221078e9a8df..3d14f487d9a910 100644
--- a/libcxx/docs/Status/SpaceshipProjects.csv
+++ b/libcxx/docs/Status/SpaceshipProjects.csv
@@ -173,7 +173,7 @@ Section,Description,Dependencies,Assignee,Complete
| `year_month_weekday_last <https://reviews.llvm.org/D152699>`_",None,Hristo Hristov,|Complete|
`[time.zone.nonmembers] <https://wg21.link/time.zone.nonmembers>`_,"`chrono::time_zone`",A ``<chrono>`` implementation,Mark de Wever,|Complete|
`[time.zone.zonedtime.nonmembers] <https://wg21.link/time.zone.zonedtime.nonmembers>`_,"`chrono::zoned_time`",A ``<chrono>`` implementation,Mark de Wever,|In Progress|
-`[time.zone.leap.nonmembers] <https://wg21.link/time.zone.leap.nonmembers>`_,"`chrono::time_leap_seconds`",A ``<chrono>`` implementation,Mark de Wever,|In Progress|
+`[time.zone.leap.nonmembers] <https://wg21.link/time.zone.leap.nonmembers>`_,"`chrono::time_leap_seconds`",A ``<chrono>`` implementation,Mark de Wever,|Complete|
`[time.zone.link.nonmembers] <https://wg21.link/time.zone.link.nonmembers>`_,"`chrono::time_zone_link`",A ``<chrono>`` implementation,Mark de Wever,|Complete|
- `5.13 Clause 28: Localization library <https://wg21.link/p1614r2#clause-28-localization-library>`_,,,,
"| `[locale] <https://wg21.link/locale>`_
diff --git a/libcxx/include/CMakeLists.txt b/libcxx/include/CMakeLists.txt
index fa3b464e56c4d0..e550e6bfcdefd3 100644
--- a/libcxx/include/CMakeLists.txt
+++ b/libcxx/include/CMakeLists.txt
@@ -281,6 +281,7 @@ set(files
__chrono/formatter.h
__chrono/hh_mm_ss.h
__chrono/high_resolution_clock.h
+ __chrono/leap_second.h
__chrono/literals.h
__chrono/month.h
__chrono/month_weekday.h
diff --git a/libcxx/include/__chrono/leap_second.h b/libcxx/include/__chrono/leap_second.h
new file mode 100644
index 00000000000000..7c1f5ff72498e4
--- /dev/null
+++ b/libcxx/include/__chrono/leap_second.h
@@ -0,0 +1,112 @@
+// -*- 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
+//
+//===----------------------------------------------------------------------===//
+
+// For information see https://libcxx.llvm.org/DesignDocs/TimeZone.html
+
+#ifndef _LIBCPP___CHRONO_LEAP_SECOND_H
+#define _LIBCPP___CHRONO_LEAP_SECOND_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 <__compare/ordering.h>
+# include <__compare/three_way_comparable.h>
+# include <__config>
+
+# if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+# pragma GCC system_header
+# endif
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+# if _LIBCPP_STD_VER >= 20
+
+namespace chrono {
+
+class leap_second {
+public:
+ struct __constructor_tag;
+ [[nodiscard]]
+ _LIBCPP_HIDE_FROM_ABI explicit constexpr leap_second(__constructor_tag&&, sys_seconds __date, seconds __value)
+ : __date_(__date), __value_(__value) {}
+
+ _LIBCPP_HIDE_FROM_ABI leap_second(const leap_second&) = default;
+ _LIBCPP_HIDE_FROM_ABI leap_second& operator=(const leap_second&) = default;
+
+ _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr sys_seconds date() const noexcept { return __date_; }
+
+ _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr seconds value() const noexcept { return __value_; }
+
+private:
+ sys_seconds __date_;
+ seconds __value_;
+};
+
+_LIBCPP_HIDE_FROM_ABI constexpr bool operator==(const leap_second& __x, const leap_second& __y) {
+ return __x.date() == __y.date();
+}
+_LIBCPP_HIDE_FROM_ABI constexpr strong_ordering operator<=>(const leap_second& __x, const leap_second& __y) {
+ return __x.date() <=> __y.date();
+}
+
+template <class _Duration>
+_LIBCPP_HIDE_FROM_ABI constexpr bool operator==(const leap_second& __x, const sys_time<_Duration>& __y) {
+ return __x.date() == __y;
+}
+template <class _Duration>
+_LIBCPP_HIDE_FROM_ABI constexpr bool operator<(const leap_second& __x, const sys_time<_Duration>& __y) {
+ return __x.date() < __y;
+}
+template <class _Duration>
+_LIBCPP_HIDE_FROM_ABI constexpr bool operator<(const sys_time<_Duration>& __x, const leap_second& __y) {
+ return __x < __y.date();
+}
+template <class _Duration>
+_LIBCPP_HIDE_FROM_ABI constexpr bool operator>(const leap_second& __x, const sys_time<_Duration>& __y) {
+ return __y < __x;
+}
+template <class _Duration>
+_LIBCPP_HIDE_FROM_ABI constexpr bool operator>(const sys_time<_Duration>& __x, const leap_second& __y) {
+ return __y < __x;
+}
+template <class _Duration>
+_LIBCPP_HIDE_FROM_ABI constexpr bool operator<=(const leap_second& __x, const sys_time<_Duration>& __y) {
+ return !(__y < __x);
+}
+template <class _Duration>
+_LIBCPP_HIDE_FROM_ABI constexpr bool operator<=(const sys_time<_Duration>& __x, const leap_second& __y) {
+ return !(__y < __x);
+}
+template <class _Duration>
+_LIBCPP_HIDE_FROM_ABI constexpr bool operator>=(const leap_second& __x, const sys_time<_Duration>& __y) {
+ return !(__x < __y);
+}
+template <class _Duration>
+_LIBCPP_HIDE_FROM_ABI constexpr bool operator>=(const sys_time<_Duration>& __x, const leap_second& __y) {
+ return !(__x < __y);
+}
+template <class _Duration>
+ requires three_way_comparable_with<sys_seconds, sys_time<_Duration>>
+_LIBCPP_HIDE_FROM_ABI constexpr auto operator<=>(const leap_second& __x, const sys_time<_Duration>& __y) {
+ return __x.date() <=> __y;
+}
+
+} // namespace chrono
+
+# endif //_LIBCPP_STD_VER >= 20
+
+_LIBCPP_END_NAMESPACE_STD
+
+#endif // !defined(_LIBCPP_HAS_NO_INCOMPLETE_TZDB)
+
+#endif // _LIBCPP___CHRONO_LEAP_SECOND_H
diff --git a/libcxx/include/__chrono/tzdb.h b/libcxx/include/__chrono/tzdb.h
index 582172e5df9ddb..45c20f279f9c96 100644
--- a/libcxx/include/__chrono/tzdb.h
+++ b/libcxx/include/__chrono/tzdb.h
@@ -16,6 +16,7 @@
// Enable the contents of the header only when libc++ was built with experimental features enabled.
#if !defined(_LIBCPP_HAS_NO_INCOMPLETE_TZDB)
+# include <__chrono/leap_second.h>
# include <__chrono/time_zone.h>
# include <__chrono/time_zone_link.h>
# include <__config>
@@ -40,6 +41,8 @@ struct tzdb {
string version;
vector<time_zone> zones;
vector<time_zone_link> links;
+
+ vector<leap_second> leap_seconds;
};
} // namespace chrono
diff --git a/libcxx/include/chrono b/libcxx/include/chrono
index fe73f7c772b996..01ba15d97f3d20 100644
--- a/libcxx/include/chrono
+++ b/libcxx/include/chrono
@@ -688,6 +688,7 @@ struct tzdb {
string version;
vector<time_zone> zones;
vector<time_zone_link> links;
+ vector<leap_second> leap_seconds;
};
class tzdb_list { // C++20
@@ -731,6 +732,43 @@ class time_zone {
bool operator==(const time_zone& x, const time_zone& y) noexcept; // C++20
strong_ordering operator<=>(const time_zone& x, const time_zone& y) noexcept; // C++20
+// [time.zone.leap], leap second support
+class leap_second { // C++20
+public:
+ leap_second(const leap_second&) = default;
+ leap_second& operator=(const leap_second&) = default;
+
+ // unspecified additional constructors
+
+ constexpr sys_seconds date() const noexcept;
+ constexpr seconds value() const noexcept;
+};
+
+constexpr bool operator==(const leap_second& x, const leap_second& y); // C++20
+constexpr strong_ordering operator<=>(const leap_second& x, const leap_second& y);
+
+template<class Duration> // C++20
+ constexpr bool operator==(const leap_second& x, const sys_time<Duration>& y);
+template<class Duration> // C++20
+ constexpr bool operator< (const leap_second& x, const sys_time<Duration>& y);
+template<class Duration> // C++20
+ constexpr bool operator< (const sys_time<Duration>& x, const leap_second& y);
+template<class Duration> // C++20
+ constexpr bool operator> (const leap_second& x, const sys_time<Duration>& y);
+template<class Duration> // C++20
+ constexpr bool operator> (const sys_time<Duration>& x, const leap_second& y);
+template<class Duration> // C++20
+ constexpr bool operator<=(const leap_second& x, const sys_time<Duration>& y);
+template<class Duration> // C++20
+ constexpr bool operator<=(const sys_time<Duration>& x, const leap_second& y);
+template<class Duration> // C++20
+ constexpr bool operator>=(const leap_second& x, const sys_time<Duration>& y);
+template<class Duration> // C++20
+ constexpr bool operator>=(const sys_time<Duration>& x, const leap_second& y);
+template<class Duration> // C++20
+ requires three_way_comparable_with<sys_seconds, sys_time<Duration>>
+ constexpr auto operator<=>(const leap_second& x, const sys_time<Duration>& y);
+
// [time.zone.link], class time_zone_link
class time_zone_link { // C++20
public:
@@ -863,6 +901,7 @@ constexpr chrono::year operator ""y(unsigned lo
#if !defined(_LIBCPP_HAS_NO_TIME_ZONE_DATABASE) && !defined(_LIBCPP_HAS_NO_FILESYSTEM) && \
!defined(_LIBCPP_HAS_NO_LOCALIZATION)
+# include <__chrono/leap_second.h>
# include <__chrono/time_zone.h>
# include <__chrono/time_zone_link.h>
# include <__chrono/tzdb.h>
diff --git a/libcxx/include/libcxx.imp b/libcxx/include/libcxx.imp
index fbe09fa5e54ab1..aeeb970433fc4a 100644
--- a/libcxx/include/libcxx.imp
+++ b/libcxx/include/libcxx.imp
@@ -278,6 +278,7 @@
{ include: [ "<__chrono/formatter.h>", "private", "<chrono>", "public" ] },
{ include: [ "<__chrono/hh_mm_ss.h>", "private", "<chrono>", "public" ] },
{ include: [ "<__chrono/high_resolution_clock.h>", "private", "<chrono>", "public" ] },
+ { include: [ "<__chrono/leap_second.h>", "private", "<chrono>", "public" ] },
{ include: [ "<__chrono/literals.h>", "private", "<chrono>", "public" ] },
{ include: [ "<__chrono/month.h>", "private", "<chrono>", "public" ] },
{ include: [ "<__chrono/month_weekday.h>", "private", "<chrono>", "public" ] },
diff --git a/libcxx/include/module.modulemap.in b/libcxx/include/module.modulemap.in
index e72136a58c0b1b..4f106d0a6c7e4a 100644
--- a/libcxx/include/module.modulemap.in
+++ b/libcxx/include/module.modulemap.in
@@ -1141,6 +1141,7 @@ module std_private_chrono_high_resolution_clock [system] {
export std_private_chrono_steady_clock
export std_private_chrono_system_clock
}
+module std_private_chrono_leap_second [system] { header "__chrono/leap_second.h" }
module std_private_chrono_literals [system] { header "__chrono/literals.h" }
module std_private_chrono_month [system] { header "__chrono/month.h" }
module std_private_chrono_month_weekday [system] { header "__chrono/month_weekday.h" }
diff --git a/libcxx/modules/std/chrono.inc b/libcxx/modules/std/chrono.inc
index 109023a3abc25f..2c0bd3f98a67d7 100644
--- a/libcxx/modules/std/chrono.inc
+++ b/libcxx/modules/std/chrono.inc
@@ -208,10 +208,7 @@ export namespace std {
using std::chrono::reload_tzdb;
using std::chrono::remote_version;
-# endif // !defined(_LIBCPP_HAS_NO_TIME_ZONE_DATABASE) && !defined(_LIBCPP_HAS_NO_FILESYSTEM) &&
- // !defined(_LIBCPP_HAS_NO_LOCALIZATION)
-
-# if 0
+# if 0
// [time.zone.exception], exception classes
using std::chrono::ambiguous_local_time;
using std::chrono::nonexistent_local_time;
@@ -221,11 +218,11 @@ export namespace std {
// [time.zone.timezone], class time_zone
using std::chrono::choose;
-# endif
-# ifdef _LIBCPP_ENABLE_EXPERIMENTAL
+# endif // if 0
+
using std::chrono::time_zone;
-# endif
-# if 0
+
+# if 0
// [time.zone.zonedtraits], class template zoned_traits
using std::chrono::zoned_traits;
@@ -234,22 +231,23 @@ export namespace std {
using std::chrono::zoned_time;
using std::chrono::zoned_seconds;
+# endif // if 0
// [time.zone.leap], leap second support
using std::chrono::leap_second;
-# endif
-# ifdef _LIBCPP_ENABLE_EXPERIMENTAL
// [time.zone.link], class time_zone_link
using std::chrono::time_zone_link;
-# endif
-# if 0
+# if 0
// [time.format], formatting
using std::chrono::local_time_format;
-# endif
-#endif // _LIBCPP_ENABLE_EXPERIMENTAL
- } // namespace chrono
+# endif
+# endif // _LIBCPP_ENABLE_EXPERIMENTAL
+#endif // !defined(_LIBCPP_HAS_NO_TIME_ZONE_DATABASE) && !defined(_LIBCPP_HAS_NO_FILESYSTEM) &&
+ // !defined(_LIBCPP_HAS_NO_LOCALIZATION)
+
+ } // namespace chrono
#ifndef _LIBCPP_HAS_NO_LOCALIZATION
using std::formatter;
diff --git a/libcxx/src/CMakeLists.txt b/libcxx/src/CMakeLists.txt
index cc6954a7bac37e..428770cba7138f 100644
--- a/libcxx/src/CMakeLists.txt
+++ b/libcxx/src/CMakeLists.txt
@@ -336,6 +336,7 @@ endif()
if (LIBCXX_ENABLE_LOCALIZATION AND LIBCXX_ENABLE_FILESYSTEM AND LIBCXX_ENABLE_TIME_ZONE_DATABASE)
list(APPEND LIBCXX_EXPERIMENTAL_SOURCES
+ include/tzdb/leap_second_private.h
include/tzdb/time_zone_link_private.h
include/tzdb/time_zone_private.h
include/tzdb/types_private.h
diff --git a/libcxx/src/include/tzdb/leap_second_private.h b/libcxx/src/include/tzdb/leap_second_private.h
new file mode 100644
index 00000000000000..7a811ab1975942
--- /dev/null
+++ b/libcxx/src/include/tzdb/leap_second_private.h
@@ -0,0 +1,27 @@
+// -*- 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
+//
+//===----------------------------------------------------------------------===//
+
+// For information see https://libcxx.llvm.org/DesignDocs/TimeZone.html
+
+#ifndef _LIBCPP_SRC_INCLUDE_TZDB_LEAP_SECOND_PRIVATE_H
+#define _LIBCPP_SRC_INCLUDE_TZDB_LEAP_SECOND_PRIVATE_H
+
+#include <chrono>
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+namespace chrono {
+
+struct leap_second::__constructor_tag {};
+
+} // namespace chrono
+
+_LIBCPP_END_NAMESPACE_STD
+
+#endif // _LIBCPP_SRC_INCLUDE_TZDB_LEAP_SECOND_PRIVATE_H
diff --git a/libcxx/src/tzdb.cpp b/libcxx/src/tzdb.cpp
index 2bb801e48694ee..f1b72d57d7d5c7 100644
--- a/libcxx/src/tzdb.cpp
+++ b/libcxx/src/tzdb.cpp
@@ -15,6 +15,7 @@
#include <stdexcept>
#include <string>
+#include "include/tzdb/leap_second_private.h"
#include "include/tzdb/time_zone_link_private.h"
#include "include/tzdb/time_zone_private.h"
#include "include/tzdb/types_private.h"
@@ -603,6 +604,36 @@ static void __parse_tzdata(tzdb& __db, __tz::__rules_storage_type& __rules, istr
}
}
+static void __parse_leap_seconds(vector<leap_second>& __leap_seconds, istream&& __input) {
+ // The file stores dates since 1 January 1900, 00:00:00, we want
+ // seconds since 1 January 1970.
+ constexpr auto __offset = sys_days{1970y / January / 1} - sys_days{1900y / January / 1};
+
+ while (true) {
+ switch (__input.peek()) {
+ case istream::traits_type::eof():
+ return;
+
+ case ' ':
+ case '\t':
+ case '\n':
+ __input.get();
+ continue;
+
+ case '#':
+ chrono::__skip_line(__input);
+ continue;
+ }
+
+ sys_seconds __date = sys_seconds{seconds{chrono::__parse_integral(__input, false)}} - __offset;
+ chrono::__skip_mandatory_whitespace(__input);
+ seconds __value{chrono::__parse_integral(__input, false)};
+ chrono::__skip_line(__input);
+
+ __leap_seconds.emplace_back(leap_second::__constructor_tag{}, __date, __value);
+ }
+}
+
void __init_tzdb(tzdb& __tzdb, __tz::__rules_storage_type& __rules) {
filesystem::path ...
[truncated]
|
mordante
force-pushed
the
users/mordante/implements_leap_seconds
branch
from
3c588bd
to
3710cf0
Compare
libcxx/test/std/time/time.zone/time.zone.db/time.zone.db.access/get_tzdb.pass.cpp
Show resolved
Hide resolved
libcxx/test/std/time/time.zone/time.zone.leap/cons.copy.pass.cpp
Outdated
Show resolved
Hide resolved
libcxx/test/std/time/time.zone/time.zone.leap/assign.copy.pass.cpp
Outdated
Show resolved
Hide resolved
libcxx/test/libcxx/time/time.zone/time.zone.db/leap_seconds.pass.cpp
Outdated
Show resolved
Hide resolved
mordante
force-pushed
the
users/mordante/enables_tzdb_tests
branch
3 times, most recently
from
f36336a
to
90bdb99
Compare
mordante
force-pushed
the
users/mordante/implements_leap_seconds
branch
from
3710cf0
to
ed85003
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
libcxx/test/std/time/time.zone/time.zone.leap/nonmembers/comparison.pass.cpp
Show resolved
Hide resolved
libcxx/test/std/time/time.zone/time.zone.leap/assign.copy.pass.cpp
Outdated
Show resolved
Hide resolved
mordante
force-pushed
the
users/mordante/implements_leap_seconds
branch
from
ed85003
to
6ba3dd2
Compare
libcxx/test/std/time/time.zone/time.zone.leap/nonmembers/comparison.pass.cpp
Show resolved
Hide resolved
libcxx/test/std/time/time.zone/time.zone.leap/nonmembers/comparison.pass.cpp
Outdated
Show resolved
Hide resolved
libcxx/test/std/time/time.zone/time.zone.leap/nonmembers/comparison.pass.cpp
Outdated
Show resolved
Hide resolved
libcxx/test/std/time/time.zone/time.zone.leap/nonmembers/comparison.pass.cpp
Show resolved
Hide resolved
libcxx/test/std/time/time.zone/time.zone.leap/assign.copy.pass.cpp
Outdated
Show resolved
Hide resolved
mordante
force-pushed
the
users/mordante/implements_leap_seconds
branch
2 times, most recently
from
dc67a51
to
b9094b8
Compare
libcxx/test/std/time/time.zone/time.zone.db/leap_seconds.pass.cpp
Outdated
Show resolved
Hide resolved
libcxx/test/std/time/time.zone/time.zone.db/leap_seconds.pass.cpp
Outdated
Show resolved
Hide resolved
libcxx/test/std/time/time.zone/time.zone.leap/nonmembers/comparison.pass.cpp
Outdated
Show resolved
Hide resolved
libcxx/test/std/time/time.zone/time.zone.leap/nonmembers/comparison.pass.cpp
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
I think we should split out the interface for parsing TZDB-type strings.
I've noticed a number of complications arising from testing TZDB internals using only the public standard interface. The public interface is simply insufficient to allow proper testing and validation of the libraries internals.
I think we should split our concerns into two, They are:
- Implement & Test the C++ Standard interface.
- Implement a secure, platform/system agnostic TZDB parser/adapter.
The latter of the two problems is much hairier of a task. There are a number of formats, each with a number of revisions, that we need the parser to accept. And more importantly, there's an infinite number of inputs we need the parser to securely reject.
I say "securely" reject because the time zone database parser provides a potential attack vector for any attacker with some control over the bytes we read from disk.
So validation using both unit tests and fuzzing tests is essential.
We can achieve this testibility without affecting the public interface by abstracting out the parser into a separate testable unit.
The remaining tests, for conformance of the public API, would look almost identical to what we have now, except with fewer places needing to reference internal implementation details.
libcxx/test/std/time/time.zone/time.zone.leap/nonmembers/comparison.pass.cpp
Outdated
Show resolved
Hide resolved
libcxx/test/std/time/time.zone/time.zone.leap/assign.copy.pass.cpp
Outdated
Show resolved
Hide resolved
libcxx/test/std/time/time.zone/time.zone.leap/nonmembers/comparison.pass.cpp
Show resolved
Hide resolved
Instead of including a relative path use an absolute path based on the available lit substitution. This makes it easier to understand what is included and moving the test to a different directory level no longer breaks the test. This is based on a question by @EricWF in llvm#82113.
libcxx/test/std/time/time.zone/time.zone.db/leap_seconds.pass.cpp
Outdated
Show resolved
Hide resolved
libcxx/test/std/time/time.zone/time.zone.leap/assign.copy.pass.cpp
Outdated
Show resolved
Hide resolved
libcxx/test/std/time/time.zone/time.zone.leap/nonmembers/comparison.pass.cpp
Outdated
Show resolved
Hide resolved
libcxx/test/std/time/time.zone/time.zone.leap/nonmembers/comparison.pass.cpp
Show resolved
Hide resolved
This implements the loading of the leap-seconds.list file and store its contents in the tzdb struct. This adds the required `leap_seconds` member. The class leap_seconds is fully implemented including its non-member functions. Implements parts of: - P0355 Extending <chrono> to Calendars and Time Zones - P1614 The Mothership has Landed Implements: - P1981 Rename leap to leap_second - LWG3359 <chrono> leap second support should allow for negative leap seconds - LWG3383 §[time.zone.leap.nonmembers] sys_seconds should be replaced with seconds
mordante
force-pushed
the
users/mordante/implements_leap_seconds
branch
from
b9094b8
to
51e295f
Compare
There was a problem hiding this comment.
I think we should split out the interface for parsing TZDB-type strings.
I've noticed a number of complications arising from testing TZDB internals using only the public standard interface. The public interface is simply insufficient to allow proper testing and validation of the libraries internals.
I think we should split our concerns into two, They are:
1. Implement & Test the C++ Standard interface. 2. Implement a secure, platform/system agnostic TZDB parser/adapter.The latter of the two problems is much hairier of a task. There are a number of formats, each with a number of revisions, that we need the parser to accept. And more importantly, there's an infinite number of inputs we need the parser to securely reject.
The binary format files have different revisions. From what I can tell we only need to parse one revision for the text format.
I say "securely" reject because the time zone database parser provides a potential attack vector for any attacker with some control over the bytes we read from disk. So validation using both unit tests and fuzzing tests is essential.
We can achieve this testibility without affecting the public interface by abstracting out the parser into a separate testable unit.
That would give a public symbol to the parser. I kept it all internal so we can adapt the code if the IANA database changes its format. Currently I need to parse two files, but who knows what happens in a few years time.
Since we can already feed hand-crafted databases to these tests I think we don't need to change anything for the testability.
The remaining tests, for conformance of the public API, would look almost identical to what we have now, except with fewer places needing to reference internal implementation details.
I'm not sure what you exactly mean. There are public tests that don't use internals. There libc++ specific tests that use the internals. These private tests test boundary conditions. I think it would be too much effort to manually validate every permutation; we should use fuzz tests instead.
|
Code review is a consensus building activity. There are two or more engineers, both with different skills and experience, and the goal is utilize the knowledge of both parties to validate changes. I have not always been easy to make concede and I'm working to improve that. I would appreciate a similar effort on your part.
We should meet in private to discuss the security issues further.
You misunderstand me. The point of separating out the interface is to avoid public symbols in the library. When testing the parser, we would compile the source code into Here's how you do it without exposing any public symbols: // src/tzdb_parser.hpp
// We can add a mechanism to ensure this is only included by one source file in the build.
namespace { // anonymous namespace, internal linkage
struct Parser {
// implementation
};
}
// src/tzdb.cpp
#include "tzdb_parser.hpp"
// This is the only source file which uses the header (outside of the tests).
// test/libcxx/test_tzdb_parser.pass.cpp
// INCLUDE_DIRECTORIES: %LIBCXX_ROOT/
#include "src/tzdb_parser.hpp" // Do this in as many tests as you want.
void test() {
TZDBParser Parser;
// tests...
}Since exporting private symbols is a concern, you can use this method to remove the private symbols TZDB already exports like
Your right, the What I meant to say is that you can write tests for the internals that don't reference the public API or even private bits of namespace |
I agree and try that too. I'll reach out to you on Discord to discuss this further. FYI TZDB at the moment intentionally experimental so we can change the names of exports without fearing an ABI break. |
Instead of including a relative path use an absolute path based on the available lit substitution. This makes it easier to understand what is included and moving the test to a different directory level no longer breaks the test. This is based on a question by @EricWF in llvm/llvm-project#82113. NOKEYCHECK=True GitOrigin-RevId: 6aa53888a8e8a6e3f0bd279539703f4d4701b4e7
This implements the loading of the leap-seconds.list file and store its contents in the tzdb struct.
This adds the required
leap_secondsmember.The class leap_seconds is fully implemented including its non-member functions.
Implements parts of:
Implements: