[llvm-branch-commits] [libcxx] release/20.x: [libc++] Replace __is_trivially_relocatable by is_trivially_copyable (#124970) (PR #125996)

[via llvm-branch-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libcxx

Author: None (llvmbot)

<details>
<summary>Changes</summary>

Backport accfbd4cb327411ad66c0109ba1841482b871967

Requested by: @<!-- -->ldionne

---
Full diff: https://github.com/llvm/llvm-project/pull/125996.diff


3 Files Affected:

- (modified) libcxx/include/__type_traits/is_trivially_relocatable.h (+5-3) 
- (modified) libcxx/test/libcxx/type_traits/is_trivially_relocatable.compile.pass.cpp (+10-4) 
- (added) libcxx/test/std/containers/sequences/vector/trivial_relocation.pass.cpp (+71) 


``````````diff
diff --git a/libcxx/include/__type_traits/is_trivially_relocatable.h b/libcxx/include/__type_traits/is_trivially_relocatable.h
index c0871731cc0016b..9b0e240de55f4ec 100644
--- a/libcxx/include/__type_traits/is_trivially_relocatable.h
+++ b/libcxx/include/__type_traits/is_trivially_relocatable.h
@@ -11,7 +11,6 @@
 
 #include <__config>
 #include <__type_traits/enable_if.h>
-#include <__type_traits/integral_constant.h>
 #include <__type_traits/is_same.h>
 #include <__type_traits/is_trivially_copyable.h>
 
@@ -23,8 +22,11 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 
 // A type is trivially relocatable if a move construct + destroy of the original object is equivalent to
 // `memcpy(dst, src, sizeof(T))`.
-
-#if __has_builtin(__is_trivially_relocatable)
+//
+// Note that we don't use the __is_trivially_relocatable Clang builtin right now because it does not
+// implement the semantics of any current or future trivial relocation proposal and it can lead to
+// incorrect optimizations on some platforms (Windows) and supported compilers (AppleClang).
+#if __has_builtin(__is_trivially_relocatable) && 0
 template <class _Tp, class = void>
 struct __libcpp_is_trivially_relocatable : integral_constant<bool, __is_trivially_relocatable(_Tp)> {};
 #else
diff --git a/libcxx/test/libcxx/type_traits/is_trivially_relocatable.compile.pass.cpp b/libcxx/test/libcxx/type_traits/is_trivially_relocatable.compile.pass.cpp
index 674df1d0219057d..213d06d314a075f 100644
--- a/libcxx/test/libcxx/type_traits/is_trivially_relocatable.compile.pass.cpp
+++ b/libcxx/test/libcxx/type_traits/is_trivially_relocatable.compile.pass.cpp
@@ -54,11 +54,17 @@ struct MoveOnlyTriviallyCopyable {
   MoveOnlyTriviallyCopyable(MoveOnlyTriviallyCopyable&&)                 = default;
   MoveOnlyTriviallyCopyable& operator=(MoveOnlyTriviallyCopyable&&)      = default;
 };
-#ifndef _MSC_VER
 static_assert(std::__libcpp_is_trivially_relocatable<MoveOnlyTriviallyCopyable>::value, "");
-#else
-static_assert(!std::__libcpp_is_trivially_relocatable<MoveOnlyTriviallyCopyable>::value, "");
-#endif
+
+struct NonTrivialMoveConstructor {
+  NonTrivialMoveConstructor(NonTrivialMoveConstructor&&);
+};
+static_assert(!std::__libcpp_is_trivially_relocatable<NonTrivialMoveConstructor>::value, "");
+
+struct NonTrivialDestructor {
+  ~NonTrivialDestructor() {}
+};
+static_assert(!std::__libcpp_is_trivially_relocatable<NonTrivialDestructor>::value, "");
 
 // library-internal types
 // ----------------------
diff --git a/libcxx/test/std/containers/sequences/vector/trivial_relocation.pass.cpp b/libcxx/test/std/containers/sequences/vector/trivial_relocation.pass.cpp
new file mode 100644
index 000000000000000..fbd597d07d6e32e
--- /dev/null
+++ b/libcxx/test/std/containers/sequences/vector/trivial_relocation.pass.cpp
@@ -0,0 +1,71 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// <vector>
+
+// Make sure we don't miscompile vector operations for types that shouldn't be considered
+// trivially relocatable.
+
+#include <vector>
+#include <cassert>
+#include <cstddef>
+#include <type_traits>
+#include <utility>
+
+#include "test_macros.h"
+
+struct Tracker {
+  std::size_t move_constructs = 0;
+};
+
+struct [[clang::trivial_abi]] Inner {
+  TEST_CONSTEXPR_CXX20 explicit Inner(Tracker* tracker) : tracker_(tracker) {}
+  TEST_CONSTEXPR_CXX20 Inner(const Inner& rhs) : tracker_(rhs.tracker_) { tracker_->move_constructs += 1; }
+  TEST_CONSTEXPR_CXX20 Inner(Inner&& rhs) : tracker_(rhs.tracker_) { tracker_->move_constructs += 1; }
+  Tracker* tracker_;
+};
+
+// Even though this type contains a trivial_abi type, it is not trivially move-constructible,
+// so we should not attempt to optimize its move construction + destroy using trivial relocation.
+struct NotTriviallyMovable {
+  TEST_CONSTEXPR_CXX20 explicit NotTriviallyMovable(Tracker* tracker) : inner_(tracker) {}
+  TEST_CONSTEXPR_CXX20 NotTriviallyMovable(NotTriviallyMovable&& other) : inner_(std::move(other.inner_)) {}
+  Inner inner_;
+};
+static_assert(!std::is_trivially_copyable<NotTriviallyMovable>::value, "");
+LIBCPP_STATIC_ASSERT(!std::__libcpp_is_trivially_relocatable<NotTriviallyMovable>::value, "");
+
+TEST_CONSTEXPR_CXX20 bool tests() {
+  Tracker track;
+  std::vector<NotTriviallyMovable> v;
+
+  // Fill the vector at its capacity, such that any subsequent push_back would require growing.
+  v.reserve(5);
+  std::size_t const capacity = v.capacity(); // could technically be more than 5
+  while (v.size() < v.capacity()) {
+    v.emplace_back(&track);
+  }
+  assert(track.move_constructs == 0);
+  assert(v.capacity() == capacity);
+  assert(v.size() == capacity);
+
+  // Force a reallocation of the buffer + relocalization of the elements.
+  // All the existing elements of the vector should be move-constructed to their new location.
+  v.emplace_back(&track);
+  assert(track.move_constructs == capacity);
+
+  return true;
+}
+
+int main(int, char**) {
+  tests();
+#if TEST_STD_VER >= 20
+  static_assert(tests());
+#endif
+  return 0;
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/125996