[libcxx-commits] [libcxx] [libc++] Fix triviality of std::pair for trivially copyable types without an assignment operator (PR #95444)
via libcxx-commits
libcxx-commits at lists.llvm.org
Thu Jun 13 10:51:26 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-libcxx
Author: Louis Dionne (ldionne)
<details>
<summary>Changes</summary>
Since 83ead2b, std::pair would not be trivially copyable when it holds a trivially copyable type without an assignment operator. That is because pair gained an elligible copy-assignment-operator (the const version) in 83ead2b in C++ >= 23.
This means that the trivially copyable property of std::pair for such types would be inconsistent between C++11/14/17/20 (trivially copyable) and C++23/26 (not trivially copyable). This patch makes std::pair's behavior consistent in all Standard modes EXCEPT C++03, which is a pre-existing condition and we have no way of changing (also, it shouldn't matter because the std::is_trivially_copyable trait was introduced in C++11).
While this is not technically an ABI break, in practice we do know that folks sometimes use a different representation based on whether a type is trivially copyable. So we're treating 83ead2b as an ABI break and this patch is fixing said breakage.
This patch also adds tests stolen from #<!-- -->89652 that pin down the ABI of std::pair with respect to being trivially copyable.
Fixes #<!-- -->95428
---
Full diff: https://github.com/llvm/llvm-project/pull/95444.diff
4 Files Affected:
- (modified) libcxx/include/__utility/pair.h (+2)
- (renamed) libcxx/test/libcxx/utilities/utility/pairs/pairs.pair/abi.non_trivial_copy_move.pass.cpp ()
- (renamed) libcxx/test/libcxx/utilities/utility/pairs/pairs.pair/abi.trivial_copy_move.pass.cpp (+31)
- (added) libcxx/test/libcxx/utilities/utility/pairs/pairs.pair/abi.trivially_copyable.compile.pass.cpp (+56)
``````````diff
diff --git a/libcxx/include/__utility/pair.h b/libcxx/include/__utility/pair.h
index 3ffab2f7fe4f6..380a2a21b9d4b 100644
--- a/libcxx/include/__utility/pair.h
+++ b/libcxx/include/__utility/pair.h
@@ -261,6 +261,7 @@ struct _LIBCPP_TEMPLATE_VIS pair
}
# if _LIBCPP_STD_VER >= 23
+ template <class = void>
_LIBCPP_HIDE_FROM_ABI constexpr const pair& operator=(pair const& __p) const
noexcept(is_nothrow_copy_assignable_v<const first_type> && is_nothrow_copy_assignable_v<const second_type>)
requires(is_copy_assignable_v<const first_type> && is_copy_assignable_v<const second_type>)
@@ -270,6 +271,7 @@ struct _LIBCPP_TEMPLATE_VIS pair
return *this;
}
+ template <class = void>
_LIBCPP_HIDE_FROM_ABI constexpr const pair& operator=(pair&& __p) const
noexcept(is_nothrow_assignable_v<const first_type&, first_type> &&
is_nothrow_assignable_v<const second_type&, second_type>)
diff --git a/libcxx/test/libcxx/utilities/utility/pairs/pairs.pair/non_trivial_copy_move_ABI.pass.cpp b/libcxx/test/libcxx/utilities/utility/pairs/pairs.pair/abi.non_trivial_copy_move.pass.cpp
similarity index 100%
rename from libcxx/test/libcxx/utilities/utility/pairs/pairs.pair/non_trivial_copy_move_ABI.pass.cpp
rename to libcxx/test/libcxx/utilities/utility/pairs/pairs.pair/abi.non_trivial_copy_move.pass.cpp
diff --git a/libcxx/test/libcxx/utilities/utility/pairs/pairs.pair/trivial_copy_move_ABI.pass.cpp b/libcxx/test/libcxx/utilities/utility/pairs/pairs.pair/abi.trivial_copy_move.pass.cpp
similarity index 79%
rename from libcxx/test/libcxx/utilities/utility/pairs/pairs.pair/trivial_copy_move_ABI.pass.cpp
rename to libcxx/test/libcxx/utilities/utility/pairs/pairs.pair/abi.trivial_copy_move.pass.cpp
index fe098a7af7e8f..3ec60c08b8eab 100644
--- a/libcxx/test/libcxx/utilities/utility/pairs/pairs.pair/trivial_copy_move_ABI.pass.cpp
+++ b/libcxx/test/libcxx/utilities/utility/pairs/pairs.pair/abi.trivial_copy_move.pass.cpp
@@ -71,6 +71,17 @@ struct Trivial {
static_assert(HasTrivialABI<Trivial>::value, "");
#endif
+struct TrivialNoAssignment {
+ int arr[4];
+ TrivialNoAssignment& operator=(const TrivialNoAssignment&) = delete;
+};
+
+struct TrivialNoConstruction {
+ int arr[4];
+ TrivialNoConstruction() = default;
+ TrivialNoConstruction(const TrivialNoConstruction&) = delete;
+ TrivialNoConstruction& operator=(const TrivialNoConstruction&) = default;
+};
void test_trivial()
{
@@ -135,6 +146,26 @@ void test_trivial()
static_assert(HasTrivialABI<P>::value, "");
}
#endif
+ {
+ using P = std::pair<TrivialNoAssignment, int>;
+ static_assert(std::is_trivially_copy_constructible<P>::value, "");
+ static_assert(std::is_trivially_move_constructible<P>::value, "");
+#if TEST_STD_VER >= 11 // This is https://llvm.org/PR90605
+ static_assert(!std::is_trivially_copy_assignable<P>::value, "");
+ static_assert(!std::is_trivially_move_assignable<P>::value, "");
+#endif // TEST_STD_VER >= 11
+ static_assert(std::is_trivially_destructible<P>::value, "");
+ }
+ {
+ using P = std::pair<TrivialNoConstruction, int>;
+#if TEST_STD_VER >= 11
+ static_assert(!std::is_trivially_copy_constructible<P>::value, "");
+ static_assert(!std::is_trivially_move_constructible<P>::value, "");
+#endif // TEST_STD_VER >= 11
+ static_assert(!std::is_trivially_copy_assignable<P>::value, "");
+ static_assert(!std::is_trivially_move_assignable<P>::value, "");
+ static_assert(std::is_trivially_destructible<P>::value, "");
+ }
}
void test_layout() {
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
new file mode 100644
index 0000000000000..86c014213947c
--- /dev/null
+++ b/libcxx/test/libcxx/utilities/utility/pairs/pairs.pair/abi.trivially_copyable.compile.pass.cpp
@@ -0,0 +1,56 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+//
+// This test pins down the ABI of std::pair with respect to being "trivially copyable".
+//
+
+#include <type_traits>
+#include <utility>
+
+#include "test_macros.h"
+
+struct trivially_copyable {
+ int arr[4];
+};
+
+struct trivially_copyable_no_assignment {
+ int arr[4];
+ trivially_copyable_no_assignment& operator=(const trivially_copyable_no_assignment&) = delete;
+};
+static_assert(std::is_trivially_copyable<trivially_copyable_no_assignment>::value, "");
+
+struct trivially_copyable_no_construction {
+ int arr[4];
+ trivially_copyable_no_construction() = default;
+ trivially_copyable_no_construction(const trivially_copyable_no_construction&) = delete;
+ trivially_copyable_no_construction& operator=(const trivially_copyable_no_construction&) = default;
+};
+static_assert(std::is_trivially_copyable<trivially_copyable_no_construction>::value, "");
+
+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), "");
+
+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), "");
+#if TEST_STD_VER == 03 // Known ABI difference
+static_assert((!std::is_trivially_copyable<std::pair<trivially_copyable_no_assignment, int> >::value), "");
+#else
+static_assert((std::is_trivially_copyable<std::pair<trivially_copyable_no_assignment, int> >::value), "");
+#endif
+static_assert((!std::is_trivially_copyable<std::pair<trivially_copyable_no_construction, int> >::value), "");
+
+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_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), "");
``````````
</details>
https://github.com/llvm/llvm-project/pull/95444
More information about the libcxx-commits
mailing list