Skip to content

Commit

Permalink
type_traits: don't use __is_trivially_relocatable with Clang on Windows.
Browse files Browse the repository at this point in the history
It currently gives the wrong result for types with a user-provided destructor:

    struct S {
      ~S();
    };

    static_assert(!__is_trivially_relocatable(S));

PiperOrigin-RevId: 519855600
Change-Id: I5f0659ad0831974805a5ed4b615e3b6250d23154
  • Loading branch information
jacobsa authored and Copybara-Service committed Mar 27, 2023
1 parent 0dc9430 commit 6b4af24
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 19 deletions.
11 changes: 9 additions & 2 deletions absl/meta/type_traits.h
Expand Up @@ -495,11 +495,18 @@ using swap_internal::StdSwapIsUnconstrained;
// Upstream documentation:
//
// https://clang.llvm.org/docs/LanguageExtensions.html#:~:text=__is_trivially_relocatable
//
#if ABSL_HAVE_BUILTIN(__is_trivially_relocatable)

// If the compiler offers a builtin that tells us the answer, we can use that.
// This covers all of the cases in the fallback below, plus types that opt in
// using e.g. [[clang::trivial_abi]].
//
// Clang on Windows has the builtin, but it falsely claims types with a
// user-provided destructor are trivial (http://b/275003464). So we opt out
// there.
//
// TODO(b/275003464): remove the opt-out once the bug is fixed.
#if ABSL_HAVE_BUILTIN(__is_trivially_relocatable) && \
!(defined(__clang__) && (defined(_WIN32) || defined(_WIN64)))
template <class T>
struct is_trivially_relocatable
: std::integral_constant<bool, __is_trivially_relocatable(T)> {};
Expand Down
22 changes: 5 additions & 17 deletions absl/meta/type_traits_test.cc
Expand Up @@ -780,20 +780,6 @@ TEST(TriviallyRelocatable, UserProvidedCopyConstructor) {
static_assert(!absl::is_trivially_relocatable<S>::value, "");
}

// HACK: disable this test on Windows, which includes lexan, which gives the
// incorrect result for this case (http://b/275003464).
//
// TODO(b/275003464): make this hack more precise after figuring out how to
// detect --config=lexan in particular. If there is no reliable way, modify
// --config=lexan itself to set a define we can use.
//
// TODO(b/274984172): hoist this hack to the definition of
// absl::is_trivially_relocatable itself, since it's currently giving the wrong
// results under lexan. We can't trust __is_trivially_relocatable on that
// platform.
//
// TODO(b/275003464): remove this hack when the bug is fixed.
#if !(defined(_WIN32) || defined(_WIN64))
// A user-provided destructor disqualifies a type from being trivially
// relocatable.
TEST(TriviallyRelocatable, UserProvidedDestructor) {
Expand All @@ -803,10 +789,12 @@ TEST(TriviallyRelocatable, UserProvidedDestructor) {

static_assert(!absl::is_trivially_relocatable<S>::value, "");
}
#endif

#if defined(ABSL_HAVE_ATTRIBUTE_TRIVIAL_ABI) && \
ABSL_HAVE_BUILTIN(__is_trivially_relocatable)
// TODO(b/275003464): remove the opt-out for Clang on Windows once
// __is_trivially_relocatable is used there again.
#if defined(ABSL_HAVE_ATTRIBUTE_TRIVIAL_ABI) && \
ABSL_HAVE_BUILTIN(__is_trivially_relocatable) && \
!(defined(__clang__) && (defined(_WIN32) || defined(_WIN64)))
// 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.
Expand Down

0 comments on commit 6b4af24

Please sign in to comment.