diff --git a/include/llama/View.hpp b/include/llama/View.hpp index 7e08620dbd..14b91161be 100644 --- a/include/llama/View.hpp +++ b/include/llama/View.hpp @@ -585,24 +585,51 @@ namespace llama namespace internal { + // remove in C++23, from: https://en.cppreference.com/w/cpp/utility/forward_like + // NOLINTBEGIN + template + [[nodiscard]] constexpr auto&& forward_like(U&& x) noexcept + { + constexpr bool is_adding_const = std::is_const_v>; + if constexpr(std::is_lvalue_reference_v) + { + if constexpr(is_adding_const) + return std::as_const(x); + else + return static_cast(x); + } + else + { + if constexpr(is_adding_const) + return std::move(std::as_const(x)); + else + return std::move(x); + } + } + // NOLINTEND + template LLAMA_FN_HOST_ACC_INLINE auto makeTransformedBlobArray( - Blobs& blobs, + Blobs&& blobs, const TransformBlobFunc& transformBlob, std::integer_sequence) { - return llama::Array{transformBlob(blobs[Is])...}; + return llama::Array{transformBlob(forward_like(blobs[Is]))...}; } } // namespace internal /// Applies the given transformation to the blobs of a view and creates a new view with the transformed blobs and /// the same mapping and accessor as the old view. - template>>> - LLAMA_FN_HOST_ACC_INLINE auto transformBlobs(View& view, const TransformBlobFunc& transformBlob) + template>>> + LLAMA_FN_HOST_ACC_INLINE auto transformBlobs(ViewFwd&& view, const TransformBlobFunc& transformBlob) { - constexpr auto blobCount = std::decay_t::Mapping::blobCount; - auto blobs - = internal::makeTransformedBlobArray(view.blobs(), transformBlob, std::make_index_sequence{}); + using View = std::decay_t; + constexpr auto blobCount = View::Mapping::blobCount; + + auto blobs = internal::makeTransformedBlobArray( + internal::forward_like(view.blobs()), + transformBlob, + std::make_index_sequence{}); return llama::View{ view.mapping(), std::move(blobs), @@ -614,15 +641,15 @@ namespace llama /// \return A new view with the same mapping as view, where each blob refers to the blob in view. template< typename View, - typename NewBlobType = CopyConst*, + typename NewBlobType = CopyConst, std::byte>*, typename = std::enable_if_t>>> - LLAMA_FN_HOST_ACC_INLINE auto shallowCopy(View& view) + LLAMA_FN_HOST_ACC_INLINE auto shallowCopy(View&& view) { if constexpr(std::is_same_v::BlobType, NewBlobType>) return view; else return transformBlobs( - view, + std::forward(view), [](auto& blob) { LLAMA_BEGIN_SUPPRESS_HOST_DEVICE_WARNING @@ -632,33 +659,33 @@ namespace llama } // Creates a new view from an existing view with the given accessor. - // \param view A view which's mapping and blobs are copied into a new view with the different accessor. If you no - // longer need the old view, consider moving it into the argument of this function. - template - LLAMA_FN_HOST_ACC_INLINE auto withAccessor(View view, NewAccessor newAccessor = {}) + // \param view A view which's mapping and blobs are forwarded into a new view with the different accessor. + template>>> + LLAMA_FN_HOST_ACC_INLINE auto withAccessor(ViewFwd&& view, NewAccessor newAccessor = {}) { - return View{ - std::move(view.mapping()), - std::move(view.blobs()), + using OldView = std::decay_t; + return View{ + internal::forward_like(view.mapping()), + internal::forward_like(view.blobs()), std::move(newAccessor)}; } // Creates a new view from an existing view with the given mapping. - // \param view A view which's accessor and blobs are copied into a new view with the different mapping. If you no - // longer need the old view, consider moving it into the argument of this function. - template - LLAMA_FN_HOST_ACC_INLINE auto withMapping(View view, NewMapping newMapping = {}) + // \param view A view which's accessor and blobs are forwarded into a new view with the different mapping. + template>>> + LLAMA_FN_HOST_ACC_INLINE auto withMapping(ViewFwd&& view, NewMapping newMapping = {}) { - static_assert(Mapping::blobCount == NewMapping::blobCount); - for(std::size_t i = 0; i < Mapping::blobCount; i++) + using OldView = std::decay_t; + static_assert(OldView::Mapping::blobCount == NewMapping::blobCount); + for(std::size_t i = 0; i < NewMapping::blobCount; i++) { assert(view.mapping().blobSize(i) == newMapping.blobSize(i)); } - return View{ + return View{ std::move(newMapping), - std::move(view.blobs()), - std::move(view.accessor())}; + internal::forward_like(view.blobs()), + internal::forward_like(view.accessor())}; } /// Like a \ref View, but array indices are shifted. diff --git a/tests/view.cpp b/tests/view.cpp index eb12c60aa6..742f6c0451 100644 --- a/tests/view.cpp +++ b/tests/view.cpp @@ -4,7 +4,6 @@ #include "common.hpp" #include -#include namespace tag { @@ -311,18 +310,48 @@ TEST_CASE("view.indexing") }); } -TEST_CASE("view.transformBlobs") +TEST_CASE("view.transformBlobs.copy") { auto view = llama::allocView(llama::mapping::AoS{llama::ArrayExtents{16, 16}, Particle{}}); iotaFillView(view); auto copy = llama::transformBlobs( view, - [](auto& vector) { return std::deque(vector.begin(), vector.end()); }); - STATIC_REQUIRE(std::is_same_v, std::deque>); + [](auto& vector) + { + auto* p = reinterpret_cast(vector.data()); + return std::vector(p, p + vector.size()); + }); + STATIC_REQUIRE(std::is_same_v, std::vector>); + iotaCheckView(view); iotaCheckView(copy); } +TEST_CASE("view.transformBlobs.move") +{ + auto view = llama::allocView( + llama::mapping::AoS{llama::ArrayExtents{16, 16}, Particle{}}, + llama::bloballoc::UniquePtr{}); + iotaFillView(view); + + auto newDeleter + = [](unsigned char* ptr) { decltype(view)::BlobType::deleter_type{}(reinterpret_cast(ptr)); }; + auto moved = llama::transformBlobs( + std::move(view), + [&](auto&& p) + { + return std::unique_ptr{ + reinterpret_cast(p.release()), + newDeleter}; + }); + STATIC_REQUIRE(std::is_same_v< + std::decay_t, + std::unique_ptr>); + iotaCheckView(moved); + for(const auto& blob : view.blobs()) // NOLINT(bugprone-use-after-move,hicpp-invalid-access-moved) + CHECK(blob.get() == nullptr); +} + TEMPLATE_TEST_CASE( "view.shallowCopy", "",