Fix self-move handling in absl::linked_hash_{set|map}
PiperOrigin-RevId: 853340964
Change-Id: I818695f01d53b5515be24122e1988e69ad6421d4
diff --git a/absl/container/linked_hash_map.h b/absl/container/linked_hash_map.h
index 8348011..e42a1f7 100644
--- a/absl/container/linked_hash_map.h
+++ b/absl/container/linked_hash_map.h
@@ -248,22 +248,25 @@
}
linked_hash_map& operator=(const linked_hash_map& other) {
- if (this == &other) return *this;
- // Make a new set, with other's hash/eq/alloc.
- set_ = SetType(other.bucket_count(), other.set_.hash_function(),
- other.set_.key_eq(), other.get_allocator());
- // Copy the list, with other's allocator.
- list_ = ListType(other.get_allocator());
- CopyFrom(other);
+ if (this != &other) {
+ // Make a new set, with other's hash/eq/alloc.
+ set_ = SetType(other.bucket_count(), other.set_.hash_function(),
+ other.set_.key_eq(), other.get_allocator());
+ // Copy the list, with other's allocator.
+ list_ = ListType(other.get_allocator());
+ CopyFrom(other);
+ }
return *this;
}
linked_hash_map& operator=(linked_hash_map&& other) noexcept {
- // underlying containers will handle progagate_on_container_move details
- set_ = std::move(other.set_);
- list_ = std::move(other.list_);
- other.set_.clear();
- other.list_.clear();
+ if (this != &other) {
+ // underlying containers will handle progagate_on_container_move details
+ set_ = std::move(other.set_);
+ list_ = std::move(other.list_);
+ other.set_.clear();
+ other.list_.clear();
+ }
return *this;
}
diff --git a/absl/container/linked_hash_map_test.cc b/absl/container/linked_hash_map_test.cc
index 54d42fe..9f530d7 100644
--- a/absl/container/linked_hash_map_test.cc
+++ b/absl/container/linked_hash_map_test.cc
@@ -98,6 +98,14 @@
FAIL() << "Assigned map's find method returned an invalid iterator.";
}
+// Tests that self-assignment works.
+TEST(LinkedHashMapTest, SelfAssign) {
+ linked_hash_map<int, int> a{{1, 1}, {2, 2}, {3, 3}};
+ auto& a_ref = a;
+ a = a_ref;
+ EXPECT_THAT(a, ElementsAre(Pair(1, 1), Pair(2, 2), Pair(3, 3)));
+}
+
// Tests that move constructor works.
TEST(LinkedHashMapTest, Move) {
// Use unique_ptr as an example of a non-copyable type.
@@ -108,6 +116,14 @@
EXPECT_THAT(n, ElementsAre(Pair(2, Pointee(12)), Pair(3, Pointee(13))));
}
+// Tests that self-moving works.
+TEST(LinkedHashMapTest, SelfMove) {
+ linked_hash_map<int, int> a{{1, 1}, {2, 2}, {3, 3}};
+ auto& a_ref = a;
+ a = std::move(a_ref);
+ EXPECT_THAT(a, ElementsAre(Pair(1, 1), Pair(2, 2), Pair(3, 3)));
+}
+
TEST(LinkedHashMapTest, CanInsertMoveOnly) {
linked_hash_map<int, std::unique_ptr<int>> m;
struct Data {
@@ -512,6 +528,13 @@
ASSERT_EQ(2, m2.size());
}
+TEST(LinkedHashMapTest, SelfSwap) {
+ linked_hash_map<int, int> a{{1, 1}, {2, 2}, {3, 3}};
+ using std::swap;
+ swap(a, a);
+ EXPECT_THAT(a, ElementsAre(Pair(1, 1), Pair(2, 2), Pair(3, 3)));
+}
+
TEST(LinkedHashMapTest, InitializerList) {
linked_hash_map<int, int> m{{1, 2}, {3, 4}};
ASSERT_EQ(2, m.size());
diff --git a/absl/container/linked_hash_set.h b/absl/container/linked_hash_set.h
index f874cd1..1b5d578 100644
--- a/absl/container/linked_hash_set.h
+++ b/absl/container/linked_hash_set.h
@@ -238,21 +238,24 @@
}
linked_hash_set& operator=(const linked_hash_set& other) {
- if (this == &other) return *this;
- // Make a new set, with other's hash/eq/alloc.
- set_ = SetType(other.bucket_count(), other.set_.hash_function(),
- other.set_.key_eq(), other.get_allocator());
- // Copy the list, with other's allocator.
- list_ = ListType(other.get_allocator());
- CopyFrom(other);
+ if (this != &other) {
+ // Make a new set, with other's hash/eq/alloc.
+ set_ = SetType(other.bucket_count(), other.set_.hash_function(),
+ other.set_.key_eq(), other.get_allocator());
+ // Copy the list, with other's allocator.
+ list_ = ListType(other.get_allocator());
+ CopyFrom(other);
+ }
return *this;
}
linked_hash_set& operator=(linked_hash_set&& other) noexcept {
- set_ = std::move(other.set_);
- list_ = std::move(other.list_);
- other.set_.clear();
- other.list_.clear();
+ if (this != &other) {
+ set_ = std::move(other.set_);
+ list_ = std::move(other.list_);
+ other.set_.clear();
+ other.list_.clear();
+ }
return *this;
}
diff --git a/absl/container/linked_hash_set_test.cc b/absl/container/linked_hash_set_test.cc
index b641b97..9a3af62 100644
--- a/absl/container/linked_hash_set_test.cc
+++ b/absl/container/linked_hash_set_test.cc
@@ -91,6 +91,21 @@
FAIL() << "Assigned set's find method returned an invalid iterator.";
}
+// Tests that self-assignment works.
+TEST(LinkedHashSetTest, SelfAssign) {
+ linked_hash_set<int> a{1, 2, 3};
+ auto& a_ref = a;
+ a = a_ref;
+
+ EXPECT_TRUE(a.contains(2));
+ auto found = a.find(2);
+ ASSERT_TRUE(found != a.end());
+ for (auto iter = a.begin(); iter != a.end(); ++iter) {
+ if (iter == found) return;
+ }
+ FAIL() << "Assigned set's find method returned an invalid iterator.";
+}
+
// Tests that move constructor works.
TEST(LinkedHashSetTest, Move) {
// Use unique_ptr as an example of a non-copyable type.
@@ -101,6 +116,14 @@
EXPECT_THAT(n, ElementsAre(Pointee(2), Pointee(3)));
}
+// Tests that self-moving works.
+TEST(LinkedHashSetTest, SelfMove) {
+ linked_hash_set<int> a{1, 2, 3};
+ auto& a_ref = a;
+ a = std::move(a_ref);
+ EXPECT_THAT(a, ElementsAre(1, 2, 3));
+}
+
struct IntUniquePtrHash {
size_t operator()(const std::unique_ptr<int>& p) const {
return static_cast<size_t>(*p);
@@ -521,6 +544,13 @@
ASSERT_EQ(2, m2.size());
}
+TEST(LinkedHashSetTest, SelfSwap) {
+ linked_hash_set<int> a{1, 2, 3};
+ using std::swap;
+ swap(a, a);
+ EXPECT_THAT(a, ElementsAre(1, 2, 3));
+}
+
TEST(LinkedHashSetTest, InitializerList) {
linked_hash_set<int> m{1, 3};
ASSERT_EQ(2, m.size());
