From af87d19ae7d59e0b90b573b529175cbc607ac944 Mon Sep 17 00:00:00 2001 From: Arthur O'Dwyer Date: Sat, 17 Feb 2024 09:32:01 -0800 Subject: [PATCH] PR #1625: absl::is_trivially_relocatable now respects assignment operators Imported from GitHub PR https://github.com/abseil/abseil-cpp/pull/1625 Trivial relocatability also requires that the type not do anything weird with its assignment operator; update the type-trait to reflect this. (This is the definition used by BSL, Folly, HPX, Thrust, Parlay, Amadeus, and P1144.) This is important if we want to use `absl::is_trivially_relocatable` as a gate for memcpy optimizations in `inlined_vector::erase` and/or `inlined_vector::swap`, because in those cases relocation is used to replace part of a sequence involving assignment; the optimization requires an assignment operator that behaves value-semantically. Clang's builtin currently fails to check the assignment operator, so we stop using it entirely for now. We already refused to use it on Win32, Win64, and Apple, for various unrelated reasons. I'm working on giving Clang's builtin the behavior that would let us re-enable it here. Assume that any compiler providing both `__cpp_impl_trivially_relocatable` and a builtin `__is_trivially_relocatable(T)` will use the appropriate (P1144) definition for its builtin. Right now there's only one such compiler (the P1144 reference implementation, which forks Clang), so this is largely a moot point, but I'm being optimistic. Merge d943abdbabc1b7080aa5f0a2fff3e724135164dc into 34604d5b1f6ae14c65b3992478b59f7108051979 Merging this change closes #1625 COPYBARA_INTEGRATE_REVIEW=https://github.com/abseil/abseil-cpp/pull/1625 from Quuxplusone:trivially-relocatable d943abdbabc1b7080aa5f0a2fff3e724135164dc PiperOrigin-RevId: 607977323 Change-Id: I6436a60326c6d1064bdd71ec2e15b86b7a29efd4 --- absl/meta/type_traits.h | 50 ++++++++++++++++------------- absl/meta/type_traits_test.cc | 59 ++++++++++++++++++++++++----------- 2 files changed, 70 insertions(+), 39 deletions(-) diff --git a/absl/meta/type_traits.h b/absl/meta/type_traits.h index 438bfc76613..88633b25d30 100644 --- a/absl/meta/type_traits.h +++ b/absl/meta/type_traits.h @@ -152,8 +152,8 @@ template struct disjunction : std::false_type {}; template -struct disjunction : - std::conditional>::type {}; +struct disjunction + : std::conditional>::type {}; template struct disjunction : T {}; @@ -315,22 +315,23 @@ using common_type_t = typename std::common_type::type; template using underlying_type_t = typename std::underlying_type::type; - namespace type_traits_internal { #if (defined(__cpp_lib_is_invocable) && __cpp_lib_is_invocable >= 201703L) || \ (defined(_MSVC_LANG) && _MSVC_LANG >= 201703L) // std::result_of is deprecated (C++17) or removed (C++20) -template struct result_of; -template +template +struct result_of; +template struct result_of : std::invoke_result {}; #else -template using result_of = std::result_of; +template +using result_of = std::result_of; #endif } // namespace type_traits_internal -template +template using result_of_t = typename type_traits_internal::result_of::type; namespace type_traits_internal { @@ -463,20 +464,23 @@ namespace type_traits_internal { // Make the swap-related traits/function accessible from this namespace. using swap_internal::IsNothrowSwappable; using swap_internal::IsSwappable; -using swap_internal::Swap; using swap_internal::StdSwapIsUnconstrained; +using swap_internal::Swap; } // namespace type_traits_internal // absl::is_trivially_relocatable // // Detects whether a type is known to be "trivially relocatable" -- meaning it -// can be relocated without invoking the constructor/destructor, using a form of -// move elision. +// can be relocated from one place to another as if by memcpy/memmove. +// This implies that its object representation doesn't depend on its address, +// and also none of its special member functions do anything strange. // -// This trait is conservative, for backwards compatibility. If it's true then -// the type is definitely trivially relocatable, but if it's false then the type -// may or may not be. +// This trait is conservative. If it's true then the type is definitely +// trivially relocatable, but if it's false then the type may or may not be. For +// example, std::vector is trivially relocatable on every known STL +// implementation, but absl::is_trivially_relocatable> remains +// false. // // Example: // @@ -509,22 +513,26 @@ using swap_internal::StdSwapIsUnconstrained; // TODO(b/324278148): If all versions we use have the bug fixed, then // remove the condition. // +// Clang on all platforms fails to detect that a type with a user-provided +// move-assignment operator is not trivially relocatable. So in fact we +// opt out of Clang altogether, for now. +// +// TODO(b/325479096): Remove the opt-out once Clang's behavior is fixed. +// // According to https://github.com/abseil/abseil-cpp/issues/1479, this does not // work with NVCC either. -#if ABSL_HAVE_BUILTIN(__is_trivially_relocatable) && \ - !(defined(__clang__) && (defined(_WIN32) || defined(_WIN64))) && \ - !(defined(__APPLE__)) && !defined(__NVCC__) +#if ABSL_HAVE_BUILTIN(__is_trivially_relocatable) && \ + (defined(__cpp_impl_trivially_relocatable) || \ + (!defined(__clang__) && !defined(__APPLE__) && !defined(__NVCC__))) template struct is_trivially_relocatable : std::integral_constant {}; #else // Otherwise we use a fallback that detects only those types we can feasibly -// detect. Any time that has trivial move-construction and destruction -// operations is by definition trivially relocatable. +// detect. Any type that is trivially copyable is by definition trivially +// relocatable. template -struct is_trivially_relocatable - : absl::conjunction, - absl::is_trivially_destructible> {}; +struct is_trivially_relocatable : std::is_trivially_copyable {}; #endif // absl::is_constant_evaluated() diff --git a/absl/meta/type_traits_test.cc b/absl/meta/type_traits_test.cc index 8f926901244..25f5abbce70 100644 --- a/absl/meta/type_traits_test.cc +++ b/absl/meta/type_traits_test.cc @@ -362,8 +362,8 @@ TEST(TypeTraitsTest, TestIsFunction) { EXPECT_TRUE(absl::is_function::value); EXPECT_TRUE(absl::is_function::value); - EXPECT_FALSE(absl::is_function::value); - EXPECT_FALSE(absl::is_function::value); + EXPECT_FALSE(absl::is_function::value); + EXPECT_FALSE(absl::is_function::value); EXPECT_FALSE(absl::is_function::value); EXPECT_FALSE(absl::is_function::value); } @@ -382,8 +382,8 @@ TEST(TypeTraitsTest, TestRemoveCVRef) { // Does not remove const in this case. EXPECT_TRUE((std::is_same::type, const int*>::value)); - EXPECT_TRUE((std::is_same::type, - int[2]>::value)); + EXPECT_TRUE( + (std::is_same::type, int[2]>::value)); EXPECT_TRUE((std::is_same::type, int[2]>::value)); EXPECT_TRUE((std::is_same::type, @@ -580,7 +580,7 @@ TEST(TypeTraitsTest, TestDecay) { ABSL_INTERNAL_EXPECT_ALIAS_EQUIVALENCE(decay, int[][1]); ABSL_INTERNAL_EXPECT_ALIAS_EQUIVALENCE(decay, int()); - ABSL_INTERNAL_EXPECT_ALIAS_EQUIVALENCE(decay, int(float)); // NOLINT + ABSL_INTERNAL_EXPECT_ALIAS_EQUIVALENCE(decay, int(float)); // NOLINT ABSL_INTERNAL_EXPECT_ALIAS_EQUIVALENCE(decay, int(char, ...)); // NOLINT } @@ -664,8 +664,7 @@ TEST(TypeTraitsTest, TestResultOf) { namespace adl_namespace { -struct DeletedSwap { -}; +struct DeletedSwap {}; void swap(DeletedSwap&, DeletedSwap&) = delete; @@ -751,7 +750,7 @@ TEST(TriviallyRelocatable, PrimitiveTypes) { // User-defined types can be trivially relocatable as long as they don't have a // user-provided move constructor or destructor. -TEST(TriviallyRelocatable, UserDefinedTriviallyReconstructible) { +TEST(TriviallyRelocatable, UserDefinedTriviallyRelocatable) { struct S { int x; int y; @@ -780,6 +779,30 @@ TEST(TriviallyRelocatable, UserProvidedCopyConstructor) { static_assert(!absl::is_trivially_relocatable::value, ""); } +// A user-provided copy assignment operator disqualifies a type from +// being trivially relocatable. +TEST(TriviallyRelocatable, UserProvidedCopyAssignment) { + struct S { + S(const S&) = default; + S& operator=(const S&) { // NOLINT(modernize-use-equals-default) + return *this; + } + }; + + static_assert(!absl::is_trivially_relocatable::value, ""); +} + +// A user-provided move assignment operator disqualifies a type from +// being trivially relocatable. +TEST(TriviallyRelocatable, UserProvidedMoveAssignment) { + struct S { + S(S&&) = default; + S& operator=(S&&) { return *this; } // NOLINT(modernize-use-equals-default) + }; + + static_assert(!absl::is_trivially_relocatable::value, ""); +} + // A user-provided destructor disqualifies a type from being trivially // relocatable. TEST(TriviallyRelocatable, UserProvidedDestructor) { @@ -794,18 +817,19 @@ TEST(TriviallyRelocatable, UserProvidedDestructor) { // __is_trivially_relocatable is used there again. // TODO(b/324278148): remove the opt-out for Apple once // __is_trivially_relocatable is fixed there. -#if defined(ABSL_HAVE_ATTRIBUTE_TRIVIAL_ABI) && \ - ABSL_HAVE_BUILTIN(__is_trivially_relocatable) && \ - !(defined(__clang__) && (defined(_WIN32) || defined(_WIN64))) && \ - !defined(__APPLE__) +#if defined(ABSL_HAVE_ATTRIBUTE_TRIVIAL_ABI) && \ + ABSL_HAVE_BUILTIN(__is_trivially_relocatable) && \ + (defined(__cpp_impl_trivially_relocatable) || \ + (!defined(__clang__) && !defined(__APPLE__) && !defined(__NVCC__))) // A type marked with the "trivial ABI" attribute is trivially relocatable even -// if it has user-provided move/copy constructors and a user-provided -// destructor. -TEST(TrivallyRelocatable, TrivialAbi) { +// if it has user-provided special members. +TEST(TriviallyRelocatable, TrivialAbi) { struct ABSL_ATTRIBUTE_TRIVIAL_ABI S { S(S&&) {} // NOLINT(modernize-use-equals-default) S(const S&) {} // NOLINT(modernize-use-equals-default) - ~S() {} // NOLINT(modernize-use-equals-default) + void operator=(S&&) {} + void operator=(const S&) {} + ~S() {} // NOLINT(modernize-use-equals-default) }; static_assert(absl::is_trivially_relocatable::value, ""); @@ -824,7 +848,7 @@ constexpr int64_t NegateIfConstantEvaluated(int64_t i) { #endif // ABSL_HAVE_CONSTANT_EVALUATED -TEST(TrivallyRelocatable, is_constant_evaluated) { +TEST(IsConstantEvaluated, is_constant_evaluated) { #ifdef ABSL_HAVE_CONSTANT_EVALUATED constexpr int64_t constant = NegateIfConstantEvaluated(42); EXPECT_EQ(constant, -42); @@ -840,5 +864,4 @@ TEST(TrivallyRelocatable, is_constant_evaluated) { #endif // ABSL_HAVE_CONSTANT_EVALUATED } - } // namespace