diff --git a/absl/container/inlined_vector_test.cc b/absl/container/inlined_vector_test.cc index 5ecf88a978c..6954262ea92 100644 --- a/absl/container/inlined_vector_test.cc +++ b/absl/container/inlined_vector_test.cc @@ -333,6 +333,57 @@ TEST(UniquePtr, Swap) { } } +// Erasing from a container of unique pointers should work fine, with no +// leaks, despite the fact that unique pointers are trivially relocatable but +// not trivially destructible. +// TODO(absl-team): Using unique_ptr here is technically correct, but +// a trivially relocatable struct would be less semantically confusing. +TEST(UniquePtr, EraseSingle) { + for (size_t size = 4; size < 16; ++size) { + absl::InlinedVector, 8> a; + for (size_t i = 0; i < size; ++i) { + a.push_back(std::make_unique(i)); + } + a.erase(a.begin()); + ASSERT_THAT(a, SizeIs(size - 1)); + for (size_t i = 0; i < size - 1; ++i) { + ASSERT_THAT(a[i], Pointee(i + 1)); + } + a.erase(a.begin() + 2); + ASSERT_THAT(a, SizeIs(size - 2)); + ASSERT_THAT(a[0], Pointee(1)); + ASSERT_THAT(a[1], Pointee(2)); + for (size_t i = 2; i < size - 2; ++i) { + ASSERT_THAT(a[i], Pointee(i + 2)); + } + } +} + +// Erasing from a container of unique pointers should work fine, with no +// leaks, despite the fact that unique pointers are trivially relocatable but +// not trivially destructible. +// TODO(absl-team): Using unique_ptr here is technically correct, but +// a trivially relocatable struct would be less semantically confusing. +TEST(UniquePtr, EraseMulti) { + for (size_t size = 5; size < 16; ++size) { + absl::InlinedVector, 8> a; + for (size_t i = 0; i < size; ++i) { + a.push_back(std::make_unique(i)); + } + a.erase(a.begin(), a.begin() + 2); + ASSERT_THAT(a, SizeIs(size - 2)); + for (size_t i = 0; i < size - 2; ++i) { + ASSERT_THAT(a[i], Pointee(i + 2)); + } + a.erase(a.begin() + 1, a.begin() + 3); + ASSERT_THAT(a, SizeIs(size - 4)); + ASSERT_THAT(a[0], Pointee(2)); + for (size_t i = 1; i < size - 4; ++i) { + ASSERT_THAT(a[i], Pointee(i + 4)); + } + } +} + // At the end of this test loop, the elements between [erase_begin, erase_end) // should have reference counts == 0, and all others elements should have // reference counts == 1. diff --git a/absl/container/internal/inlined_vector.h b/absl/container/internal/inlined_vector.h index 90a74dc7ffa..13c379ab93f 100644 --- a/absl/container/internal/inlined_vector.h +++ b/absl/container/internal/inlined_vector.h @@ -893,16 +893,30 @@ auto Storage::Erase(ConstIterator from, std::distance(ConstIterator(storage_view.data), from)); SizeType erase_end_index = erase_index + erase_size; - IteratorValueAdapter> move_values( - MoveIterator(storage_view.data + erase_end_index)); - - AssignElements(storage_view.data + erase_index, move_values, - storage_view.size - erase_end_index); + // Fast path: if the value type is trivially relocatable and we know + // the allocator doesn't do anything fancy, then we know it is legal for us to + // simply destroy the elements in the "erasure window" (which cannot throw) + // and then memcpy downward to close the window. + if (absl::is_trivially_relocatable>::value && + std::is_nothrow_destructible>::value && + std::is_same>>::value) { + DestroyAdapter::DestroyElements( + GetAllocator(), storage_view.data + erase_index, + erase_size); + std::memmove(reinterpret_cast(storage_view.data + erase_index), + reinterpret_cast(storage_view.data + erase_end_index), + (storage_view.size - erase_end_index) * sizeof(ValueType)); + } else { + IteratorValueAdapter> move_values( + MoveIterator(storage_view.data + erase_end_index)); - DestroyAdapter::DestroyElements( - GetAllocator(), storage_view.data + (storage_view.size - erase_size), - erase_size); + AssignElements(storage_view.data + erase_index, move_values, + storage_view.size - erase_end_index); + DestroyAdapter::DestroyElements( + GetAllocator(), storage_view.data + (storage_view.size - erase_size), + erase_size); + } SubtractSize(erase_size); return Iterator(storage_view.data + erase_index); }