PR #1790: Respect the allocator's .destroy method in ~InlinedVector

Imported from GitHub PR https://github.com/abseil/abseil-cpp/pull/1790

InlinedVector goes out of its way to use `A::construct` to construct new elements, so it should not skip the `A::destroy` call for those elements, either. Most codepaths seem fine (I didn't exhaustively explore this), but `DestroyAdapter` specifically failed to check whether the allocator had a non-trivial `destroy` method: it checked only whether the element type was trivially destructible in the absence of any `destroy` method.

Use the same condition in `DestroyAdapter` as we already use in the special short-circuit case on line 357 and in the conditions for the relocation optimizations on lines 304 and 324.

Unit test updated by @derekmauro

Merging this change closes #1790

COPYBARA_INTEGRATE_REVIEW=https://github.com/abseil/abseil-cpp/pull/1790 from Quuxplusone:inlined-vector-destroy 2c52d7577a
PiperOrigin-RevId: 705581176
Change-Id: Ic728747c41401863f8c01083e22b1d1e536347db
This commit is contained in:
Arthur O'Dwyer
2024-12-12 11:51:58 -08:00
committed by Copybara-Service
parent c935131138
commit 940e0ec36a
2 changed files with 50 additions and 29 deletions

View File

@@ -1749,61 +1749,80 @@ TEST(AllocatorSupportTest, CountAllocations) {
using MyAlloc = CountingAllocator<int>;
using AllocVec = absl::InlinedVector<int, 4, MyAlloc>;
const int ia[] = {0, 1, 2, 3, 4, 5, 6, 7};
int64_t allocated = 0;
MyAlloc alloc(&allocated);
int64_t bytes_allocated = 0;
int64_t instance_count = 0;
MyAlloc alloc(&bytes_allocated, &instance_count);
{
AllocVec ABSL_ATTRIBUTE_UNUSED v(ia, ia + 4, alloc);
EXPECT_THAT(allocated, Eq(0));
EXPECT_THAT(bytes_allocated, Eq(0));
EXPECT_THAT(instance_count, Eq(4));
}
EXPECT_THAT(allocated, Eq(0));
EXPECT_THAT(bytes_allocated, Eq(0));
EXPECT_THAT(instance_count, Eq(0));
{
AllocVec ABSL_ATTRIBUTE_UNUSED v(ia, ia + ABSL_ARRAYSIZE(ia), alloc);
EXPECT_THAT(allocated, Eq(static_cast<int64_t>(v.size() * sizeof(int))));
EXPECT_THAT(bytes_allocated,
Eq(static_cast<int64_t>(v.size() * sizeof(int))));
EXPECT_THAT(instance_count, Eq(static_cast<int64_t>(v.size())));
}
EXPECT_THAT(allocated, Eq(0));
EXPECT_THAT(bytes_allocated, Eq(0));
EXPECT_THAT(instance_count, Eq(0));
{
AllocVec v(4, 1, alloc);
EXPECT_THAT(allocated, Eq(0));
EXPECT_THAT(bytes_allocated, Eq(0));
EXPECT_THAT(instance_count, Eq(4));
int64_t allocated2 = 0;
MyAlloc alloc2(&allocated2);
int64_t bytes_allocated2 = 0;
MyAlloc alloc2(&bytes_allocated2);
ABSL_ATTRIBUTE_UNUSED AllocVec v2(v, alloc2);
EXPECT_THAT(allocated2, Eq(0));
EXPECT_THAT(bytes_allocated2, Eq(0));
int64_t allocated3 = 0;
MyAlloc alloc3(&allocated3);
int64_t bytes_allocated3 = 0;
MyAlloc alloc3(&bytes_allocated3);
ABSL_ATTRIBUTE_UNUSED AllocVec v3(std::move(v), alloc3);
EXPECT_THAT(allocated3, Eq(0));
EXPECT_THAT(bytes_allocated3, Eq(0));
}
EXPECT_THAT(allocated, 0);
EXPECT_THAT(bytes_allocated, Eq(0));
EXPECT_THAT(instance_count, Eq(0));
{
AllocVec v(8, 2, alloc);
EXPECT_THAT(allocated, Eq(static_cast<int64_t>(v.size() * sizeof(int))));
EXPECT_THAT(bytes_allocated,
Eq(static_cast<int64_t>(v.size() * sizeof(int))));
EXPECT_THAT(instance_count, Eq(static_cast<int64_t>(v.size())));
int64_t allocated2 = 0;
MyAlloc alloc2(&allocated2);
int64_t bytes_allocated2 = 0;
MyAlloc alloc2(&bytes_allocated2);
AllocVec v2(v, alloc2);
EXPECT_THAT(allocated2, Eq(static_cast<int64_t>(v2.size() * sizeof(int))));
EXPECT_THAT(bytes_allocated2,
Eq(static_cast<int64_t>(v2.size() * sizeof(int))));
int64_t allocated3 = 0;
MyAlloc alloc3(&allocated3);
int64_t bytes_allocated3 = 0;
MyAlloc alloc3(&bytes_allocated3);
AllocVec v3(std::move(v), alloc3);
EXPECT_THAT(allocated3, Eq(static_cast<int64_t>(v3.size() * sizeof(int))));
EXPECT_THAT(bytes_allocated3,
Eq(static_cast<int64_t>(v3.size() * sizeof(int))));
}
EXPECT_EQ(allocated, 0);
EXPECT_EQ(bytes_allocated, 0);
EXPECT_EQ(instance_count, 0);
{
// Test shrink_to_fit deallocations.
AllocVec v(8, 2, alloc);
EXPECT_EQ(allocated, static_cast<int64_t>(8 * sizeof(int)));
EXPECT_EQ(bytes_allocated, static_cast<int64_t>(8 * sizeof(int)));
EXPECT_EQ(instance_count, 8);
v.resize(5);
EXPECT_EQ(allocated, static_cast<int64_t>(8 * sizeof(int)));
EXPECT_EQ(bytes_allocated, static_cast<int64_t>(8 * sizeof(int)));
EXPECT_EQ(instance_count, 5);
v.shrink_to_fit();
EXPECT_EQ(allocated, static_cast<int64_t>(5 * sizeof(int)));
EXPECT_EQ(bytes_allocated, static_cast<int64_t>(5 * sizeof(int)));
EXPECT_EQ(instance_count, 5);
v.resize(4);
EXPECT_EQ(allocated, static_cast<int64_t>(5 * sizeof(int)));
EXPECT_EQ(bytes_allocated, static_cast<int64_t>(5 * sizeof(int)));
EXPECT_EQ(instance_count, 4);
v.shrink_to_fit();
EXPECT_EQ(allocated, 0);
EXPECT_EQ(bytes_allocated, 0);
EXPECT_EQ(instance_count, 4);
}
EXPECT_EQ(instance_count, 0);
}
TEST(AllocatorSupportTest, SwapBothAllocated) {

View File

@@ -83,8 +83,10 @@ using IsMoveAssignOk = std::is_move_assignable<ValueType<A>>;
template <typename A>
using IsSwapOk = absl::type_traits_internal::IsSwappable<ValueType<A>>;
template <typename A, bool IsTriviallyDestructible =
absl::is_trivially_destructible<ValueType<A>>::value>
template <typename A,
bool IsTriviallyDestructible =
absl::is_trivially_destructible<ValueType<A>>::value &&
std::is_same<A, std::allocator<ValueType<A>>>::value>
struct DestroyAdapter;
template <typename A>