Delete TODOs to move functors when moving hashtables and add a test that fails when we do so.

PiperOrigin-RevId: 676894552
Change-Id: I68619a6ae48e65c7c58466b0be7ec79f5797066c

absl/container/BUILD.bazel

@@ -305,6 +305,7 @@ cc_test(
     deps = [
     deps = [
         ":container_memory",
         ":container_memory",
         ":flat_hash_set",
         ":flat_hash_set",
+        ":hash_container_defaults",
         ":hash_generator_testing",
         ":hash_generator_testing",
         ":test_allocator",
         ":test_allocator",
         ":unordered_set_constructor_test",
         ":unordered_set_constructor_test",
@@ -312,6 +313,7 @@ cc_test(
         ":unordered_set_members_test",
         ":unordered_set_members_test",
         ":unordered_set_modifiers_test",
         ":unordered_set_modifiers_test",
         "//absl/base:config",
         "//absl/base:config",
+        "//absl/hash",
         "//absl/log:check",
         "//absl/log:check",
         "//absl/memory",
         "//absl/memory",
         "//absl/strings",
         "//absl/strings",

absl/container/CMakeLists.txt

@@ -350,6 +350,8 @@ absl_cc_test(
     absl::config
     absl::config
     absl::container_memory
     absl::container_memory
     absl::flat_hash_set
     absl::flat_hash_set
+    absl::hash
+    absl::hash_container_defaults
     absl::hash_generator_testing
     absl::hash_generator_testing
     absl::memory
     absl::memory
     absl::strings
     absl::strings

absl/container/flat_hash_set_test.cc

@@ -14,6 +14,7 @@
 
 
 #include "absl/container/flat_hash_set.h"
 #include "absl/container/flat_hash_set.h"
 
 
+#include <cstddef>
 #include <cstdint>
 #include <cstdint>
 #include <memory>
 #include <memory>
 #include <type_traits>
 #include <type_traits>
@@ -23,6 +24,7 @@
 #include "gmock/gmock.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include "gtest/gtest.h"
 #include "absl/base/config.h"
 #include "absl/base/config.h"
+#include "absl/container/hash_container_defaults.h"
 #include "absl/container/internal/container_memory.h"
 #include "absl/container/internal/container_memory.h"
 #include "absl/container/internal/hash_generator_testing.h"
 #include "absl/container/internal/hash_generator_testing.h"
 #include "absl/container/internal/test_allocator.h"
 #include "absl/container/internal/test_allocator.h"
@@ -30,6 +32,7 @@
 #include "absl/container/internal/unordered_set_lookup_test.h"
 #include "absl/container/internal/unordered_set_lookup_test.h"
 #include "absl/container/internal/unordered_set_members_test.h"
 #include "absl/container/internal/unordered_set_members_test.h"
 #include "absl/container/internal/unordered_set_modifiers_test.h"
 #include "absl/container/internal/unordered_set_modifiers_test.h"
+#include "absl/hash/hash.h"
 #include "absl/log/check.h"
 #include "absl/log/check.h"
 #include "absl/memory/memory.h"
 #include "absl/memory/memory.h"
 #include "absl/strings/string_view.h"
 #include "absl/strings/string_view.h"
@@ -291,6 +294,49 @@ TEST(FlatHashSet, FlatHashSetPolicyDestroyReturnsTrue) {
                          std::allocator<int>>(nullptr, nullptr))()));
                          std::allocator<int>>(nullptr, nullptr))()));
 }
 }
 
 
+struct HashEqInvalidOnMove {
+  HashEqInvalidOnMove() = default;
+  HashEqInvalidOnMove(const HashEqInvalidOnMove& rhs) = default;
+  HashEqInvalidOnMove(HashEqInvalidOnMove&& rhs) { rhs.moved = true; }
+  HashEqInvalidOnMove& operator=(const HashEqInvalidOnMove& rhs) = default;
+  HashEqInvalidOnMove& operator=(HashEqInvalidOnMove&& rhs) {
+    rhs.moved = true;
+    return *this;
+  }
+
+  size_t operator()(int x) const {
+    CHECK(!moved);
+    return absl::HashOf(x);
+  }
+
+  bool operator()(int x, int y) const {
+    CHECK(!moved);
+    return x == y;
+  }
+
+  bool moved = false;
+};
+
+TEST(FlatHashSet, MovedFromCleared_HashMustBeValid) {
+  flat_hash_set<int, HashEqInvalidOnMove> s1, s2;
+  // Moving the hashtable must not move the hasher because we need to support
+  // this behavior.
+  s2 = std::move(s1);
+  s1.clear();
+  s1.insert(2);
+  EXPECT_THAT(s1, UnorderedElementsAre(2));
+}
+
+TEST(FlatHashSet, MovedFromCleared_EqMustBeValid) {
+  flat_hash_set<int, DefaultHashContainerHash<int>, HashEqInvalidOnMove> s1, s2;
+  // Moving the hashtable must not move the equality functor because we need to
+  // support this behavior.
+  s2 = std::move(s1);
+  s1.clear();
+  s1.insert(2);
+  EXPECT_THAT(s1, UnorderedElementsAre(2));
+}
+
 }  // namespace
 }  // namespace
 }  // namespace container_internal
 }  // namespace container_internal
 ABSL_NAMESPACE_END
 ABSL_NAMESPACE_END

absl/container/internal/raw_hash_set.h

@@ -2841,7 +2841,6 @@ class raw_hash_set {
       :  // Hash, equality and allocator are copied instead of moved because
       :  // Hash, equality and allocator are copied instead of moved because
          // `that` must be left valid. If Hash is std::function<Key>, moving it
          // `that` must be left valid. If Hash is std::function<Key>, moving it
          // would create a nullptr functor that cannot be called.
          // would create a nullptr functor that cannot be called.
-         // TODO(b/296061262): move instead of copying hash/eq/alloc.
          // Note: we avoid using exchange for better generated code.
          // Note: we avoid using exchange for better generated code.
         settings_(PolicyTraits::transfer_uses_memcpy() || !that.is_full_soo()
         settings_(PolicyTraits::transfer_uses_memcpy() || !that.is_full_soo()
                       ? std::move(that.common())
                       ? std::move(that.common())
@@ -3831,7 +3830,6 @@ class raw_hash_set {
     destructor_impl();
     destructor_impl();
     move_common(that.is_full_soo(), that.alloc_ref(), common(),
     move_common(that.is_full_soo(), that.alloc_ref(), common(),
                 std::move(that.common()));
                 std::move(that.common()));
-    // TODO(b/296061262): move instead of copying hash/eq/alloc.
     hash_ref() = that.hash_ref();
     hash_ref() = that.hash_ref();
     eq_ref() = that.eq_ref();
     eq_ref() = that.eq_ref();
     CopyAlloc(alloc_ref(), that.alloc_ref(),
     CopyAlloc(alloc_ref(), that.alloc_ref(),
@@ -3870,7 +3868,6 @@ class raw_hash_set {
     // We can't take over that's memory so we need to move each element.
     // We can't take over that's memory so we need to move each element.
     // While moving elements, this should have that's hash/eq so copy hash/eq
     // While moving elements, this should have that's hash/eq so copy hash/eq
     // before moving elements.
     // before moving elements.
-    // TODO(b/296061262): move instead of copying hash/eq.
     hash_ref() = that.hash_ref();
     hash_ref() = that.hash_ref();
     eq_ref() = that.eq_ref();
     eq_ref() = that.eq_ref();
     return move_elements_allocs_unequal(std::move(that));
     return move_elements_allocs_unequal(std::move(that));