[libc++][chrono] Loads tzdata.zi in tzdb. #74928
Merged
Conversation
This file contains hidden or 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) ChangesThis implements the loading of the tzdata.zi file and store its contents in the tzdb struct. This adds all required members except:
The class time_zone is incomplete and only contains the parts needed for storing the parsed data. The class time_zone_link is fully implemented including its non-member functions. Implements parts of:
Implements:
Patch is 91.90 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/74928.diff 31 Files Affected:
diff --git a/libcxx/docs/Status/Cxx20Papers.csv b/libcxx/docs/Status/Cxx20Papers.csv
index 13c126c1ba8be..53d57acacf01a 100644
--- a/libcxx/docs/Status/Cxx20Papers.csv
+++ b/libcxx/docs/Status/Cxx20Papers.csv
@@ -180,7 +180,7 @@
"`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","* *",""
-"`P1982R0 <https://wg21.link/P1982R0>`__","LWG","Rename link to time_zone_link","Prague","* *",""
+"`P1982R0 <https://wg21.link/P1982R0>`__","LWG","Rename link to time_zone_link","Prague","|Complete|","18.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|"
"`P2002R1 <https://wg21.link/P2002R1>`__","CWG","Defaulted comparison specification cleanups","Prague","* *",""
diff --git a/libcxx/docs/Status/SpaceshipProjects.csv b/libcxx/docs/Status/SpaceshipProjects.csv
index aaa9278a50c1e..c8221078e9a8d 100644
--- a/libcxx/docs/Status/SpaceshipProjects.csv
+++ b/libcxx/docs/Status/SpaceshipProjects.csv
@@ -171,10 +171,10 @@ Section,Description,Dependencies,Assignee,Complete
| `month_weekday_last <https://reviews.llvm.org/D152699>`_
| `year_month_weekday <https://reviews.llvm.org/D152699>`_
| `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,|In Progress|
+`[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.link.nonmembers] <https://wg21.link/time.zone.link.nonmembers>`_,"`chrono::time_zone_link`",A ``<chrono>`` implementation,Mark de Wever,|In Progress|
+`[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>`_
| `[locale.operators] <https://wg21.link/locale.operators>`_",| remove ops `locale <https://reviews.llvm.org/D152654>`_,None,Hristo Hristov,|Complete|
diff --git a/libcxx/include/CMakeLists.txt b/libcxx/include/CMakeLists.txt
index d8faf6467b79a..a71d2f0a02b3f 100644
--- a/libcxx/include/CMakeLists.txt
+++ b/libcxx/include/CMakeLists.txt
@@ -287,6 +287,9 @@ set(files
__chrono/steady_clock.h
__chrono/system_clock.h
__chrono/time_point.h
+ __chrono/time_zone.h
+ __chrono/time_zone_link.h
+ __chrono/time_zone_types.h
__chrono/tzdb.h
__chrono/tzdb_list.h
__chrono/weekday.h
diff --git a/libcxx/include/__chrono/time_zone.h b/libcxx/include/__chrono/time_zone.h
new file mode 100644
index 0000000000000..7b6f689511a2c
--- /dev/null
+++ b/libcxx/include/__chrono/time_zone.h
@@ -0,0 +1,75 @@
+// -*- 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_TIME_ZONE_H
+#define _LIBCPP___CHRONO_TIME_ZONE_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/time_zone_types.h>
+# include <__compare/strong_order.h>
+# include <__config>
+# include <string>
+# include <string_view>
+# include <vector>
+
+# 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 _LIBCPP_AVAILABILITY_TZDB time_zone {
+public:
+ explicit time_zone(string&& __name) : __name_(std::move(__name)) {}
+
+ _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI string_view name() const noexcept { return __name_; }
+
+ [[nodiscard]] _LIBCPP_HIDE_FROM_ABI vector<__tz::__continuation>& __continuations() { return __continuations_; }
+ [[nodiscard]] _LIBCPP_HIDE_FROM_ABI const vector<__tz::__continuation>& __continuations() const {
+ return __continuations_;
+ }
+
+private:
+ string __name_;
+ // Note the first line has a name + __continuation, the other lines
+ // are just __continuations. So there is always at least one item in
+ // the vector.
+ vector<__tz::__continuation> __continuations_;
+};
+
+_LIBCPP_NODISCARD_EXT _LIBCPP_AVAILABILITY_TZDB _LIBCPP_HIDE_FROM_ABI inline bool
+operator==(const time_zone& __x, const time_zone& __y) noexcept {
+ return __x.name() == __y.name();
+}
+
+_LIBCPP_NODISCARD_EXT _LIBCPP_AVAILABILITY_TZDB _LIBCPP_HIDE_FROM_ABI inline strong_ordering
+operator<=>(const time_zone& __x, const time_zone& __y) noexcept {
+ return __x.name() <=> __y.name();
+}
+
+} // 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_TIME_ZONE_H
diff --git a/libcxx/include/__chrono/time_zone_link.h b/libcxx/include/__chrono/time_zone_link.h
new file mode 100644
index 0000000000000..42fd3bb63ca20
--- /dev/null
+++ b/libcxx/include/__chrono/time_zone_link.h
@@ -0,0 +1,72 @@
+// -*- 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_TIME_ZONE_LINK_H
+#define _LIBCPP___CHRONO_TIME_ZONE_LINK_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 <__compare/strong_order.h>
+# include <__config>
+# include <string>
+# include <string_view>
+
+# 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 _LIBCPP_AVAILABILITY_TZDB time_zone_link {
+public:
+ _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI explicit time_zone_link(string_view __name, string_view __target)
+ : __name_{__name}, __target_{__target} {}
+
+ time_zone_link(time_zone_link&&) = default;
+ time_zone_link& operator=(time_zone_link&&) = default;
+
+ _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI string_view name() const noexcept { return __name_; }
+ _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI string_view target() const noexcept { return __target_; }
+
+private:
+ string __name_;
+ // TODO TZDB instead of the name we can store the pointer to a zone. These
+ // pointers are immutable. This makes it possible to directly return a
+ // pointer in the time_zone in the 'locate_zone' function.
+ string __target_;
+};
+
+_LIBCPP_NODISCARD_EXT _LIBCPP_AVAILABILITY_TZDB _LIBCPP_HIDE_FROM_ABI inline bool
+operator==(const time_zone_link& __x, const time_zone_link& __y) noexcept {
+ return __x.name() == __y.name();
+}
+
+_LIBCPP_NODISCARD_EXT _LIBCPP_AVAILABILITY_TZDB _LIBCPP_HIDE_FROM_ABI inline strong_ordering
+operator<=>(const time_zone_link& __x, const time_zone_link& __y) noexcept {
+ return __x.name() <=> __y.name();
+}
+
+} // namespace chrono
+
+# endif //_LIBCPP_STD_VER >= 20
+
+_LIBCPP_END_NAMESPACE_STD
+
+#endif // !defined(_LIBCPP_HAS_NO_INCOMPLETE_TZDB)
+
+#endif // _LIBCPP___CHRONO_TIME_ZONE_LINK_H
diff --git a/libcxx/include/__chrono/time_zone_types.h b/libcxx/include/__chrono/time_zone_types.h
new file mode 100644
index 0000000000000..e8d7b7ddf8465
--- /dev/null
+++ b/libcxx/include/__chrono/time_zone_types.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_TIME_ZONE_TYPES_H
+#define _LIBCPP___CHRONO_TIME_ZONE_TYPES_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/day.h>
+# include <__chrono/duration.h>
+# include <__chrono/month.h>
+# include <__chrono/weekday.h>
+# include <__chrono/year.h>
+# include <__config>
+# include <string>
+# include <variant>
+
+# 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::__tz {
+
+// Sun>=8 first Sunday on or after the eighth
+// Sun<=25 last Sunday on or before the 25th
+struct _LIBCPP_AVAILABILITY_TZDB __constrained_weekday {
+ /* year_month_day operator()(year __year, month __month);*/ // needed but not implemented
+
+ weekday __weekday;
+ enum __comparison_t { __le, __ge } __comparison;
+ day __day;
+};
+
+// The on field has a few alternative presentations
+// 5 the fifth of the month
+// lastSun the last Sunday in the month
+// lastMon the last Monday in the month
+// Sun>=8 first Sunday on or after the eighth
+// Sun<=25 last Sunday on or before the 25th
+using __on = variant<day, weekday_last, __constrained_weekday>;
+
+enum class _LIBCPP_AVAILABILITY_TZDB __clock { __local, __standard, __universal };
+
+struct _LIBCPP_AVAILABILITY_TZDB __at {
+ seconds __time{0};
+ __tz::__clock __clock{__tz::__clock::__local};
+};
+
+struct _LIBCPP_AVAILABILITY_TZDB __save {
+ seconds __time;
+ bool __is_dst;
+};
+
+// The names of the fields match the fields of a Rule.
+struct _LIBCPP_AVAILABILITY_TZDB __rule {
+ year __from;
+ year __to;
+ month __in_month; // __in is a reserved name
+ __tz::__on __on;
+ __tz::__at __at;
+ __tz::__save __save;
+ string __letters;
+};
+
+struct _LIBCPP_AVAILABILITY_TZDB __continuation {
+ seconds __stdoff;
+
+ // The RULES is either a SAVE or a NAME.
+ // The size_t is used as cache. After loading the rules they are
+ // sorted and remain stable, then an index in the vector can be
+ // used.
+ // If this field contains - then standard time always
+ // applies. This is indicated by the monostate.
+ using __rules_t = variant<monostate, __tz::__save, string, size_t>;
+
+ __rules_t __rules;
+
+ string __format;
+ // TODO TZDB until can be contain more than just a year.
+ // Parts of the UNTIL, the optional parts are default initialized
+ // optional<year> __until_;
+ year __year = chrono::year::min();
+ month __in_month{January}; // __in is a reserved name
+ __tz::__on __on{chrono::day{1}};
+ __tz::__at __at{chrono::seconds{0}, __tz::__clock::__local};
+};
+
+} // namespace chrono::__tz
+
+# 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_TIME_ZONE_TYPES_H
diff --git a/libcxx/include/__chrono/tzdb.h b/libcxx/include/__chrono/tzdb.h
index bd7b05d478e50..fc230c550327d 100644
--- a/libcxx/include/__chrono/tzdb.h
+++ b/libcxx/include/__chrono/tzdb.h
@@ -16,7 +16,13 @@
// Enable the contents of the header only when libc++ was built with experimental features enabled.
#if !defined(_LIBCPP_HAS_NO_INCOMPLETE_TZDB)
+# include <__availability>
+# include <__chrono/time_zone.h>
+# include <__chrono/time_zone_link.h>
+# include <__chrono/time_zone_types.h>
+# include <__config>
# include <string>
+# include <vector>
# if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
# pragma GCC system_header
@@ -31,6 +37,9 @@ namespace chrono {
struct _LIBCPP_AVAILABILITY_TZDB tzdb {
string version;
+ vector<pair<string, vector<__tz::__rule>>> __rules;
+ vector<time_zone> zones;
+ vector<time_zone_link> links;
};
} // namespace chrono
diff --git a/libcxx/include/chrono b/libcxx/include/chrono
index b3ed9acc5e5de..8eaeabeae9c8b 100644
--- a/libcxx/include/chrono
+++ b/libcxx/include/chrono
@@ -682,6 +682,8 @@ constexpr hours make24(const hours& h, bool is_pm) noexcept;
// [time.zone.db], time zone database
struct tzdb { // C++20
string version;
+ vector<time_zone> zones;
+ vector<time_zone_link> links;
};
class tzdb_list { // C++20
@@ -712,15 +714,34 @@ tzdb_list& get_tzdb_list();
const tzdb& reload_tzdb(); // C++20
string remote_version(); // C++20
-// 25.10.5, class time_zone // C++20
+// 25.10.5, class time_zone // C++20
enum class choose {earliest, latest};
-class time_zone;
-bool operator==(const time_zone& x, const time_zone& y) noexcept;
-bool operator!=(const time_zone& x, const time_zone& y) noexcept;
-bool operator<(const time_zone& x, const time_zone& y) noexcept;
-bool operator>(const time_zone& x, const time_zone& y) noexcept;
-bool operator<=(const time_zone& x, const time_zone& y) noexcept;
-bool operator>=(const time_zone& x, const time_zone& y) noexcept;
+class time_zone {
+ time_zone(time_zone&&) = default;
+ time_zone& operator=(time_zone&&) = default;
+
+ // unspecified additional constructors
+
+ string_view name() const noexcept;
+};
+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.link], class time_zone_link
+class time_zone_link { // C++20
+public:
+ time_zone_link(time_zone_link&&) = default;
+ time_zone_link& operator=(time_zone_link&&) = default;
+
+ // unspecified additional constructors
+
+ string_view name() const noexcept;
+ string_view target() const noexcept;
+};
+
+bool operator==(const time_zone_link& x, const time_zone_link& y); // C++20
+strong_ordering operator<=>(const time_zone_link& x, const time_zone_link& y); // C++20
+
} // chrono
namespace std {
@@ -838,6 +859,9 @@ 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/time_zone.h>
+# include <__chrono/time_zone_link.h>
+# include <__chrono/time_zone_types.h>
# include <__chrono/tzdb.h>
# include <__chrono/tzdb_list.h>
#endif
diff --git a/libcxx/modules/std/chrono.inc b/libcxx/modules/std/chrono.inc
index 65dc973936c47..8ebe8d26065dc 100644
--- a/libcxx/modules/std/chrono.inc
+++ b/libcxx/modules/std/chrono.inc
@@ -221,7 +221,9 @@ export namespace std {
// [time.zone.timezone], class time_zone
using std::chrono::choose;
+# endif
using std::chrono::time_zone;
+# if 0
// [time.zone.zonedtraits], class template zoned_traits
using std::chrono::zoned_traits;
@@ -233,10 +235,12 @@ export namespace std {
// [time.zone.leap], leap second support
using std::chrono::leap_second;
+# endif
// [time.zone.link], class time_zone_link
using std::chrono::time_zone_link;
+# if 0
// [time.format], formatting
using std::chrono::local_time_format;
# endif
diff --git a/libcxx/src/tz.cpp b/libcxx/src/tz.cpp
index 4425f0e6b91bd..472cac136dd77 100644
--- a/libcxx/src/tz.cpp
+++ b/libcxx/src/tz.cpp
@@ -8,6 +8,7 @@
// For information see https://libcxx.llvm.org/DesignDocs/TimeZone.html
+#include <algorithm>
#include <chrono>
#include <filesystem>
#include <fstream>
@@ -49,6 +50,10 @@ _LIBCPP_WEAK string_view __libcpp_tzdb_directory() {
#endif
}
+//===----------------------------------------------------------------------===//
+// Details
+//===----------------------------------------------------------------------===//
+
[[nodiscard]] static bool __is_whitespace(int __c) { return __c == ' ' || __c == '\t'; }
static void __skip_optional_whitespace(istream& __input) {
@@ -63,6 +68,26 @@ static void __skip_mandatory_whitespace(istream& __input) {
chrono::__skip_optional_whitespace(__input);
}
+[[nodiscard]] static bool __is_eol(int __c) { return __c == '\n' || __c == std::char_traits<char>::eof(); }
+
+static void __skip_line(istream& __input) {
+ while (!chrono::__is_eol(__input.peek())) {
+ __input.get();
+ }
+ __input.get();
+}
+
+static void __skip(istream& __input, char __suffix) {
+ if (std::tolower(__input.peek()) == __suffix)
+ __input.get();
+}
+
+static void __skip(istream& __input, string_view __suffix) {
+ for (auto __c : __suffix)
+ if (std::tolower(__input.peek()) == __c)
+ __input.get();
+}
+
static void __matches(istream& __input, char __expected) {
if (std::tolower(__input.get()) != __expected)
std::__throw_runtime_error((string("corrupt tzdb: expected character '") + __expected + '\'').c_str());
@@ -96,6 +121,369 @@ static void __matches(istream& __input, string_view __expected) {
}
}
+[[nodiscard]] static int64_t __parse_integral(istream& __input, bool __leading_zero_allowed) {
+ int64_t __result = __input.get();
+ if (__leading_zero_allowed) {
+ if (__result < '0' || __result > '9')
+ std::__throw_runtime_error("corrupt tzdb: expected a digit");
+ } else {
+ if (__result < '1' || __result > '9')
+ std::__throw_runtime_error("corrupt tzdb: expected a non-zero digit");
+ }
+ __result -= '0';
+ while (true) {
+ if (__input.peek() < '0' || __input.peek() > '9')
+ return __result;
+
+ // In order to avoid possible overflows we limit the accepted range.
+ // Most values parsed are expected to be very small:
+ // - 8784 hours in a year
+ // - 31 days in a month
+ // - year no real maximum, these values are expected to be less than
+ // the range of the year type.
+ //
+ // However the leapseconds use a seconds after epoch value. Using an
+ // int would run into an overflow in 2038. By using a 64-bit value
+ // the range is large enough for the bilions of years. Limiting that
+ // range slightly to make the code easier is not an issue.
+ if (__result > (std::numeric_li...
[truncated]
|
mordante
force-pushed
the
GH-load_tzdata
branch
2 times, most recently
from
a6cadd2 to
4b696a9
Compare
mordante
commented
Jan 22, 2024
ldionne
reviewed
Jan 23, 2024
libcxx/include/__chrono/tzdb.h
Outdated
| @@ -31,6 +37,9 @@ namespace chrono { | |||
|
|
|||
| struct _LIBCPP_AVAILABILITY_TZDB tzdb { | |||
| string version; | |||
| vector<pair<string, vector<__tz::__rule>>> __rules; | |||
There was a problem hiding this comment.
What we really want here is std::flat_map<string, vector<__tz::__rule>>. I suggest we use std::map for now and switch it to flat_map once it is implemented. We could aim to have both in LLVM 19.
There was a problem hiding this comment.
Since we move the code to the dylib we can do this whenever we want. Since the code expects a vector and the move to the dylib loses test coverage I've only added a TODO and did not use std::map as a temporary measure.
| // Sun<=25 last Sunday on or before the 25th | ||
| using __on = variant<day, weekday_last, __constrained_weekday>; | ||
|
|
||
| enum class _LIBCPP_AVAILABILITY_TZDB __clock { __local, __standard, __universal }; |
There was a problem hiding this comment.
If those don't rely on anything in the dylib, we should not mark them with availability.
There was a problem hiding this comment.
Since I move the code to the dylib these can indeed be removed.
libcxx/include/__chrono/tzdb.h
Outdated
| @@ -31,6 +37,9 @@ namespace chrono { | |||
|
|
|||
| struct _LIBCPP_AVAILABILITY_TZDB tzdb { | |||
| string version; | |||
| vector<pair<string, vector<__tz::__rule>>> __rules; | |||
There was a problem hiding this comment.
Also, can we make this a private member?
There was a problem hiding this comment.
We can't. I've removed the vector here and added it to the pimpl of tzdb_list.
libcxx/src/tz.cpp
Outdated
| break; | ||
|
|
||
| case 'l': | ||
| chrono::__skip(__input, "link"); |
There was a problem hiding this comment.
Suggested change
| chrono::__skip(__input, "link"); | |
| chrono::__skip(__input, "ink"); |
I guess I'm going to be annoying and ask whether this is caught by a test.
There was a problem hiding this comment.
it's not. In the end I like to test with multiple real databases, especially for parsing to see whether they all pass. This also helps to catch things in the database that are contradict the specs.
ldionne
reviewed
Jan 30, 2024
libcxx/src/tz.cpp
Outdated
| __input.get(); | ||
|
|
||
| static_assert(year::min() == -year::max()); | ||
| int __result = std::min<int64_t>(__parse_integral(__input, true), static_cast<int>(year::max())); |
There was a problem hiding this comment.
If we parse an integral larger than year::max(), my understanding is that we'll silently read it as year::max() instead. Is there a reason for not throwing an exception here instead? Given that we have an implementation limit based on chrono::year, I think it makes sense to error out in some way at least. We could also assert here, I think that would be reasonable, but throwing an exception might provide a better user experience.
Silently clamping to year::max() seems like a really surprising behavior to me.
libcxx/src/tz.cpp
Outdated
| return __tz::__constrained_weekday::__ge; | ||
|
|
||
| case '<': | ||
| chrono::__matches(__input, '='); |
There was a problem hiding this comment.
I would suggest the following renames:
__skip -> __skip_optional
__matches -> __skip_exactly
I'm not 100% happy about those names either, but __matches sounds like something that would return a bool, not something that skips but requires an exact match.
Just food for thought.
libcxx/include/__chrono/time_zone.h
Outdated
Comment on lines
49
to
53
| string __name_; | ||
| // Note the first line has a name + __continuation, the other lines | ||
| // are just __continuations. So there is always at least one item in | ||
| // the vector. | ||
| vector<__tz::__continuation> __continuations_; |
There was a problem hiding this comment.
I wonder if this is the kind of place where we'd benefit in using a pimpl idiom just to hide the implementation details from the headers. This could result in more flexibility down the line, since this is not performance sensitive AFAIU.
|
|
||
| // For information see https://libcxx.llvm.org/DesignDocs/TimeZone.html | ||
|
|
||
| #ifndef _LIBCPP___CHRONO_TIME_ZONE_TYPES_H |
There was a problem hiding this comment.
As discussed during live review, maybe this whole header can be an implementation detail of the dylib if we use pimpl more?
There was a problem hiding this comment.
I've done this. It would be possible to add proper constructors to these classes instead of builder functions in the dylib. I want to do that at a later stage, just in case parts are needed in the header. (I don't expect that but the code still misses a lot of functionality.)
A disadvantage of moving the code to the dylib is that testing it is harder. At the moment it's quite impossible since the code that uses the data has not been implemented. For now I disabled the tests and left TODOs so I can revisit that later.
mordante
force-pushed
the
GH-load_tzdata
branch
2 times, most recently
from
c000e5e to
96dc101
Compare
ldionne
requested changes
Feb 6, 2024
libcxx/include/__chrono/time_zone.h
Outdated
| class _LIBCPP_AVAILABILITY_TZDB time_zone { | ||
| public: | ||
| class __impl; // public so it can be used by make_unique. | ||
| _LIBCPP_NODISCARD_EXT _LIBCPP_EXPORTED_FROM_ABI explicit time_zone(unique_ptr<__impl>&& __p); |
There was a problem hiding this comment.
I would avoid exporting a public constructor like this from the dylib, since that ties our API to our ABI in a really tight way. Instead, I would do this
// in the header
class time_zone {
private:
_LIBCPP_HIDE_FROM_ABI explicit time_zone(unique_ptr<__impl>&& __p) : __impl_(std::move(__p)) { }
public:
_LIBCPP_HIDE_FROM_ABI static time_zone __create(...); // declare only, don't define
// ...
};
// in the dylib
time_zone time_zone::__create(...) { ... }There was a problem hiding this comment.
This solution does not work, it requires the constructor to have the definition of __impl. Instead I added a private default constructor and an exported public static __create function. Since only the dylib knows the definition of __impl the public function can't be used externally.
libcxx/test/std/time/time.zone/time.zone.db/time.zone.db.tzdb/tzdb.members.pass.cpp
Outdated
Show resolved
Hide resolved
libcxx/test/std/time/time.zone/time.zone.db/time.zone.db.tzdb/tzdb.members.pass.cpp
Outdated
Show resolved
Hide resolved
libcxx/test/std/time/time.zone/time.zone.link/time.zone.link.members/name.pass.cpp
Outdated
Show resolved
Hide resolved
libcxx/test/std/time/time.zone/time.zone.timezone/types.compile.pass.cpp
Show resolved
Hide resolved
This implements the loading of the tzdata.zi file and store its contents in the tzdb struct. This adds all required members except: - the leap seconds, - the locate_zone, and - current_zone. The class time_zone is incomplete and only contains the parts needed for storing the parsed data. The class time_zone_link 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: - P1982 Rename link to time_zone_link
mordante
force-pushed
the
GH-load_tzdata
branch
from
96dc101 to
965058b
Compare
You can test this locally with the following command:git-clang-format --diff 803374994602910aae2cb483d03bcbdb294b21bb b5c1d3a633aa1a2b9639918f01a2032a54da801a -- libcxx/include/__chrono/time_zone.h libcxx/include/__chrono/time_zone_link.h libcxx/src/include/tzdb/time_zone_link_private.h libcxx/src/include/tzdb/time_zone_private.h libcxx/src/include/tzdb/types_private.h libcxx/src/include/tzdb/tzdb_list_private.h libcxx/src/include/tzdb/tzdb_private.h libcxx/src/time_zone.cpp libcxx/src/tzdb.cpp libcxx/test/libcxx/time/time.zone/time.zone.db/links.pass.cpp libcxx/test/libcxx/time/time.zone/time.zone.db/rules.pass.cpp libcxx/test/libcxx/time/time.zone/time.zone.db/zones.pass.cpp libcxx/test/std/time/time.zone/time.zone.link/time.zone.link.members/name.pass.cpp libcxx/test/std/time/time.zone/time.zone.link/time.zone.link.members/target.pass.cpp libcxx/test/std/time/time.zone/time.zone.link/time.zone.link.nonmembers/comparison.pass.cpp libcxx/test/std/time/time.zone/time.zone.link/types.compile.pass.cpp libcxx/test/std/time/time.zone/time.zone.timezone/time.zone.members/name.pass.cpp libcxx/test/std/time/time.zone/time.zone.timezone/time.zone.nonmembers/comparison.pass.cpp libcxx/test/std/time/time.zone/time.zone.timezone/types.compile.pass.cpp libcxx/include/__chrono/tzdb.h libcxx/include/__chrono/tzdb_list.h libcxx/include/chrono libcxx/modules/std/chrono.inc libcxx/src/tzdb_list.cpp libcxx/test/libcxx/diagnostics/chrono.nodiscard_extensions.compile.pass.cpp libcxx/test/libcxx/diagnostics/chrono.nodiscard_extensions.verify.cpp libcxx/test/std/time/time.zone/time.zone.db/time.zone.db.access/get_tzdb.pass.cpp libcxx/test/std/time/time.zone/time.zone.db/time.zone.db.access/get_tzdb_list.pass.cpp libcxx/test/std/time/time.zone/time.zone.db/time.zone.db.list/front.pass.cpp libcxx/test/std/time/time.zone/time.zone.db/time.zone.db.list/iterators.pass.cpp libcxx/test/std/time/time.zone/time.zone.db/time.zone.db.remote/reload_tzdb.pass.cpp libcxx/test/std/time/time.zone/time.zone.db/time.zone.db.remote/remote_version.pass.cpp libcxx/test/std/time/time.zone/time.zone.db/time.zone.db.tzdb/tzdb.members.pass.cppView the diff from clang-format here.diff --git a/libcxx/include/__chrono/time_zone_link.h b/libcxx/include/__chrono/time_zone_link.h
index 17e915d267..78cae33853 100644
--- a/libcxx/include/__chrono/time_zone_link.h
+++ b/libcxx/include/__chrono/time_zone_link.h
@@ -38,8 +38,8 @@ namespace chrono {
class time_zone_link {
public:
struct __constructor_tag;
- _LIBCPP_NODISCARD_EXT
- _LIBCPP_HIDE_FROM_ABI explicit time_zone_link(__constructor_tag&&, string_view __name, string_view __target)
+ _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI explicit time_zone_link(
+ __constructor_tag&&, string_view __name, string_view __target)
: __name_{__name}, __target_{__target} {}
_LIBCPP_HIDE_FROM_ABI time_zone_link(time_zone_link&&) = default;
|
ldionne
requested changes
Feb 13, 2024
libcxx/test/std/time/time.zone/time.zone.db/time.zone.db.tzdb/tzdb.members.pass.cpp
Outdated
Show resolved
Hide resolved
libcxx/test/std/time/time.zone/time.zone.db/time.zone.db.tzdb/tzdb.members.pass.cpp
Outdated
Show resolved
Hide resolved
| // XFAIL: libcpp-has-no-incomplete-tzdb | ||
| // XFAIL: availability-tzdb-missing | ||
|
|
||
| // TODO TZDB Enable tests |
There was a problem hiding this comment.
In all these comments, can you include a link to an issue that tracks the inclusion of the tzdb in our CI? That way we can't forget.
There was a problem hiding this comment.
I've created #81654 and updated all tests, including the existing tests.
|
|
||
| #include "test_macros.h" | ||
|
|
||
| LIBCPP_STATIC_ASSERT(!std::copy_constructible<std::chrono::time_zone>); |
There was a problem hiding this comment.
Let's add a comment saying that it's impossible to actually obtain a non-const reference to a time_zone, and as a result the move constructor can never be exercised in runtime code. We still check the property pedantically.
There was a problem hiding this comment.
I did the same for the time_zone_link test.
| } | ||
| } | ||
|
|
||
| void __init_tzdb(tzdb& __tzdb, __tz::__rules_storage_type& __rules) { |
There was a problem hiding this comment.
I think this should be HIDE_FROM_ABI. Otherwise, we'll have potential ODR violations when mixing different versions of libc++ via static archives. This seems to be a widespread pre-existing problem in the library after a quick survey.
This would force you to define it in the header because HIDE_FROM_ABI is ALWAYS_INLINE on GCC. I think it would be acceptable to ignore this issue until we've cleaned that up, since I feel like this is a very deep can of worms to open.
There was a problem hiding this comment.
Then I leave it as-is. I've created #81652.
This was referenced Feb 13, 2024
ldionne
approved these changes
Feb 15, 2024
libcxx/include/__chrono/tzdb_list.h
Outdated
| class _LIBCPP_AVAILABILITY_TZDB tzdb_list { | ||
| public: | ||
| _LIBCPP_EXPORTED_FROM_ABI explicit tzdb_list(tzdb&& __tzdb); | ||
| class __impl; // public to allow construction in dylib | ||
| _LIBCPP_EXPORTED_FROM_ABI explicit tzdb_list(__impl* __p); |
There was a problem hiding this comment.
Do we really need this in the dylib too? I think the type only needs to be complete at the point where we destroy the unique_ptr, so I think this could be made HIDE_FROM_ABI.
There was a problem hiding this comment.
Thanks for catching this one.
libcxx/include/__chrono/tzdb_list.h
Outdated
| @@ -19,7 +19,7 @@ | |||
| # include <__availability> | |||
| # include <__chrono/tzdb.h> | |||
| # include <forward_list> | |||
| # include <string_view> | |||
| # include <string> | |||
There was a problem hiding this comment.
Why is this include needed? I can't find where we use std::string here.
There was a problem hiding this comment.
The existing non-member function remote_version (at the bottom of the file) returns a string. I've replaced the header with __fwd/string.h.
mordante
commented
Feb 16, 2024
| public: | ||
| explicit _LIBCPP_HIDE_FROM_ABI __impl(string&& __name) : __name_(std::move(__name)) {} | ||
|
|
||
| [[nodiscard]] _LIBCPP_HIDE_FROM_ABI string_view name() const noexcept { return __name_; } |
There was a problem hiding this comment.
since it was copy-pasted from the header. I've __uglified name.
| } | ||
| } | ||
|
|
||
| void __init_tzdb(tzdb& __tzdb, __tz::__rules_storage_type& __rules) { |
There was a problem hiding this comment.
Then I leave it as-is. I've created #81652.
| // XFAIL: libcpp-has-no-incomplete-tzdb | ||
| // XFAIL: availability-tzdb-missing | ||
|
|
||
| // TODO TZDB Enable tests |
There was a problem hiding this comment.
I've created #81654 and updated all tests, including the existing tests.
|
|
||
| #include "test_macros.h" | ||
|
|
||
| LIBCPP_STATIC_ASSERT(!std::copy_constructible<std::chrono::time_zone>); |
There was a problem hiding this comment.
I did the same for the time_zone_link test.
libcxx/include/__chrono/tzdb_list.h
Outdated
| @@ -19,7 +19,7 @@ | |||
| # include <__availability> | |||
| # include <__chrono/tzdb.h> | |||
| # include <forward_list> | |||
| # include <string_view> | |||
| # include <string> | |||
There was a problem hiding this comment.
The existing non-member function remote_version (at the bottom of the file) returns a string. I've replaced the header with __fwd/string.h.
libcxx/include/__chrono/tzdb_list.h
Outdated
| class _LIBCPP_AVAILABILITY_TZDB tzdb_list { | ||
| public: | ||
| _LIBCPP_EXPORTED_FROM_ABI explicit tzdb_list(tzdb&& __tzdb); | ||
| class __impl; // public to allow construction in dylib | ||
| _LIBCPP_EXPORTED_FROM_ABI explicit tzdb_list(__impl* __p); |
There was a problem hiding this comment.
Thanks for catching this one.
mordante
force-pushed
the
GH-load_tzdata
branch
from
da530e9 to
b5c1d3a
Compare
aheejin
added a commit
to aheejin/emscripten
that referenced
this pull request
Dec 4, 2024
We disabled C++20 time zone support in LLVM 18 update (emscripten-core#21638): emscripten-core@df9af64 The list of source files related to time zone support has changed in llvm/llvm-project#74928, so this commit reflects it.
aheejin
added a commit
to aheejin/emscripten
that referenced
this pull request
Dec 21, 2024
We disabled C++20 time zone support in LLVM 18 update (emscripten-core#21638): emscripten-core@df9af64 The list of source files related to time zone support has changed in llvm/llvm-project#74928, so this commit reflects it.
aheejin
added a commit
to emscripten-core/emscripten
that referenced
this pull request
Jan 3, 2025
This updates libcxx and libcxxabi to LLVM 19.1.4: https://github.com/llvm/llvm-project/releases/tag/llvmorg-19.1.4 The initial update was done using `update_libcxx.py` and `update_libcxxabi.py`, and subsequent fixes were made in indidual commits. The commit history here is kind of messy because of CI testing so not all individual commits are noteworthy. Additional changes: - Build libcxx and libcxxabi with C++23: 8b0bfdf https://github.com/llvm/llvm-project/blob/aadaa00de76ed0c4987b97450dd638f63a385bed/libcxx/src/expected.cpp was added in llvm/llvm-project#87390 and this file assumes C++23 to be compiled. Apparently libc++ sources are always built with C++23 so they don't guard things against it in `src/`: llvm/llvm-project#87390 (comment) This commit also builds libc++abi with C++23 because it doesn't seem there's any downside to it. - Exclude newly added `compiler_rt_shims.cpp`: 5bbcbf0 We have excluded files in https://github.com/emscripten-core/emscripten/tree/main/system/lib/libcxx/src/support/win32. This is a new file added in this directory in llvm/llvm-project#83575. - Disable time zone support: a5f2cbe We disabled C++20 time zone support in LLVM 18 update (#21638): df9af64 The list of source files related to time zone support has changed in llvm/llvm-project#74928, so this commit reflects it. - Re-add + update `__assertion_handler` from `default_assertion_handler.in`: 41f8037 This file was added as a part of LLVM 18 update (#21638) in 8d51927 and mistakenly deleted when I ran `update_libcxx.py`. This file was copied from https://github.com/llvm/llvm-project/blob/aadaa00de76ed0c4987b97450dd638f63a385bed/libcxx/vendor/llvm/default_assertion_handler.in, so this also updates the file with the newest `default_assertion_handler.in`. - `_LIBCPP_PSTL_CPU_BACKEND_SERIAL` -> `_LIBCPP_PSTL_BACKEND_SERIAL`: 4b969c3 The name changed in this update, so reflecting it on our `__config_site`. - Directly include `pthread.h` from `emscripten/val.h`: a5a76c3 Due to file rearrangements happened, this was necessary. --- Other things to note: - `std::basic_string<unsigned_char>` is not supported anymore The support for `std::basic_string<unsigned_char>`, which was being used by embind, is removed in this version.#23070 removes the uses from embind. - libcxxabi uses `__FILE__` in more places llvm/llvm-project#80689 started to use `_LIBCXXABI_ASSERT`, which [uses](https://github.com/llvm/llvm-project/blob/aadaa00de76ed0c4987b97450dd638f63a385bed/libcxxabi/src/abort_message.h#L22) `__FILE__`, in `private_typeinfo.cpp`. `__FILE__` macro produces different paths depending on the local directory names and how the file is given to clang in the command line, and this file is included in the result of one of our code size tests, `other.test_minimal_runtime_code_size_hello_embind`, which caused the result of the test to vary depending on the CI bots and how the library is built (e.g., whether by embuilder, ninja, or neither). Even though this was brought to surface during this LLVM 19 update, `__FILE__` macro could be a problem for producing reproducible builds anyway. We discussed this problem in #23195, and the fixes landed in #23222, #23225, and #23256.
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.
This implements the loading of the tzdata.zi file and store its contents in the tzdb struct.
This adds all required members except:
The class time_zone is incomplete and only contains the parts needed for storing the parsed data.
The class time_zone_link is fully implemented including its non-member functions.
Implements parts of:
Implements: