From 01ea8093e6d0293ea252e8027c17d7dff26a9c9f Mon Sep 17 00:00:00 2001
From: greg7mdp <greg7mdp@gmail.com>
Date: Sat, 25 Jun 2022 14:55:26 -0400
Subject: [PATCH] use ADL lookup for erase_if instead of adding to std (gtl
 reported issue)

---
 parallel_hashmap/phmap.h | 20 +++++++++++++-------
 tests/erase_if_test.cc   | 12 ++++++------
 2 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/parallel_hashmap/phmap.h b/parallel_hashmap/phmap.h
index 8f79cd6..4e357c7 100644
--- a/parallel_hashmap/phmap.h
+++ b/parallel_hashmap/phmap.h
@@ -3335,6 +3335,7 @@ public:
     //   flat_hash_set<std::string> s;
     //   // Uses "abc" directly without copying it into std::string.
     //   s.erase("abc");
+    //
     // --------------------------------------------------------------------
     template <class K = key_type>
     size_type erase(const key_arg<K>& key) {
@@ -3366,15 +3367,22 @@ public:
     //     ++it;
     //   }
     // }
+    //
+    // Do not use erase APIs taking iterators when accessing the map concurrently
     // --------------------------------------------------------------------
-    void _erase(iterator it) {
-        assert(it.inner_ != nullptr);
-        it.inner_->set_._erase(it.it_);
+    void _erase(iterator it, bool do_lock = true) {
+        Inner* inner = it.inner_;
+        assert(inner != nullptr);
+        auto&  set   = inner->set_;
+        // typename Lockable::UniqueLock m(*inner); // don't lock here 
+        
+        set._erase(it.it_);
     }
     void _erase(const_iterator cit) { _erase(cit.iter_); }
 
     // This overload is necessary because otherwise erase<K>(const K&) would be
     // a better match if non-const iterator is passed as an argument.
+    // Do not use erase APIs taking iterators when accessing the map concurrently
     // --------------------------------------------------------------------
     iterator erase(iterator it) { _erase(it++); return it; }
 
@@ -3387,6 +3395,7 @@ public:
 
     // Moves elements from `src` into `this`.
     // If the element already exists in `this`, it is left unmodified in `src`.
+    // Do not use erase APIs taking iterators when accessing the map concurrently
     // --------------------------------------------------------------------
     template <typename E = Eq>
     void merge(parallel_hash_set<N, RefSet, Mtx_, Policy, Hash, E, Alloc>& src) {  // NOLINT
@@ -4992,9 +5001,6 @@ namespace phmap {
             return old_size - c.size();
         }
     } // priv
-} // phmap
-
-namespace std {
 
     // ======== erase_if for phmap set containers ==================================
     template <class T, class Hash, class Eq, class Alloc, class Pred> 
@@ -5038,7 +5044,7 @@ namespace std {
         return phmap::priv::erase_if(c, std::move(pred));
     }
 
-} // std
+} // phmap
 
 #ifdef _MSC_VER
      #pragma warning(pop)  
diff --git a/tests/erase_if_test.cc b/tests/erase_if_test.cc
index 475276d..77fd4d9 100644
--- a/tests/erase_if_test.cc
+++ b/tests/erase_if_test.cc
@@ -10,11 +10,11 @@ namespace {
 
 TEST(EraseIf, FlatHashSet_uint32) {
     phmap::flat_hash_set<uint32_t> st1 = { 3, 6, 7, 9 };
-    auto num_erased = std::erase_if(st1, [](const uint32_t& v) { return v >= 7; });
+    auto num_erased = erase_if(st1, [](const uint32_t& v) { return v >= 7; });
     EXPECT_TRUE(num_erased == 2);
 
     phmap::flat_hash_set<uint32_t> st2 = { 0, 2, 3, 6 };
-    num_erased = std::erase_if(st2, [](const uint32_t& v) { return v <= 2; });
+    num_erased = erase_if(st2, [](const uint32_t& v) { return v <= 2; });
     EXPECT_TRUE(num_erased == 2);
 
     EXPECT_TRUE(st1 == st2);
@@ -23,11 +23,11 @@ TEST(EraseIf, FlatHashSet_uint32) {
 TEST(EraseIf, FlatHashMap_uint64_uint32) {
     using map = phmap::flat_hash_map<uint32_t, uint32_t>;
     map st1 = { {3, 0}, {6, 0}, {7, 0}, {9, 0} };
-    auto num_erased = std::erase_if(st1, [](const map::value_type& v) { return v.first >= 7; });
+    auto num_erased = erase_if(st1, [](const map::value_type& v) { return v.first >= 7; });
     EXPECT_TRUE(num_erased == 2);
 
     map st2 = { {0, 0}, {2, 0}, {3, 0}, {6, 0} };
-    num_erased = std::erase_if(st2, [](const map::value_type& v) { return v.first <= 2; });
+    num_erased = erase_if(st2, [](const map::value_type& v) { return v.first <= 2; });
     EXPECT_TRUE(num_erased == 2);
 
     EXPECT_TRUE(st1 == st2);
@@ -36,11 +36,11 @@ TEST(EraseIf, FlatHashMap_uint64_uint32) {
 TEST(EraseIf, ParallelFlatHashMap_uint64_uint32) {
     using map = phmap::parallel_flat_hash_map<uint32_t, uint32_t>;
     map st1 = { {3, 0}, {6, 0}, {7, 0}, {9, 0} };
-    auto num_erased = std::erase_if(st1, [](const map::value_type& v) { return v.first >= 7; });
+    auto num_erased = erase_if(st1, [](const map::value_type& v) { return v.first >= 7; });
     EXPECT_TRUE(num_erased == 2);
 
     map st2 = { {0, 0}, {2, 0}, {3, 0}, {6, 0} };
-    num_erased = std::erase_if(st2, [](const map::value_type& v) { return v.first <= 2; });
+    num_erased = erase_if(st2, [](const map::value_type& v) { return v.first <= 2; });
     EXPECT_TRUE(num_erased == 2);
 
     EXPECT_TRUE(st1 == st2);