Address review comments.

Author: mordante

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","|Complete|","18.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|"
 "`P2002R1 <https://wg21.link/P2002R1>`__","CWG","Defaulted comparison specification cleanups","Prague","* *",""

libcxx/include/CMakeLists.txt

@@ -292,7 +292,6 @@ set(files
   __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

libcxx/include/__chrono/time_zone.h

@@ -16,17 +16,18 @@
 // 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 <__memory/unique_ptr.h>
 #  include <string_view>
-#  include <vector>
 
 #  if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #    pragma GCC system_header
 #  endif
 
+_LIBCPP_PUSH_MACROS
+#  include <__undef_macros>
+
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 #  if _LIBCPP_STD_VER >= 20 && !defined(_LIBCPP_HAS_NO_TIME_ZONE_DATABASE) && !defined(_LIBCPP_HAS_NO_FILESYSTEM) &&   \
@@ -36,21 +37,17 @@ namespace chrono {
 
 class _LIBCPP_AVAILABILITY_TZDB time_zone {
 public:
-  explicit _LIBCPP_HIDE_FROM_ABI time_zone(string&& __name) : __name_(std::move(__name)) {}
+  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);
+  _LIBCPP_EXPORTED_FROM_ABI ~time_zone();
 
-  _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI string_view name() const noexcept { return __name_; }
+  time_zone(time_zone&&)            = default;
+  time_zone& operator=(time_zone&&) = default;
 
-  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI vector<__tz::__continuation>& __continuations() { return __continuations_; }
-  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI const vector<__tz::__continuation>& __continuations() const {
-    return __continuations_;
-  }
+  _LIBCPP_NODISCARD_EXT _LIBCPP_EXPORTED_FROM_ABI string_view name() const noexcept;
 
 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_;
+  unique_ptr<__impl> __impl_;
 };
 
 _LIBCPP_NODISCARD_EXT _LIBCPP_AVAILABILITY_TZDB _LIBCPP_HIDE_FROM_ABI inline bool
@@ -70,6 +67,8 @@ operator<=>(const time_zone& __x, const time_zone& __y) noexcept {
 
 _LIBCPP_END_NAMESPACE_STD
 
+_LIBCPP_POP_MACROS
+
 #endif // !defined(_LIBCPP_HAS_NO_INCOMPLETE_TZDB)
 
 #endif // _LIBCPP___CHRONO_TIME_ZONE_H

libcxx/include/__chrono/time_zone_link.h

@@ -25,14 +25,17 @@
 #    pragma GCC system_header
 #  endif
 
+_LIBCPP_PUSH_MACROS
+#  include <__undef_macros>
+
 _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 {
+class 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} {}
@@ -67,6 +70,8 @@ operator<=>(const time_zone_link& __x, const time_zone_link& __y) noexcept {
 
 _LIBCPP_END_NAMESPACE_STD
 
+_LIBCPP_POP_MACROS
+
 #endif // !defined(_LIBCPP_HAS_NO_INCOMPLETE_TZDB)
 
 #endif // _LIBCPP___CHRONO_TIME_ZONE_LINK_H

libcxx/include/__chrono/tzdb.h

@@ -16,10 +16,8 @@
 // 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>
@@ -28,16 +26,18 @@
 #    pragma GCC system_header
 #  endif
 
+_LIBCPP_PUSH_MACROS
+#  include <__undef_macros>
+
 _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 {
 
-struct _LIBCPP_AVAILABILITY_TZDB tzdb {
+struct tzdb {
   string version;
-  vector<pair<string, vector<__tz::__rule>>> __rules;
   vector<time_zone> zones;
   vector<time_zone_link> links;
 };
@@ -49,6 +49,8 @@ struct _LIBCPP_AVAILABILITY_TZDB tzdb {
 
 _LIBCPP_END_NAMESPACE_STD
 
+_LIBCPP_POP_MACROS
+
 #endif // !defined(_LIBCPP_HAS_NO_INCOMPLETE_TZDB)
 
 #endif // _LIBCPP___CHRONO_TZDB_H

libcxx/include/__chrono/tzdb_list.h

@@ -19,7 +19,7 @@
 #  include <__availability>
 #  include <__chrono/tzdb.h>
 #  include <forward_list>
-#  include <string_view>
+#  include <string>
 
 #  if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #    pragma GCC system_header
@@ -34,7 +34,8 @@ namespace chrono {
 
 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);
   _LIBCPP_EXPORTED_FROM_ABI ~tzdb_list();
 
   tzdb_list(const tzdb_list&)            = delete;
@@ -46,16 +47,15 @@ class _LIBCPP_AVAILABILITY_TZDB tzdb_list {
 
   _LIBCPP_EXPORTED_FROM_ABI const_iterator erase_after(const_iterator __p);
 
-  _LIBCPP_EXPORTED_FROM_ABI tzdb& __emplace_front(tzdb&& __tzdb);
-
   _LIBCPP_NODISCARD_EXT _LIBCPP_EXPORTED_FROM_ABI const_iterator begin() const noexcept;
   _LIBCPP_NODISCARD_EXT _LIBCPP_EXPORTED_FROM_ABI const_iterator end() const noexcept;
 
   _LIBCPP_NODISCARD_EXT _LIBCPP_EXPORTED_FROM_ABI const_iterator cbegin() const noexcept;
   _LIBCPP_NODISCARD_EXT _LIBCPP_EXPORTED_FROM_ABI const_iterator cend() const noexcept;
 
+  [[nodiscard]] _LIBCPP_EXPORTED_FROM_ABI __impl& __implementation();
+
 private:
-  class __impl;
   __impl* __impl_;
 };
 

libcxx/include/chrono

@@ -865,7 +865,6 @@ constexpr chrono::year                                  operator ""y(unsigned lo
     !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

libcxx/include/module.modulemap.in

@@ -1163,10 +1163,6 @@ module std_private_chrono_time_zone_link         [system] {
   header "__chrono/time_zone_link.h"
   export *
 }
-module std_private_chrono_time_zone_types        [system] {
-  header "__chrono/time_zone_types.h"
-  export *
-}
 module std_private_chrono_system_clock           [system] {
   header "__chrono/system_clock.h"
   export std_private_chrono_time_point

libcxx/src/CMakeLists.txt

@@ -336,6 +336,11 @@ endif()
 
 if (LIBCXX_ENABLE_LOCALIZATION AND LIBCXX_ENABLE_FILESYSTEM AND LIBCXX_ENABLE_TIME_ZONE_DATABASE)
   list(APPEND LIBCXX_EXPERIMENTAL_SOURCES
+    include/tzdb/time_zone.h
+    include/tzdb/types.h
+    include/tzdb/tz.h
+    include/tzdb/tzdb_list.h
+    time_zone.cpp
     tz.cpp
     tzdb_list.cpp
     )

libcxx/src/include/tzdb/time_zone.h

@@ -0,0 +1,48 @@
+// -*- 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_TIME_ZONE_H
+#define _LIBCPP_SRC_INCLUDE_TZDB_TIME_ZONE_H
+
+#include <chrono>
+#include <string>
+#include <vector>
+
+#include "types.h"
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+namespace chrono {
+
+class time_zone::__impl {
+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_; }
+
+  [[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_;
+};
+
+} // namespace chrono
+
+_LIBCPP_END_NAMESPACE_STD
+
+#endif // _LIBCPP_SRC_INCLUDE_TZDB_TIME_ZONE_H

libcxx/include/__chrono/time_zone_types.h renamed to libcxx/src/include/tzdb/types.h

@@ -9,36 +9,30 @@
 
 // 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
+#ifndef __LIBCPP_SRC_INCLUDE_TZDB_TYPES_H
+#define __LIBCPP_SRC_INCLUDE_TZDB_TYPES_H
+
+#include <chrono>
+#include <string>
+#include <utility>
+#include <variant>
+#include <vector>
 
 _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)
+// TODO TZDB
+// The helper classes in this header have no constructor but are loaded with
+// dedicated parse functions. In the original design this header was public and
+// the parsing is done in the dylib. In that design having constructors would
+// expand the ABI interface. Since this header is now in the dylib that design
+// should be reconsidered. (For now the design is kept as is, in case this
+// header needs to be public for unforseen reasons.)
 
 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 {
+struct __constrained_weekday {
   /*  year_month_day operator()(year __year, month __month);*/ // needed but not implemented
 
   weekday __weekday;
@@ -54,30 +48,35 @@ struct _LIBCPP_AVAILABILITY_TZDB __constrained_weekday {
 //  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 };
+enum class __clock { __local, __standard, __universal };
 
-struct _LIBCPP_AVAILABILITY_TZDB __at {
+struct __at {
   seconds __time{0};
   __tz::__clock __clock{__tz::__clock::__local};
 };
 
-struct _LIBCPP_AVAILABILITY_TZDB __save {
+struct __save {
   seconds __time;
   bool __is_dst;
 };
 
 // The names of the fields match the fields of a Rule.
-struct _LIBCPP_AVAILABILITY_TZDB __rule {
+struct __rule {
   year __from;
   year __to;
-  month __in_month; // __in is a reserved name
+  month __in;
   __tz::__on __on;
   __tz::__at __at;
   __tz::__save __save;
   string __letters;
 };
 
-struct _LIBCPP_AVAILABILITY_TZDB __continuation {
+using __rules_storage_type = std::vector<std::pair<string, vector<__tz::__rule>>>; // TODO TZDB use flat_map;
+
+struct __continuation {
+  // Non-owning link to the RULE entries.
+  __tz::__rules_storage_type* __rule_database_;
+
   seconds __stdoff;
 
   // The RULES is either a SAVE or a NAME.
@@ -95,18 +94,13 @@ struct _LIBCPP_AVAILABILITY_TZDB __continuation {
   // 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
+  month __in{January};
   __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
+#endif // __LIBCPP_SRC_INCLUDE_TZDB_TYPES_H