diff options
author | 2022-10-12 05:52:24 -0700 | |
---|---|---|
committer | 2022-10-12 05:53:14 -0700 | |
commit | 90965f4c9662a73f2eb9c345b3a5431f40fd86d3 (patch) | |
tree | 64107a9f2f6eb2baf62aa1b89d4c9c13c0ae6aa8 | |
parent | 91b7cd600a34cbd318c87196d7147c41197f8161 (diff) |
`absl::InlinedVector` supports move assignment with non-assignable types.
PiperOrigin-RevId: 480601268
Change-Id: I5a639da57b79ae600387c81e662d5c1542b2bf99
-rw-r--r-- | absl/container/BUILD.bazel | 1 | ||||
-rw-r--r-- | absl/container/CMakeLists.txt | 1 | ||||
-rw-r--r-- | absl/container/inlined_vector.h | 61 | ||||
-rw-r--r-- | absl/container/inlined_vector_test.cc | 111 | ||||
-rw-r--r-- | absl/container/internal/inlined_vector.h | 3 |
5 files changed, 165 insertions, 12 deletions
diff --git a/absl/container/BUILD.bazel b/absl/container/BUILD.bazel index 6b4cedd2..d8dad8e8 100644 --- a/absl/container/BUILD.bazel +++ b/absl/container/BUILD.bazel @@ -134,6 +134,7 @@ cc_library( "//absl/base:core_headers", "//absl/base:throw_delegate", "//absl/memory", + "//absl/meta:type_traits", ], ) diff --git a/absl/container/CMakeLists.txt b/absl/container/CMakeLists.txt index 6c2931b6..25216466 100644 --- a/absl/container/CMakeLists.txt +++ b/absl/container/CMakeLists.txt @@ -194,6 +194,7 @@ absl_cc_library( absl::inlined_vector_internal absl::throw_delegate absl::memory + absl::type_traits PUBLIC ) diff --git a/absl/container/inlined_vector.h b/absl/container/inlined_vector.h index 60f12460..10b1896b 100644 --- a/absl/container/inlined_vector.h +++ b/absl/container/inlined_vector.h @@ -52,6 +52,7 @@ #include "absl/base/port.h" #include "absl/container/internal/inlined_vector.h" #include "absl/memory/memory.h" +#include "absl/meta/type_traits.h" namespace absl { ABSL_NAMESPACE_BEGIN @@ -77,6 +78,8 @@ class InlinedVector { using MoveIterator = inlined_vector_internal::MoveIterator<TheA>; template <typename TheA> using IsMemcpyOk = inlined_vector_internal::IsMemcpyOk<TheA>; + template <typename TheA> + using IsMoveAssignOk = inlined_vector_internal::IsMoveAssignOk<TheA>; template <typename TheA, typename Iterator> using IteratorValueAdapter = @@ -94,6 +97,15 @@ class InlinedVector { using DisableIfAtLeastForwardIterator = absl::enable_if_t< !inlined_vector_internal::IsAtLeastForwardIterator<Iterator>::value, int>; + struct MemcpyPolicy {}; + struct ElementwiseAssignPolicy {}; + struct ElementwiseConstructPolicy {}; + + using MoveAssignmentPolicy = absl::conditional_t< + IsMemcpyOk<A>::value, MemcpyPolicy, + absl::conditional_t<IsMoveAssignOk<A>::value, ElementwiseAssignPolicy, + ElementwiseConstructPolicy>>; + public: using allocator_type = A; using value_type = inlined_vector_internal::ValueType<A>; @@ -486,18 +498,7 @@ class InlinedVector { // unspecified state. InlinedVector& operator=(InlinedVector&& other) { if (ABSL_PREDICT_TRUE(this != std::addressof(other))) { - if (IsMemcpyOk<A>::value || other.storage_.GetIsAllocated()) { - inlined_vector_internal::DestroyAdapter<A>::DestroyElements( - storage_.GetAllocator(), data(), size()); - storage_.DeallocateIfAllocated(); - storage_.MemcpyFrom(other.storage_); - - other.storage_.SetInlinedSize(0); - } else { - storage_.Assign(IteratorValueAdapter<A, MoveIterator<A>>( - MoveIterator<A>(other.storage_.GetInlinedData())), - other.size()); - } + MoveAssignment(MoveAssignmentPolicy{}, std::move(other)); } return *this; @@ -773,6 +774,42 @@ class InlinedVector { template <typename H, typename TheT, size_t TheN, typename TheA> friend H AbslHashValue(H h, const absl::InlinedVector<TheT, TheN, TheA>& a); + void MoveAssignment(MemcpyPolicy, InlinedVector&& other) { + inlined_vector_internal::DestroyAdapter<A>::DestroyElements( + storage_.GetAllocator(), data(), size()); + storage_.DeallocateIfAllocated(); + storage_.MemcpyFrom(other.storage_); + + other.storage_.SetInlinedSize(0); + } + + void MoveAssignment(ElementwiseAssignPolicy, InlinedVector&& other) { + if (other.storage_.GetIsAllocated()) { + MoveAssignment(MemcpyPolicy{}, std::move(other)); + } else { + storage_.Assign(IteratorValueAdapter<A, MoveIterator<A>>( + MoveIterator<A>(other.storage_.GetInlinedData())), + other.size()); + } + } + + void MoveAssignment(ElementwiseConstructPolicy, InlinedVector&& other) { + if (other.storage_.GetIsAllocated()) { + MoveAssignment(MemcpyPolicy{}, std::move(other)); + } else { + inlined_vector_internal::DestroyAdapter<A>::DestroyElements( + storage_.GetAllocator(), data(), size()); + storage_.DeallocateIfAllocated(); + + IteratorValueAdapter<A, MoveIterator<A>> other_values( + MoveIterator<A>(other.storage_.GetInlinedData())); + inlined_vector_internal::ConstructElements<A>( + storage_.GetAllocator(), storage_.GetInlinedData(), other_values, + other.storage_.GetSize()); + storage_.SetInlinedSize(other.storage_.GetSize()); + } + } + Storage storage_; }; diff --git a/absl/container/inlined_vector_test.cc b/absl/container/inlined_vector_test.cc index b872eb45..65ddbab6 100644 --- a/absl/container/inlined_vector_test.cc +++ b/absl/container/inlined_vector_test.cc @@ -16,12 +16,14 @@ #include <algorithm> #include <forward_list> +#include <iterator> #include <list> #include <memory> #include <scoped_allocator> #include <sstream> #include <stdexcept> #include <string> +#include <utility> #include <vector> #include "gmock/gmock.h" @@ -49,6 +51,7 @@ using testing::ElementsAre; using testing::ElementsAreArray; using testing::Eq; using testing::Gt; +using testing::Pointwise; using testing::PrintToString; using IntVec = absl::InlinedVector<int, 8>; @@ -1824,4 +1827,112 @@ TEST(InlinedVectorTest, AbslHashValueWorks) { EXPECT_TRUE(absl::VerifyTypeImplementsAbslHashCorrectly(cases)); } +class MoveConstructibleOnlyInstance + : public absl::test_internal::BaseCountedInstance { + public: + explicit MoveConstructibleOnlyInstance(int x) : BaseCountedInstance(x) {} + MoveConstructibleOnlyInstance(MoveConstructibleOnlyInstance&& other) = + default; + MoveConstructibleOnlyInstance& operator=( + MoveConstructibleOnlyInstance&& other) = delete; +}; + +MATCHER(HasValue, "") { + return ::testing::get<0>(arg).value() == ::testing::get<1>(arg); +} + +TEST(MoveAssignment, NonAssignable) { + using X = MoveConstructibleOnlyInstance; + { + InstanceTracker tracker; + absl::InlinedVector<X, 2> inlined; + inlined.emplace_back(1); + absl::InlinedVector<X, 2> allocated; + allocated.emplace_back(1); + allocated.emplace_back(2); + allocated.emplace_back(3); + tracker.ResetCopiesMovesSwaps(); + + inlined = std::move(allocated); + // passed ownership of the allocated storage + EXPECT_EQ(tracker.moves(), 0); + EXPECT_EQ(tracker.live_instances(), 3); + + EXPECT_THAT(inlined, Pointwise(HasValue(), {1, 2, 3})); + } + + { + InstanceTracker tracker; + absl::InlinedVector<X, 2> inlined; + inlined.emplace_back(1); + absl::InlinedVector<X, 2> allocated; + allocated.emplace_back(1); + allocated.emplace_back(2); + allocated.emplace_back(3); + tracker.ResetCopiesMovesSwaps(); + + allocated = std::move(inlined); + // Moved elements + EXPECT_EQ(tracker.moves(), 1); + EXPECT_EQ(tracker.live_instances(), 1); + + EXPECT_THAT(allocated, Pointwise(HasValue(), {1})); + } + + { + InstanceTracker tracker; + absl::InlinedVector<X, 2> inlined_a; + inlined_a.emplace_back(1); + absl::InlinedVector<X, 2> inlined_b; + inlined_b.emplace_back(1); + tracker.ResetCopiesMovesSwaps(); + + inlined_a = std::move(inlined_b); + // Moved elements + EXPECT_EQ(tracker.moves(), 1); + EXPECT_EQ(tracker.live_instances(), 1); + + EXPECT_THAT(inlined_a, Pointwise(HasValue(), {1})); + } + + { + InstanceTracker tracker; + absl::InlinedVector<X, 2> allocated_a; + allocated_a.emplace_back(1); + allocated_a.emplace_back(2); + allocated_a.emplace_back(3); + absl::InlinedVector<X, 2> allocated_b; + allocated_b.emplace_back(4); + allocated_b.emplace_back(5); + allocated_b.emplace_back(6); + allocated_b.emplace_back(7); + tracker.ResetCopiesMovesSwaps(); + + allocated_a = std::move(allocated_b); + // passed ownership of the allocated storage + EXPECT_EQ(tracker.moves(), 0); + EXPECT_EQ(tracker.live_instances(), 4); + + EXPECT_THAT(allocated_a, Pointwise(HasValue(), {4, 5, 6, 7})); + } + + { + InstanceTracker tracker; + absl::InlinedVector<X, 2> v; + v.emplace_back(1); + v.emplace_back(2); + v.emplace_back(3); + + tracker.ResetCopiesMovesSwaps(); + + // Obfuscated in order to pass -Wself-move. + v = std::move(*std::addressof(v)); + // nothing happens + EXPECT_EQ(tracker.moves(), 0); + EXPECT_EQ(tracker.live_instances(), 3); + + EXPECT_THAT(v, Pointwise(HasValue(), {1, 2, 3})); + } +} + } // anonymous namespace diff --git a/absl/container/internal/inlined_vector.h b/absl/container/internal/inlined_vector.h index a56b7573..f623494c 100644 --- a/absl/container/internal/inlined_vector.h +++ b/absl/container/internal/inlined_vector.h @@ -83,6 +83,9 @@ using IsMemcpyOk = absl::is_trivially_copy_assignable<ValueType<A>>, absl::is_trivially_destructible<ValueType<A>>>; +template <typename A> +using IsMoveAssignOk = std::is_move_assignable<ValueType<A>>; + template <typename T> struct TypeIdentity { using type = T; |