diff options
author | Evan Brown <ezb@google.com> | 2023-10-03 10:01:17 -0700 |
---|---|---|
committer | Copybara-Service <copybara-worker@google.com> | 2023-10-03 10:02:13 -0700 |
commit | 22dc7911f9a4721f6dec0cc41a36fd204aa0e4f8 (patch) | |
tree | ab4a1e578b6e509612e47b208595bf8a4f790c5f /absl/container/internal/raw_hash_set_test.cc | |
parent | 74a8f6faf075cd796885a77c5cbb9ef56f65f747 (diff) | |
download | abseil-22dc7911f9a4721f6dec0cc41a36fd204aa0e4f8.tar.gz abseil-22dc7911f9a4721f6dec0cc41a36fd204aa0e4f8.tar.bz2 abseil-22dc7911f9a4721f6dec0cc41a36fd204aa0e4f8.zip |
Refactor swisstable copy/move assignment to fix issues with allocator propagation and improve performance.
Correctness:
- We use swap to implement copy assignment and move assignment, which means that allocator propagation in copy/move assignment depends on `propagate_on_container_swap` in addition to `propagate_on_container_copy_assignment`/`propagate_on_container_move_assignment`.
- In swap, if `propagate_on_container_swap` is `false` and `get_allocator() != other.get_allocator()`, the behavior is undefined (https://en.cppreference.com/w/cpp/container/unordered_set/swap) - we should assert that this UB case isn't happening. For this reason, we also delete the NoPropagateOn_Swap test case in raw_hash_set_allocator_test.
Performance:
- Don't rely on swap so we don't have to do unnecessary copying into the moved-from sets.
- Don't use temp sets in move assignment.
- Default the move constructor of CommonFields.
- Avoid using exchange in raw_hash_set move constructor.
- In `raw_hash_set(raw_hash_set&& that, const allocator_type& a)` with unequal allocators and in move assignment with non-propagating unequal allocators, move set keys instead of copying them.
PiperOrigin-RevId: 570419290
Change-Id: I499e54f17d9cb0b0836601f5c06187d1f269a5b8
Diffstat (limited to 'absl/container/internal/raw_hash_set_test.cc')
-rw-r--r-- | absl/container/internal/raw_hash_set_test.cc | 32 |
1 files changed, 22 insertions, 10 deletions
diff --git a/absl/container/internal/raw_hash_set_test.cc b/absl/container/internal/raw_hash_set_test.cc index 4ee61220..61eb6d95 100644 --- a/absl/container/internal/raw_hash_set_test.cc +++ b/absl/container/internal/raw_hash_set_test.cc @@ -410,14 +410,14 @@ struct StringTable }; struct IntTable - : raw_hash_set<IntPolicy, container_internal::hash_default_hash<int64_t>, + : raw_hash_set<IntPolicy, hash_default_hash<int64_t>, std::equal_to<int64_t>, std::allocator<int64_t>> { using Base = typename IntTable::raw_hash_set; using Base::Base; }; struct Uint8Table - : raw_hash_set<Uint8Policy, container_internal::hash_default_hash<uint8_t>, + : raw_hash_set<Uint8Policy, hash_default_hash<uint8_t>, std::equal_to<uint8_t>, std::allocator<uint8_t>> { using Base = typename Uint8Table::raw_hash_set; using Base::Base; @@ -436,14 +436,14 @@ struct CustomAlloc : std::allocator<T> { }; struct CustomAllocIntTable - : raw_hash_set<IntPolicy, container_internal::hash_default_hash<int64_t>, + : raw_hash_set<IntPolicy, hash_default_hash<int64_t>, std::equal_to<int64_t>, CustomAlloc<int64_t>> { using Base = typename CustomAllocIntTable::raw_hash_set; using Base::Base; }; struct MinimumAlignmentUint8Table - : raw_hash_set<Uint8Policy, container_internal::hash_default_hash<uint8_t>, + : raw_hash_set<Uint8Policy, hash_default_hash<uint8_t>, std::equal_to<uint8_t>, MinimumAlignmentAlloc<uint8_t>> { using Base = typename MinimumAlignmentUint8Table::raw_hash_set; using Base::Base; @@ -1602,7 +1602,7 @@ TEST(Table, CopyConstructWithAlloc) { } struct ExplicitAllocIntTable - : raw_hash_set<IntPolicy, container_internal::hash_default_hash<int64_t>, + : raw_hash_set<IntPolicy, hash_default_hash<int64_t>, std::equal_to<int64_t>, Alloc<int64_t>> { ExplicitAllocIntTable() = default; }; @@ -1680,6 +1680,14 @@ TEST(Table, MoveAssign) { EXPECT_THAT(*u.find("a"), Pair("a", "b")); } +TEST(Table, MoveSelfAssign) { + StringTable t; + t.emplace("a", "b"); + EXPECT_EQ(1, t.size()); + t = std::move(*&t); + // As long as we don't crash, it's fine. +} + TEST(Table, Equality) { StringTable t; std::vector<std::pair<std::string, std::string>> v = {{"a", "b"}, @@ -1858,11 +1866,9 @@ TEST(Table, HeterogeneousLookupOverloads) { EXPECT_FALSE((VerifyResultOf<CallPrefetch, NonTransparentTable>())); EXPECT_FALSE((VerifyResultOf<CallCount, NonTransparentTable>())); - using TransparentTable = raw_hash_set< - StringPolicy, - absl::container_internal::hash_default_hash<absl::string_view>, - absl::container_internal::hash_default_eq<absl::string_view>, - std::allocator<int>>; + using TransparentTable = + raw_hash_set<StringPolicy, hash_default_hash<absl::string_view>, + hash_default_eq<absl::string_view>, std::allocator<int>>; EXPECT_TRUE((VerifyResultOf<CallFind, TransparentTable>())); EXPECT_TRUE((VerifyResultOf<CallErase, TransparentTable>())); @@ -2464,6 +2470,12 @@ TEST(Iterator, InvalidComparisonDifferentTables) { "Invalid iterator comparison.*non-end"); } +template <typename Alloc> +using RawHashSetAlloc = raw_hash_set<IntPolicy, hash_default_hash<int64_t>, + std::equal_to<int64_t>, Alloc>; + +TEST(Table, AllocatorPropagation) { TestAllocPropagation<RawHashSetAlloc>(); } + } // namespace } // namespace container_internal ABSL_NAMESPACE_END |