Skip to content

[libc++] Adds a global private constructor tag. #87920

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 10, 2024

Conversation

mordante
Copy link
Member

@mordante mordante commented Apr 7, 2024

This removes the similar tags used in the chrono tzdb implementation.

Fixes: #85432

@mordante mordante requested a review from a team as a code owner April 7, 2024 12:35
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Apr 7, 2024
@llvmbot
Copy link
Member

llvmbot commented Apr 7, 2024

@llvm/pr-subscribers-libcxx

Author: Mark de Wever (mordante)

Changes

This removes the similar tags used in the chrono tzdb implementation.

Fixes: #85432


Full diff: https://github.com/llvm/llvm-project/pull/87920.diff

12 Files Affected:

  • (modified) libcxx/include/CMakeLists.txt (+1)
  • (modified) libcxx/include/__chrono/leap_second.h (+2-2)
  • (modified) libcxx/include/__chrono/time_zone_link.h (+2-2)
  • (added) libcxx/include/__utility/private_constructor_tag.h (+28)
  • (modified) libcxx/include/module.modulemap (+11-10)
  • (modified) libcxx/src/CMakeLists.txt (-2)
  • (removed) libcxx/src/include/tzdb/leap_second_private.h (-27)
  • (removed) libcxx/src/include/tzdb/time_zone_link_private.h (-27)
  • (modified) libcxx/src/tzdb.cpp (+2-4)
  • (added) libcxx/test/libcxx/utilities/utility/private_constructor_tag/private_header.verify.cpp (+20)
  • (added) libcxx/test/libcxx/utilities/utility/private_constructor_tag/types.compile.pass.cpp (+20)
  • (modified) libcxx/test/support/test_chrono_leap_second.h (+1-1)
diff --git a/libcxx/include/CMakeLists.txt b/libcxx/include/CMakeLists.txt
index 097a41d4c41740..4818e0717b9fe2 100644
--- a/libcxx/include/CMakeLists.txt
+++ b/libcxx/include/CMakeLists.txt
@@ -861,6 +861,7 @@ set(files
   __utility/pair.h
   __utility/piecewise_construct.h
   __utility/priority_tag.h
+  __utility/private_constructor_tag.h
   __utility/rel_ops.h
   __utility/small_buffer.h
   __utility/swap.h
diff --git a/libcxx/include/__chrono/leap_second.h b/libcxx/include/__chrono/leap_second.h
index 4e67cc2d65277c..49565e4179d563 100644
--- a/libcxx/include/__chrono/leap_second.h
+++ b/libcxx/include/__chrono/leap_second.h
@@ -22,6 +22,7 @@
 #  include <__compare/ordering.h>
 #  include <__compare/three_way_comparable.h>
 #  include <__config>
+#  include <__utility/private_constructor_tag.h>
 
 #  if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #    pragma GCC system_header
@@ -35,9 +36,8 @@ 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)
+  _LIBCPP_HIDE_FROM_ABI explicit constexpr leap_second(__private_constructor_tag&&, sys_seconds __date, seconds __value)
       : __date_(__date), __value_(__value) {}
 
   _LIBCPP_HIDE_FROM_ABI leap_second(const leap_second&)            = default;
diff --git a/libcxx/include/__chrono/time_zone_link.h b/libcxx/include/__chrono/time_zone_link.h
index 17e915d2677a8c..4b346ae568cbfb 100644
--- a/libcxx/include/__chrono/time_zone_link.h
+++ b/libcxx/include/__chrono/time_zone_link.h
@@ -18,6 +18,7 @@
 
 #  include <__compare/strong_order.h>
 #  include <__config>
+#  include <__utility/private_constructor_tag.h>
 #  include <string>
 #  include <string_view>
 
@@ -37,9 +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_HIDE_FROM_ABI explicit time_zone_link(__private_constructor_tag&&, string_view __name, string_view __target)
       : __name_{__name}, __target_{__target} {}
 
   _LIBCPP_HIDE_FROM_ABI time_zone_link(time_zone_link&&)            = default;
diff --git a/libcxx/include/__utility/private_constructor_tag.h b/libcxx/include/__utility/private_constructor_tag.h
new file mode 100644
index 00000000000000..271c00692d04be
--- /dev/null
+++ b/libcxx/include/__utility/private_constructor_tag.h
@@ -0,0 +1,28 @@
+// -*- 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__UTILITY_PRIVATE_CONSTRUCTOR_TAG_H
+#define _LIBCPP__UTILITY_PRIVATE_CONSTRUCTOR_TAG_H
+
+#include <__config>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#  pragma GCC system_header
+#endif
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+// This struct is intended to use to make a non-standard constructor
+// exposition-only. This does not prevent users from using the constructor, but
+// they need to explicitly use __ugly_code and include a private header.
+struct __private_constructor_tag {};
+
+_LIBCPP_END_NAMESPACE_STD
+
+#endif // _LIBCPP__UTILITY_PRIVATE_CONSTRUCTOR_TAG_H
diff --git a/libcxx/include/module.modulemap b/libcxx/include/module.modulemap
index ed45a1b1833893..31614542de90ef 100644
--- a/libcxx/include/module.modulemap
+++ b/libcxx/include/module.modulemap
@@ -2085,18 +2085,19 @@ module std_private_utility_pair                   [system] {
   export std_private_type_traits_is_nothrow_move_assignable
   export std_private_utility_pair_fwd
 }
-module std_private_utility_pair_fwd               [system] { header "__fwd/pair.h" }
-module std_private_utility_piecewise_construct    [system] { header "__utility/piecewise_construct.h" }
-module std_private_utility_priority_tag           [system] { header "__utility/priority_tag.h" }
-module std_private_utility_rel_ops                [system] { header "__utility/rel_ops.h" }
-module std_private_utility_small_buffer           [system] { header "__utility/small_buffer.h" }
-module std_private_utility_swap                   [system] {
+module std_private_utility_pair_fwd                [system] { header "__fwd/pair.h" }
+module std_private_utility_piecewise_construct     [system] { header "__utility/piecewise_construct.h" }
+module std_private_utility_priority_tag            [system] { header "__utility/priority_tag.h" }
+module std_private_utility_private_constructor_tag [system] { header "__utility/private_constructor_tag.h" }
+module std_private_utility_rel_ops                 [system] { header "__utility/rel_ops.h" }
+module std_private_utility_small_buffer            [system] { header "__utility/small_buffer.h" }
+module std_private_utility_swap                    [system] {
   header "__utility/swap.h"
   export std_private_type_traits_is_swappable
 }
-module std_private_utility_to_underlying          [system] { header "__utility/to_underlying.h" }
-module std_private_utility_unreachable            [system] { header "__utility/unreachable.h" }
+module std_private_utility_to_underlying           [system] { header "__utility/to_underlying.h" }
+module std_private_utility_unreachable             [system] { header "__utility/unreachable.h" }
 
-module std_private_variant_monostate [system] { header "__variant/monostate.h" }
+module std_private_variant_monostate               [system] { header "__variant/monostate.h" }
 
-module std_private_vector_fwd [system] { header "__fwd/vector.h" }
+module std_private_vector_fwd                      [system] { header "__fwd/vector.h" }
diff --git a/libcxx/src/CMakeLists.txt b/libcxx/src/CMakeLists.txt
index 16ccb80ba3326d..208500ec14fcdc 100644
--- a/libcxx/src/CMakeLists.txt
+++ b/libcxx/src/CMakeLists.txt
@@ -334,8 +334,6 @@ 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
     include/tzdb/tzdb_list_private.h
diff --git a/libcxx/src/include/tzdb/leap_second_private.h b/libcxx/src/include/tzdb/leap_second_private.h
deleted file mode 100644
index 7a811ab1975942..00000000000000
--- a/libcxx/src/include/tzdb/leap_second_private.h
+++ /dev/null
@@ -1,27 +0,0 @@
-// -*- 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/include/tzdb/time_zone_link_private.h b/libcxx/src/include/tzdb/time_zone_link_private.h
deleted file mode 100644
index 139237625274d3..00000000000000
--- a/libcxx/src/include/tzdb/time_zone_link_private.h
+++ /dev/null
@@ -1,27 +0,0 @@
-// -*- 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_LINK_PRIVATE_H
-#define _LIBCPP_SRC_INCLUDE_TZDB_TIME_ZONE_LINK_PRIVATE_H
-
-#include <chrono>
-
-_LIBCPP_BEGIN_NAMESPACE_STD
-
-namespace chrono {
-
-struct time_zone_link::__constructor_tag {};
-
-} // namespace chrono
-
-_LIBCPP_END_NAMESPACE_STD
-
-#endif // _LIBCPP_SRC_INCLUDE_TZDB_TIME_ZONE_LINK_PRIVATE_H
diff --git a/libcxx/src/tzdb.cpp b/libcxx/src/tzdb.cpp
index 2c82a4a4317a81..59a79112c969c1 100644
--- a/libcxx/src/tzdb.cpp
+++ b/libcxx/src/tzdb.cpp
@@ -15,8 +15,6 @@
 #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"
 #include "include/tzdb/tzdb_list_private.h"
@@ -582,7 +580,7 @@ static void __parse_link(tzdb& __tzdb, istream& __input) {
   string __name = chrono::__parse_string(__input);
   chrono::__skip_line(__input);
 
-  __tzdb.links.emplace_back(time_zone_link::__constructor_tag{}, std::move(__name), std::move(__target));
+  __tzdb.links.emplace_back(std::__private_constructor_tag{}, std::move(__name), std::move(__target));
 }
 
 static void __parse_tzdata(tzdb& __db, __tz::__rules_storage_type& __rules, istream& __input) {
@@ -649,7 +647,7 @@ static void __parse_leap_seconds(vector<leap_second>& __leap_seconds, istream&&
     seconds __value{chrono::__parse_integral(__input, false)};
     chrono::__skip_line(__input);
 
-    __leap_seconds.emplace_back(leap_second::__constructor_tag{}, __date, __value);
+    __leap_seconds.emplace_back(std::__private_constructor_tag{}, __date, __value);
   }
 }
 
diff --git a/libcxx/test/libcxx/utilities/utility/private_constructor_tag/private_header.verify.cpp b/libcxx/test/libcxx/utilities/utility/private_constructor_tag/private_header.verify.cpp
new file mode 100644
index 00000000000000..80b2bff72e3503
--- /dev/null
+++ b/libcxx/test/libcxx/utilities/utility/private_constructor_tag/private_header.verify.cpp
@@ -0,0 +1,20 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// struct __private_constructor_tag {};
+
+// The private constructor tag is intended to be a trivial type that can easily
+// be used to mark a constructor exposition-only. The name is uglified and not
+// provided directly by the utility header.
+//
+// Tests whether the type is not provided by the utility header.
+
+#include <utility>
+
+// expected-error@+1 {{no type named '__private_constructor_tag' in namespace 'std'}}
+std::__private_constructor_tag tag;
diff --git a/libcxx/test/libcxx/utilities/utility/private_constructor_tag/types.compile.pass.cpp b/libcxx/test/libcxx/utilities/utility/private_constructor_tag/types.compile.pass.cpp
new file mode 100644
index 00000000000000..124503a7e8120b
--- /dev/null
+++ b/libcxx/test/libcxx/utilities/utility/private_constructor_tag/types.compile.pass.cpp
@@ -0,0 +1,20 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// struct __private_constructor_tag{};
+
+// The private constructor tag is intended to be a trivial type that can easily
+// be used to mark a constructor exposition-only. The name is uglified and not
+// provided directly by the utility header.
+//
+// Tests whether the type is trivial.
+
+#include <__utility/private_constructor_tag.h>
+#include <type_traits>
+
+static_assert(std::is_trivial<std::__private_constructor_tag>::value, "");
diff --git a/libcxx/test/support/test_chrono_leap_second.h b/libcxx/test/support/test_chrono_leap_second.h
index 485f68d91b1a15..177f017434e7fc 100644
--- a/libcxx/test/support/test_chrono_leap_second.h
+++ b/libcxx/test/support/test_chrono_leap_second.h
@@ -41,7 +41,7 @@
 
 inline constexpr std::chrono::leap_second
 test_leap_second_create(const std::chrono::sys_seconds& date, const std::chrono::seconds& value) {
-  return std::chrono::leap_second{std::chrono::leap_second::__constructor_tag{}, date, value};
+  return std::chrono::leap_second{std::__private_constructor_tag{}, date, value};
 }
 
 #else // _LIBCPP_VERSION

@mordante mordante force-pushed the review/private_constructor_tag branch 2 times, most recently from ac68a34 to 5f0009b Compare April 7, 2024 14:22
Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with some comments. Thanks for the cleanup!

This removes the similar tags used in the chrono tzdb implementation.

Fixes: llvm#85432
@mordante mordante force-pushed the review/private_constructor_tag branch from 5f0009b to 6062efe Compare April 9, 2024 17:36
@mordante mordante merged commit 0a13175 into llvm:main Apr 10, 2024
@mordante mordante deleted the review/private_constructor_tag branch April 10, 2024 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[libc++] Add a global private constructor tag
3 participants