[libcxx-commits] [libcxx] [libc++] Make std::pair trivially copyable if its members are (PR #89652)

Nikolas Klauser via libcxx-commits libcxx-commits at lists.llvm.org
Wed Jul 17 11:53:24 PDT 2024


https://github.com/philnik777 updated https://github.com/llvm/llvm-project/pull/89652

>From d804641555f279eb759a2963eec46c3a23c7f4ce Mon Sep 17 00:00:00 2001
From: Nikolas Klauser <nikolasklauser at berlin.de>
Date: Mon, 22 Apr 2024 20:58:34 +0200
Subject: [PATCH] [libc++][ABI BREAK] Make std::pair trivially copyable if its
 members are

---
 libcxx/include/__configuration/abi.h          |  4 ++
 libcxx/include/__type_traits/datasizeof.h     |  1 +
 libcxx/include/__utility/pair.h               | 45 +++++++++++++++----
 .../abi.trivially_copyable.compile.pass.cpp   | 22 ++++++++-
 4 files changed, 63 insertions(+), 9 deletions(-)

diff --git a/libcxx/include/__configuration/abi.h b/libcxx/include/__configuration/abi.h
index 513da6e3b81b6..bc4ecff679f7a 100644
--- a/libcxx/include/__configuration/abi.h
+++ b/libcxx/include/__configuration/abi.h
@@ -91,6 +91,8 @@
 #  define _LIBCPP_ABI_USE_WRAP_ITER_IN_STD_STRING_VIEW
 // Dont' add an inline namespace for `std::filesystem`
 #  define _LIBCPP_ABI_NO_FILESYSTEM_INLINE_NAMESPACE
+// Make a std::pair of trivially copyable types trivially copyable
+#  define _LIBCPP_ABI_TRIVIALLY_COPYABLE_PAIR
 #elif _LIBCPP_ABI_VERSION == 1
 #  if !(defined(_LIBCPP_OBJECT_FORMAT_COFF) || defined(_LIBCPP_OBJECT_FORMAT_XCOFF))
 // Enable compiling copies of now inline methods into the dylib to support
@@ -110,6 +112,8 @@
 #  endif
 #endif
 
+#  define _LIBCPP_ABI_TRIVIALLY_COPYABLE_PAIR
+
 // We had some bugs where we use [[no_unique_address]] together with construct_at,
 // which causes UB as the call on construct_at could write to overlapping subobjects
 //
diff --git a/libcxx/include/__type_traits/datasizeof.h b/libcxx/include/__type_traits/datasizeof.h
index 35c12921e8ffa..a27baf67cc2d8 100644
--- a/libcxx/include/__type_traits/datasizeof.h
+++ b/libcxx/include/__type_traits/datasizeof.h
@@ -54,6 +54,7 @@ struct _FirstPaddingByte<_Tp, true> {
 // the use as an extension.
 _LIBCPP_DIAGNOSTIC_PUSH
 _LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Winvalid-offsetof")
+_LIBCPP_GCC_DIAGNOSTIC_IGNORED("-Winvalid-offsetof")
 template <class _Tp>
 inline const size_t __datasizeof_v = offsetof(_FirstPaddingByte<_Tp>, __first_padding_byte_);
 _LIBCPP_DIAGNOSTIC_POP
diff --git a/libcxx/include/__utility/pair.h b/libcxx/include/__utility/pair.h
index 0afbebcdc9f2a..d5b34a1f3f713 100644
--- a/libcxx/include/__utility/pair.h
+++ b/libcxx/include/__utility/pair.h
@@ -32,6 +32,7 @@
 #include <__type_traits/is_implicitly_default_constructible.h>
 #include <__type_traits/is_nothrow_assignable.h>
 #include <__type_traits/is_nothrow_constructible.h>
+#include <__type_traits/is_reference.h>
 #include <__type_traits/is_same.h>
 #include <__type_traits/is_swappable.h>
 #include <__type_traits/is_trivially_relocatable.h>
@@ -80,6 +81,31 @@ struct _LIBCPP_TEMPLATE_VIS pair
   _LIBCPP_HIDE_FROM_ABI pair(pair const&) = default;
   _LIBCPP_HIDE_FROM_ABI pair(pair&&)      = default;
 
+  // When we are requested for pair to be trivially copyable by the ABI macro, we use defaulted members
+  // if it is both legal to do it (i.e. no references) and we have a way to actually implement it, which requires
+  // the __enable_if__ attribute before C++20.
+#ifdef _LIBCPP_ABI_TRIVIALLY_COPYABLE_PAIR
+  static const bool __has_defaulted_members = !is_reference<first_type>::value && !is_reference<second_type>::value;
+#if _LIBCPP_STD_VER >= 20
+  _LIBCPP_HIDE_FROM_ABI constexpr pair& operator=(const pair&)
+    requires __has_defaulted_members
+  = default;
+
+  _LIBCPP_HIDE_FROM_ABI constexpr pair& operator=(pair&&)
+    requires __has_defaulted_members
+  = default;
+#elif __has_attribute(__enable_if__)
+  _LIBCPP_HIDE_FROM_ABI pair& operator=(const pair&)
+      __attribute__((__enable_if__(__has_defaulted_members, ""))) = default;
+
+  _LIBCPP_HIDE_FROM_ABI pair& operator=(pair&&) __attribute__((__enable_if__(__has_defaulted_members, ""))) = default;
+#else
+#  error "_LIBCPP_ABI_TRIVIALLY_COPYABLE_PAIR isn't supported with this compiler"
+#endif
+#else
+  static const bool __has_defaulted_members = false;
+#endif // defined(_LIBCPP_ABI_TRIVIALLY_COPYABLE_PAIR) && __has_attribute(__enable_if__)
+
 #ifdef _LIBCPP_CXX03_LANG
   _LIBCPP_HIDE_FROM_ABI pair() : first(), second() {}
 
@@ -225,19 +251,22 @@ struct _LIBCPP_TEMPLATE_VIS pair
              typename __make_tuple_indices<sizeof...(_Args2) >::type()) {}
 
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 pair&
-  operator=(__conditional_t<is_copy_assignable<first_type>::value && is_copy_assignable<second_type>::value,
-                            pair,
-                            __nat> const& __p) noexcept(is_nothrow_copy_assignable<first_type>::value &&
-                                                        is_nothrow_copy_assignable<second_type>::value) {
+  operator=(__conditional_t<
+            !__has_defaulted_members && is_copy_assignable<first_type>::value && is_copy_assignable<second_type>::value,
+            pair,
+            __nat> const& __p) noexcept(is_nothrow_copy_assignable<first_type>::value &&
+                                        is_nothrow_copy_assignable<second_type>::value) {
     first  = __p.first;
     second = __p.second;
     return *this;
   }
 
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 pair& operator=(
-      __conditional_t<is_move_assignable<first_type>::value && is_move_assignable<second_type>::value, pair, __nat>&&
-          __p) noexcept(is_nothrow_move_assignable<first_type>::value &&
-                        is_nothrow_move_assignable<second_type>::value) {
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 pair&
+  operator=(__conditional_t<
+            !__has_defaulted_members && is_move_assignable<first_type>::value && is_move_assignable<second_type>::value,
+            pair,
+            __nat>&& __p) noexcept(is_nothrow_move_assignable<first_type>::value &&
+                                   is_nothrow_move_assignable<second_type>::value) {
     first  = std::forward<first_type>(__p.first);
     second = std::forward<second_type>(__p.second);
     return *this;
diff --git a/libcxx/test/libcxx/utilities/utility/pairs/pairs.pair/abi.trivially_copyable.compile.pass.cpp b/libcxx/test/libcxx/utilities/utility/pairs/pairs.pair/abi.trivially_copyable.compile.pass.cpp
index 1132b3e5def18..c5f9c8d0f2559 100644
--- a/libcxx/test/libcxx/utilities/utility/pairs/pairs.pair/abi.trivially_copyable.compile.pass.cpp
+++ b/libcxx/test/libcxx/utilities/utility/pairs/pairs.pair/abi.trivially_copyable.compile.pass.cpp
@@ -47,11 +47,20 @@ static_assert(!std::is_trivially_copyable<std::pair<int&, int> >::value, "");
 static_assert(!std::is_trivially_copyable<std::pair<int, int&> >::value, "");
 static_assert(!std::is_trivially_copyable<std::pair<int&, int&> >::value, "");
 
+#ifdef _LIBCPP_ABI_TRIVIALLY_COPYABLE_PAIR
+static_assert(std::is_trivially_copyable<std::pair<int, int> >::value, "");
+static_assert(std::is_trivially_copyable<std::pair<int, char> >::value, "");
+static_assert(std::is_trivially_copyable<std::pair<char, int> >::value, "");
+static_assert(std::is_trivially_copyable<std::pair<std::pair<char, char>, int> >::value, "");
+static_assert(std::is_trivially_copyable<std::pair<trivially_copyable, int> >::value, "");
+#else
 static_assert(!std::is_trivially_copyable<std::pair<int, int> >::value, "");
 static_assert(!std::is_trivially_copyable<std::pair<int, char> >::value, "");
 static_assert(!std::is_trivially_copyable<std::pair<char, int> >::value, "");
 static_assert(!std::is_trivially_copyable<std::pair<std::pair<char, char>, int> >::value, "");
 static_assert(!std::is_trivially_copyable<std::pair<trivially_copyable, int> >::value, "");
+#endif // _LIBCPP_ABI_TRIVIALLY_COPYABLE_PAIR
+
 #if TEST_STD_VER == 03 // Known ABI difference
 static_assert(!std::is_trivially_copyable<std::pair<trivially_copyable_no_copy_assignment, int> >::value, "");
 static_assert(!std::is_trivially_copyable<std::pair<trivially_copyable_no_move_assignment, int> >::value, "");
@@ -59,10 +68,21 @@ static_assert(!std::is_trivially_copyable<std::pair<trivially_copyable_no_move_a
 static_assert(std::is_trivially_copyable<std::pair<trivially_copyable_no_copy_assignment, int> >::value, "");
 static_assert(std::is_trivially_copyable<std::pair<trivially_copyable_no_move_assignment, int> >::value, "");
 #endif
+
+#ifdef _LIBCPP_ABI_TRIVIALLY_COPYABLE_PAIR
+static_assert(std::is_trivially_copyable<std::pair<trivially_copyable_no_construction, int> >::value, "");
+#else
 static_assert(!std::is_trivially_copyable<std::pair<trivially_copyable_no_construction, int> >::value, "");
+#endif
 
 static_assert(std::is_trivially_copy_constructible<std::pair<int, int> >::value, "");
 static_assert(std::is_trivially_move_constructible<std::pair<int, int> >::value, "");
+static_assert(std::is_trivially_destructible<std::pair<int, int> >::value, "");
+
+#ifdef _LIBCPP_ABI_TRIVIALLY_COPYABLE_PAIR
+static_assert(std::is_trivially_copy_assignable<std::pair<int, int> >::value, "");
+static_assert(std::is_trivially_move_assignable<std::pair<int, int> >::value, "");
+#else
 static_assert(!std::is_trivially_copy_assignable<std::pair<int, int> >::value, "");
 static_assert(!std::is_trivially_move_assignable<std::pair<int, int> >::value, "");
-static_assert(std::is_trivially_destructible<std::pair<int, int> >::value, "");
+#endif // _LIBCPP_ABI_TRIVIALLY_COPYABLE_PAIR



More information about the libcxx-commits mailing list