Skip to content

Commit

Permalink
use ADL lookup for erase_if instead of adding to std (gtl reported is…
Browse files Browse the repository at this point in the history
…sue)
  • Loading branch information
greg7mdp committed Jun 25, 2022
1 parent ce7710e commit 01ea809
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 13 deletions.
20 changes: 13 additions & 7 deletions parallel_hashmap/phmap.h
Original file line number Diff line number Diff line change
Expand Up @@ -3335,6 +3335,7 @@ class parallel_hash_set
// 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) {
Expand Down Expand Up @@ -3366,15 +3367,22 @@ class parallel_hash_set
// ++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; }

Expand All @@ -3387,6 +3395,7 @@ class parallel_hash_set

// 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
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -5038,7 +5044,7 @@ namespace std {
return phmap::priv::erase_if(c, std::move(pred));
}

} // std
} // phmap

#ifdef _MSC_VER
#pragma warning(pop)
Expand Down
12 changes: 6 additions & 6 deletions tests/erase_if_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down

0 comments on commit 01ea809

Please sign in to comment.