Address review comments and CI fixes.

Author: mordante

libcxx/include/__chrono/time_zone.h

@@ -43,9 +43,9 @@ class _LIBCPP_AVAILABILITY_TZDB time_zone {
 
   // The "constructor".
   //
-  // The default constructor is private to avoid the constructor from being an
-  // ABI interface. Instead use an __ugly_named function as an ABI interface,
-  // since users should not depend on __ugly_names.
+  // The default constructor is private to avoid the constructor from being
+  // part of the ABI. Instead use an __ugly_named function as an ABI interface,
+  // since that gives us the ability to change it in the future.
   [[nodiscard]] _LIBCPP_EXPORTED_FROM_ABI static time_zone __create(unique_ptr<__impl>&& __p);
 
   _LIBCPP_EXPORTED_FROM_ABI ~time_zone();
@@ -55,9 +55,9 @@ class _LIBCPP_AVAILABILITY_TZDB time_zone {
 
   _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI string_view name() const noexcept { return __name(); }
 
-  [[nodiscard]] _LIBCPP_EXPORTED_FROM_ABI const __impl& __implementation() const noexcept;
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI const __impl& __implementation() const noexcept { return *__impl_; }
 
-  // private:
+private:
   [[nodiscard]] _LIBCPP_EXPORTED_FROM_ABI string_view __name() const noexcept;
   unique_ptr<__impl> __impl_;
 };

libcxx/include/__chrono/time_zone_link.h

@@ -39,7 +39,7 @@ class time_zone_link {
 public:
   struct __constructor_tag;
   _LIBCPP_NODISCARD_EXT
-  _LIBCPP_HIDE_FROM_ABI explicit time_zone_link(string_view __name, string_view __target, __constructor_tag&&)
+  _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;

libcxx/include/__chrono/tzdb_list.h

@@ -59,7 +59,7 @@ class _LIBCPP_AVAILABILITY_TZDB tzdb_list {
   _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();
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI __impl& __implementation() { return *__impl_; }
 
 private:
   __impl* __impl_;

libcxx/modules/std/chrono.inc

@@ -222,7 +222,9 @@ export namespace std {
     // [time.zone.timezone], class time_zone
     using std::chrono::choose;
 #  endif
+#  ifdef _LIBCPP_ENABLE_EXPERIMENTAL
     using std::chrono::time_zone;
+#  endif
 #  if 0
 
     // [time.zone.zonedtraits], class template zoned_traits
@@ -237,8 +239,10 @@ export namespace std {
     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
     // [time.format], formatting

libcxx/src/include/tzdb/time_zone_private.h

@@ -26,7 +26,7 @@ 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 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 {

libcxx/src/time_zone.cpp

@@ -16,15 +16,7 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 
 namespace chrono {
 
-#if 0
-_LIBCPP_NODISCARD_EXT _LIBCPP_EXPORTED_FROM_ABI time_zone::time_zone(unique_ptr<time_zone::__impl>&& __p)
-    : __impl_(std::move(__p)) {
-  _LIBCPP_ASSERT_NON_NULL(__impl_ != nullptr, "initialized time_zone without a valid pimpl object");
-}
-#endif
-
 [[nodiscard]] _LIBCPP_EXPORTED_FROM_ABI time_zone time_zone::__create(unique_ptr<time_zone::__impl>&& __p) {
-  //   : __impl_(std::move(__p)) {
   _LIBCPP_ASSERT_NON_NULL(__p != nullptr, "initialized time_zone without a valid pimpl object");
   time_zone result;
   result.__impl_ = std::move(__p);
@@ -33,11 +25,7 @@ _LIBCPP_NODISCARD_EXT _LIBCPP_EXPORTED_FROM_ABI time_zone::time_zone(unique_ptr<
 
 _LIBCPP_EXPORTED_FROM_ABI time_zone::~time_zone() = default;
 
-[[nodiscard]] _LIBCPP_EXPORTED_FROM_ABI string_view time_zone::__name() const noexcept { return __impl_->name(); }
-
-[[nodiscard]] _LIBCPP_EXPORTED_FROM_ABI const time_zone::__impl& time_zone::__implementation() const noexcept {
-  return *__impl_;
-}
+[[nodiscard]] _LIBCPP_EXPORTED_FROM_ABI string_view time_zone::__name() const noexcept { return __impl_->__name(); }
 
 } // namespace chrono
 

libcxx/src/tzdb.cpp

@@ -562,7 +562,7 @@ static void __parse_link(tzdb& __tzdb, istream& __input) {
   string __name = chrono::__parse_string(__input);
   chrono::__skip_line(__input);
 
-  __tzdb.links.emplace_back(std::move(__name), std::move(__target), time_zone_link::__constructor_tag{});
+  __tzdb.links.emplace_back(time_zone_link::__constructor_tag{}, std::move(__name), std::move(__target));
 }
 
 static void __parse_tzdata(tzdb& __db, __tz::__rules_storage_type& __rules, istream& __input) {

libcxx/src/tzdb_list.cpp

@@ -44,8 +44,6 @@ _LIBCPP_NODISCARD_EXT _LIBCPP_EXPORTED_FROM_ABI tzdb_list::const_iterator tzdb_l
   return __impl_->cend();
 }
 
-[[nodiscard]] _LIBCPP_EXPORTED_FROM_ABI tzdb_list::__impl& tzdb_list::__implementation() { return *__impl_; }
-
 } // namespace chrono
 
 _LIBCPP_END_NAMESPACE_STD

libcxx/test/libcxx/diagnostics/chrono.nodiscard_extensions.compile.pass.cpp

@@ -24,8 +24,8 @@
 #include "test_macros.h"
 
 // These types have "private" constructors.
-extern  std::chrono::time_zone tz;
-extern  std::chrono::time_zone_link link;
+extern std::chrono::time_zone tz;
+extern std::chrono::time_zone_link link;
 
 void test() {
   std::chrono::tzdb_list& list = std::chrono::get_tzdb_list();

libcxx/test/libcxx/diagnostics/chrono.nodiscard_extensions.verify.cpp

@@ -21,8 +21,8 @@
 #include "test_macros.h"
 
 // These types have "private" constructors.
-extern  std::chrono::time_zone tz;
-extern  std::chrono::time_zone_link link;
+extern std::chrono::time_zone tz;
+extern std::chrono::time_zone_link link;
 
 void test() {
   std::chrono::tzdb_list& list = std::chrono::get_tzdb_list();