[libcxx-commits] [libcxx] [libc++] Fix triviality of std::pair for trivially copyable types without an assignment operator (PR #95444)
Louis Dionne via libcxx-commits
libcxx-commits at lists.llvm.org
Wed Jun 19 08:24:23 PDT 2024
https://github.com/ldionne updated https://github.com/llvm/llvm-project/pull/95444
>From 074e91ce94d24539cecec49fa75ea2b5903e3d5a Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Thu, 13 Jun 2024 13:31:55 -0400
Subject: [PATCH 1/5] [libc++] Fix triviality of std::pair for trivially
copyable types without an assignment operator
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
---
libcxx/include/__utility/pair.h | 2 +
...cpp => abi.non_trivial_copy_move.pass.cpp} | 0
...ass.cpp => abi.trivial_copy_move.pass.cpp} | 31 ++++++++++
.../abi.trivially_copyable.compile.pass.cpp | 56 +++++++++++++++++++
4 files changed, 89 insertions(+)
rename libcxx/test/libcxx/utilities/utility/pairs/pairs.pair/{non_trivial_copy_move_ABI.pass.cpp => abi.non_trivial_copy_move.pass.cpp} (100%)
rename libcxx/test/libcxx/utilities/utility/pairs/pairs.pair/{trivial_copy_move_ABI.pass.cpp => abi.trivial_copy_move.pass.cpp} (79%)
create mode 100644 libcxx/test/libcxx/utilities/utility/pairs/pairs.pair/abi.trivially_copyable.compile.pass.cpp
diff --git a/libcxx/include/__utility/pair.h b/libcxx/include/__utility/pair.h
index 7e17c4c415c41..6f293e1b5d65c 100644
--- a/libcxx/include/__utility/pair.h
+++ b/libcxx/include/__utility/pair.h
@@ -267,6 +267,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>)
@@ -276,6 +277,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), "");
>From e142dc166e1c5c060f420c112e339a07a2586468 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Mon, 17 Jun 2024 16:52:32 -0400
Subject: [PATCH 2/5] UNSUPPORT on FreeBSD
---
.../pairs/pairs.pair/abi.trivially_copyable.compile.pass.cpp | 4 ++++
libcxx/utils/libcxx/test/features.py | 1 +
2 files changed, 5 insertions(+)
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 86c014213947c..821c103519714 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
@@ -10,6 +10,10 @@
// This test pins down the ABI of std::pair with respect to being "trivially copyable".
//
+// This test doesn't work when the deprecated ABI to turn off pair triviality is enabled.
+// See libcxx/test/libcxx/utilities/utility/pairs/pairs.pair/abi.non_trivial_copy_move.pass.cpp instead.
+// UNSUPPORTED: libcpp-deprecated-abi-disable-pair-trivial-copy-ctor
+
#include <type_traits>
#include <utility>
diff --git a/libcxx/utils/libcxx/test/features.py b/libcxx/utils/libcxx/test/features.py
index 7a9631a56e4bb..a3395436a9bc0 100644
--- a/libcxx/utils/libcxx/test/features.py
+++ b/libcxx/utils/libcxx/test/features.py
@@ -352,6 +352,7 @@ def _mingwSupportsModules(cfg):
"_LIBCPP_NO_VCRUNTIME": "libcpp-no-vcruntime",
"_LIBCPP_ABI_VERSION": "libcpp-abi-version",
"_LIBCPP_ABI_BOUNDED_ITERATORS": "libcpp-has-abi-bounded-iterators",
+ "_LIBCPP_DEPRECATED_ABI_DISABLE_PAIR_TRIVIAL_COPY_CTOR": "libcpp-deprecated-abi-disable-pair-trivial-copy-ctor",
"_LIBCPP_HAS_NO_FILESYSTEM": "no-filesystem",
"_LIBCPP_HAS_NO_RANDOM_DEVICE": "no-random-device",
"_LIBCPP_HAS_NO_LOCALIZATION": "no-localization",
>From 4f86620b34fc60b780f9050ad03bc2ab135c22f1 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Mon, 17 Jun 2024 22:19:25 -0400
Subject: [PATCH 3/5] Try fixing ABI feature detection
---
libcxx/utils/libcxx/test/dsl.py | 4 ++--
libcxx/utils/libcxx/test/features.py | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/libcxx/utils/libcxx/test/dsl.py b/libcxx/utils/libcxx/test/dsl.py
index 7ac66d449b1cf..624a00d7e8ed3 100644
--- a/libcxx/utils/libcxx/test/dsl.py
+++ b/libcxx/utils/libcxx/test/dsl.py
@@ -308,8 +308,8 @@ def compilerMacros(config, flags=""):
with open(test.getSourcePath(), "w") as sourceFile:
sourceFile.write(
"""
- #if __has_include(<__config_site>)
- # include <__config_site>
+ #if __has_include(<__config>)
+ # include <__config>
#endif
"""
)
diff --git a/libcxx/utils/libcxx/test/features.py b/libcxx/utils/libcxx/test/features.py
index a3395436a9bc0..28c9c35ae9f87 100644
--- a/libcxx/utils/libcxx/test/features.py
+++ b/libcxx/utils/libcxx/test/features.py
@@ -335,10 +335,10 @@ def _mingwSupportsModules(cfg):
]
# Deduce and add the test features that that are implied by the #defines in
-# the <__config_site> header.
+# the <__config> header.
#
# For each macro of the form `_LIBCPP_XXX_YYY_ZZZ` defined below that
-# is defined after including <__config_site>, add a Lit feature called
+# is defined after including <__config>, add a Lit feature called
# `libcpp-xxx-yyy-zzz`. When a macro is defined to a specific value
# (e.g. `_LIBCPP_ABI_VERSION=2`), the feature is `libcpp-xxx-yyy-zzz=<value>`.
#
>From b6bfc176792f9bbd5a1dd4e6df32bbd543166c5c Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Wed, 19 Jun 2024 11:22:29 -0400
Subject: [PATCH 4/5] Remove double parens
---
.../abi.trivially_copyable.compile.pass.cpp | 34 +++++++++----------
1 file changed, 17 insertions(+), 17 deletions(-)
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 821c103519714..735bbd771d1b6 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
@@ -37,24 +37,24 @@ struct trivially_copyable_no_construction {
};
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), "");
+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), "");
+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), "");
+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_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), "");
+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, "");
>From d503bcf717a83a9ceb72916cb8267abc3b81108f Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Wed, 19 Jun 2024 11:23:51 -0400
Subject: [PATCH 5/5] Add test for move assignment
---
.../abi.trivially_copyable.compile.pass.cpp | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
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 735bbd771d1b6..1132b3e5def18 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
@@ -23,11 +23,17 @@ struct trivially_copyable {
int arr[4];
};
-struct trivially_copyable_no_assignment {
+struct trivially_copyable_no_copy_assignment {
int arr[4];
- trivially_copyable_no_assignment& operator=(const trivially_copyable_no_assignment&) = delete;
+ trivially_copyable_no_copy_assignment& operator=(const trivially_copyable_no_copy_assignment&) = delete;
};
-static_assert(std::is_trivially_copyable<trivially_copyable_no_assignment>::value, "");
+static_assert(std::is_trivially_copyable<trivially_copyable_no_copy_assignment>::value, "");
+
+struct trivially_copyable_no_move_assignment {
+ int arr[4];
+ trivially_copyable_no_move_assignment& operator=(const trivially_copyable_no_move_assignment&) = delete;
+};
+static_assert(std::is_trivially_copyable<trivially_copyable_no_move_assignment>::value, "");
struct trivially_copyable_no_construction {
int arr[4];
@@ -47,9 +53,11 @@ 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, "");
+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, "");
#else
-static_assert(std::is_trivially_copyable<std::pair<trivially_copyable_no_assignment, int> >::value, "");
+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
static_assert(!std::is_trivially_copyable<std::pair<trivially_copyable_no_construction, int> >::value, "");
More information about the libcxx-commits
mailing list