From d943abdbabc1b7080aa5f0a2fff3e724135164dc Mon Sep 17 00:00:00 2001 From: Arthur O'Dwyer Date: Tue, 13 Feb 2024 11:59:52 -0500 Subject: [PATCH] absl::is_trivially_relocatable now respects assignment operators 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. --- absl/meta/type_traits.h | 21 +++++++++++++-------- absl/meta/type_traits_test.cc | 26 ++++++++++++++++++++++++-- 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/absl/meta/type_traits.h b/absl/meta/type_traits.h index d16dac9d979..92ad1e559c2 100644 --- a/absl/meta/type_traits.h +++ b/absl/meta/type_traits.h @@ -473,7 +473,7 @@ using swap_internal::StdSwapIsUnconstrained; // Detects whether a type is known to be "trivially relocatable" -- meaning it // 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 its move constructor and destructor don't do anything strange. +// and also none of its special member functions do anything strange. // // This trait is conservative. If it's true then the type is definitely // trivially relocatable, but there are many types which are "Platonically" @@ -514,22 +514,27 @@ 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: 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> {}; + : 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 10423eb5dd5..13a27985e93 100644 --- a/absl/meta/type_traits_test.cc +++ b/absl/meta/type_traits_test.cc @@ -780,6 +780,28 @@ 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&) { return *this; } // NOLINT(modernize-use-equals-default) + }; + + 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) { @@ -796,8 +818,8 @@ TEST(TriviallyRelocatable, UserProvidedDestructor) { // __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__) + (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 special members. TEST(TriviallyRelocatable, TrivialAbi) {