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 _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
