Skip to content

Commit

Permalink
[libc++] Fix triviality of std::pair for trivially copyable types wit…
Browse files Browse the repository at this point in the history
…hout an assignment operator (llvm#95444)

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 llvm#89652 that pin down the ABI of
std::pair with respect to being trivially copyable.

Fixes llvm#95428
  • Loading branch information
ldionne authored Jun 19, 2024
1 parent eb8d036 commit 02af67c
Show file tree
Hide file tree
Showing 6 changed files with 106 additions and 4 deletions.
2 changes: 2 additions & 0 deletions libcxx/include/__utility/pair.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>)
Expand All @@ -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>)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
//===----------------------------------------------------------------------===//
//
// 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".
//

// 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>

#include "test_macros.h"

struct trivially_copyable {
int arr[4];
};

struct trivially_copyable_no_copy_assignment {
int arr[4];
trivially_copyable_no_copy_assignment& operator=(const trivially_copyable_no_copy_assignment&) = delete;
};
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];
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_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_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, "");

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, "");
4 changes: 2 additions & 2 deletions libcxx/utils/libcxx/test/dsl.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
"""
)
Expand Down
5 changes: 3 additions & 2 deletions libcxx/utils/libcxx/test/features.py
Original file line number Diff line number Diff line change
Expand Up @@ -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>`.
#
Expand All @@ -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",
Expand Down

0 comments on commit 02af67c

Please sign in to comment.