Skip to content

Commit 4198d9a

Browse files
ezbrcopybara-github
authored andcommitted
Change the definition of is_trivially_relocatable to be a bit less conservative.
Instead of ignoring __is_trivially_relocatable, use __is_trivially_relocatable && std::is_trivially_move_assignable. PiperOrigin-RevId: 709106861 Change-Id: I2d53bd440133ad17bb676e53ee6a17d420a565d1
1 parent 90a7ba6 commit 4198d9a

File tree

2 files changed

+37
-5
lines changed

2 files changed

+37
-5
lines changed

absl/meta/type_traits.h

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -503,10 +503,11 @@ using swap_internal::Swap;
503503
// remove the condition.
504504
//
505505
// Clang on all platforms fails to detect that a type with a user-provided
506-
// move-assignment operator is not trivially relocatable. So in fact we
507-
// opt out of Clang altogether, for now.
506+
// move-assignment operator is not trivially relocatable so we also check for
507+
// is_trivially_move_assignable for Clang.
508508
//
509-
// TODO(b/325479096): Remove the opt-out once Clang's behavior is fixed.
509+
// TODO(b/325479096): Remove the Clang is_trivially_move_assignable version once
510+
// Clang's behavior is fixed.
510511
//
511512
// According to https://github.com/abseil/abseil-cpp/issues/1479, this does not
512513
// work with NVCC either.
@@ -516,6 +517,15 @@ using swap_internal::Swap;
516517
template <class T>
517518
struct is_trivially_relocatable
518519
: std::integral_constant<bool, __is_trivially_relocatable(T)> {};
520+
#elif ABSL_HAVE_BUILTIN(__is_trivially_relocatable) && defined(__clang__) && \
521+
!(defined(_WIN32) || defined(_WIN64)) && !defined(__APPLE__) && \
522+
!defined(__NVCC__)
523+
template <class T>
524+
struct is_trivially_relocatable
525+
: std::integral_constant<
526+
bool, std::is_trivially_copyable<T>::value ||
527+
(__is_trivially_relocatable(T) &&
528+
std::is_trivially_move_assignable<T>::value)> {};
519529
#else
520530
// Otherwise we use a fallback that detects only those types we can feasibly
521531
// detect. Any type that is trivially copyable is by definition trivially

absl/meta/type_traits_test.cc

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -769,6 +769,8 @@ TEST(TriviallyRelocatable, UserProvidedDestructor) {
769769
// __is_trivially_relocatable is used there again.
770770
// TODO(b/324278148): remove the opt-out for Apple once
771771
// __is_trivially_relocatable is fixed there.
772+
// TODO(b/325479096): remove the opt-out for Clang once
773+
// __is_trivially_relocatable is fixed there.
772774
#if defined(ABSL_HAVE_ATTRIBUTE_TRIVIAL_ABI) && \
773775
ABSL_HAVE_BUILTIN(__is_trivially_relocatable) && \
774776
(defined(__cpp_impl_trivially_relocatable) || \
@@ -779,8 +781,28 @@ TEST(TriviallyRelocatable, TrivialAbi) {
779781
struct ABSL_ATTRIBUTE_TRIVIAL_ABI S {
780782
S(S&&) {} // NOLINT(modernize-use-equals-default)
781783
S(const S&) {} // NOLINT(modernize-use-equals-default)
782-
void operator=(S&&) {}
783-
void operator=(const S&) {}
784+
S& operator=(S&&) { return *this; }
785+
S& operator=(const S&) { return *this; }
786+
~S() {} // NOLINT(modernize-use-equals-default)
787+
};
788+
789+
static_assert(absl::is_trivially_relocatable<S>::value, "");
790+
}
791+
#endif
792+
793+
// TODO(b/275003464): remove the opt-out for Clang on Windows once
794+
// __is_trivially_relocatable is used there again.
795+
// TODO(b/324278148): remove the opt-out for Apple once
796+
// __is_trivially_relocatable is fixed there.
797+
#if defined(ABSL_HAVE_ATTRIBUTE_TRIVIAL_ABI) && \
798+
ABSL_HAVE_BUILTIN(__is_trivially_relocatable) && defined(__clang__) && \
799+
!(defined(_WIN32) || defined(_WIN64)) && !defined(__APPLE__) && \
800+
!defined(__NVCC__)
801+
// A type marked with the "trivial ABI" attribute is trivially relocatable even
802+
// if it has a user-provided copy constructor and a user-provided destructor.
803+
TEST(TriviallyRelocatable, TrivialAbi_NoUserProvidedMove) {
804+
struct ABSL_ATTRIBUTE_TRIVIAL_ABI S {
805+
S(const S&) {} // NOLINT(modernize-use-equals-default)
784806
~S() {} // NOLINT(modernize-use-equals-default)
785807
};
786808

0 commit comments

Comments
 (0)