From 940e0ec36afbc7acf5de31167d4820d274d28c92 Mon Sep 17 00:00:00 2001 From: Arthur O'Dwyer Date: Thu, 12 Dec 2024 11:51:58 -0800 Subject: [PATCH] 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 2c52d7577ad00f9bde9fee433d43081f0e55cbef PiperOrigin-RevId: 705581176 Change-Id: Ic728747c41401863f8c01083e22b1d1e536347db --- absl/container/inlined_vector_test.cc | 73 +++++++++++++++--------- absl/container/internal/inlined_vector.h | 6 +- 2 files changed, 50 insertions(+), 29 deletions(-) diff --git a/absl/container/inlined_vector_test.cc b/absl/container/inlined_vector_test.cc index 21504cb0..54cd3105 100644 --- a/absl/container/inlined_vector_test.cc +++ b/absl/container/inlined_vector_test.cc @@ -1749,61 +1749,80 @@ TEST(AllocatorSupportTest, CountAllocations) { using MyAlloc = CountingAllocator; using AllocVec = absl::InlinedVector; 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(v.size() * sizeof(int)))); + EXPECT_THAT(bytes_allocated, + Eq(static_cast(v.size() * sizeof(int)))); + EXPECT_THAT(instance_count, Eq(static_cast(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(v.size() * sizeof(int)))); + EXPECT_THAT(bytes_allocated, + Eq(static_cast(v.size() * sizeof(int)))); + EXPECT_THAT(instance_count, Eq(static_cast(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(v2.size() * sizeof(int)))); + EXPECT_THAT(bytes_allocated2, + Eq(static_cast(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(v3.size() * sizeof(int)))); + EXPECT_THAT(bytes_allocated3, + Eq(static_cast(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(8 * sizeof(int))); + EXPECT_EQ(bytes_allocated, static_cast(8 * sizeof(int))); + EXPECT_EQ(instance_count, 8); v.resize(5); - EXPECT_EQ(allocated, static_cast(8 * sizeof(int))); + EXPECT_EQ(bytes_allocated, static_cast(8 * sizeof(int))); + EXPECT_EQ(instance_count, 5); v.shrink_to_fit(); - EXPECT_EQ(allocated, static_cast(5 * sizeof(int))); + EXPECT_EQ(bytes_allocated, static_cast(5 * sizeof(int))); + EXPECT_EQ(instance_count, 5); v.resize(4); - EXPECT_EQ(allocated, static_cast(5 * sizeof(int))); + EXPECT_EQ(bytes_allocated, static_cast(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) { diff --git a/absl/container/internal/inlined_vector.h b/absl/container/internal/inlined_vector.h index 2f24e461..0bd0a1c4 100644 --- a/absl/container/internal/inlined_vector.h +++ b/absl/container/internal/inlined_vector.h @@ -83,8 +83,10 @@ using IsMoveAssignOk = std::is_move_assignable>; template using IsSwapOk = absl::type_traits_internal::IsSwappable>; -template >::value> +template >::value && + std::is_same>>::value> struct DestroyAdapter; template