From 03edf7f4a8a8e1e77349bdf37154297aa57d9b83 Mon Sep 17 00:00:00 2001 From: LeonineKing1199 Date: Thu, 2 Dec 2021 15:30:17 -0800 Subject: [PATCH 01/12] Add member function template `find_previous_node_impl` so it can be used in heterogenous contexts --- include/boost/unordered/detail/implementation.hpp | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/include/boost/unordered/detail/implementation.hpp b/include/boost/unordered/detail/implementation.hpp index daf9b8f6..eb11219b 100644 --- a/include/boost/unordered/detail/implementation.hpp +++ b/include/boost/unordered/detail/implementation.hpp @@ -3618,9 +3618,9 @@ namespace boost { } } - // Find the node before the key, so that it can be erased. - link_pointer find_previous_node( - const_key_type& k, std::size_t bucket_index) + template + link_pointer find_previous_node_impl( + KeyEqual const& eq, Key const& k, std::size_t const bucket_index) { link_pointer prev = this->get_previous_start(bucket_index); if (!prev) { @@ -3634,7 +3634,7 @@ namespace boost { } else if (n->is_first_in_group()) { if (node_bucket(n) != bucket_index) { return link_pointer(); - } else if (this->key_eq()(k, this->get_key(n))) { + } else if (eq(k, this->get_key(n))) { return prev; } } @@ -3642,6 +3642,13 @@ namespace boost { } } + // Find the node before the key, so that it can be erased. + link_pointer find_previous_node( + const_key_type& k, std::size_t bucket_index) + { + return find_previous_node_impl(this->key_eq(), k, bucket_index); + } + // Extract and erase inline node_pointer extract_by_key(const_key_type& k) From c9df887c4c68c619decbefe860cbb53e9102dfec Mon Sep 17 00:00:00 2001 From: LeonineKing1199 Date: Thu, 2 Dec 2021 15:38:07 -0800 Subject: [PATCH 02/12] Add member function template `erase_key_unique_impl` for usage in heterogeneous lookups --- include/boost/unordered/detail/implementation.hpp | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/include/boost/unordered/detail/implementation.hpp b/include/boost/unordered/detail/implementation.hpp index eb11219b..a6ebce08 100644 --- a/include/boost/unordered/detail/implementation.hpp +++ b/include/boost/unordered/detail/implementation.hpp @@ -4054,13 +4054,13 @@ namespace boost { // // no throw - std::size_t erase_key_unique(const_key_type& k) - { + template + std::size_t erase_key_unique_impl(KeyEqual const& eq, Key const& k) { if (!this->size_) return 0; std::size_t key_hash = this->hash(k); std::size_t bucket_index = this->hash_to_bucket(key_hash); - link_pointer prev = this->find_previous_node(k, bucket_index); + link_pointer prev = this->find_previous_node_impl(eq, k, bucket_index); if (!prev) return 0; node_pointer n = next_node(prev); @@ -4069,7 +4069,12 @@ namespace boost { --size_; this->fix_bucket(bucket_index, prev, n2); this->destroy_node(n); - return 1; + return 1; + } + + std::size_t erase_key_unique(const_key_type& k) + { + return this->erase_key_unique_impl(this->key_eq(), k); } void erase_nodes_unique(node_pointer i, node_pointer j) From f252480beec0954ce0fe859efde21054537cb309 Mon Sep 17 00:00:00 2001 From: LeonineKing1199 Date: Thu, 2 Dec 2021 15:44:02 -0800 Subject: [PATCH 03/12] Add missing formatting --- include/boost/unordered/detail/implementation.hpp | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/include/boost/unordered/detail/implementation.hpp b/include/boost/unordered/detail/implementation.hpp index a6ebce08..bebf7486 100644 --- a/include/boost/unordered/detail/implementation.hpp +++ b/include/boost/unordered/detail/implementation.hpp @@ -4055,21 +4055,28 @@ namespace boost { // no throw template - std::size_t erase_key_unique_impl(KeyEqual const& eq, Key const& k) { + std::size_t erase_key_unique_impl(KeyEqual const& eq, Key const& k) + { if (!this->size_) return 0; + std::size_t key_hash = this->hash(k); std::size_t bucket_index = this->hash_to_bucket(key_hash); - link_pointer prev = this->find_previous_node_impl(eq, k, bucket_index); + + link_pointer prev = + this->find_previous_node_impl(eq, k, bucket_index); + if (!prev) return 0; + node_pointer n = next_node(prev); node_pointer n2 = next_node(n); prev->next_ = n2; --size_; this->fix_bucket(bucket_index, prev, n2); this->destroy_node(n); - return 1; + + return 1; } std::size_t erase_key_unique(const_key_type& k) From 33f84624ec82f4a8bff53e062de34e4dba87415b Mon Sep 17 00:00:00 2001 From: LeonineKing1199 Date: Thu, 2 Dec 2021 15:59:12 -0800 Subject: [PATCH 04/12] Add initial draft of heterogeneous `erase()` --- include/boost/unordered/unordered_map.hpp | 15 ++++++++++++ test/unordered/transparent_tests.cpp | 30 +++++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/include/boost/unordered/unordered_map.hpp b/include/boost/unordered/unordered_map.hpp index 17dc987d..8621f913 100644 --- a/include/boost/unordered/unordered_map.hpp +++ b/include/boost/unordered/unordered_map.hpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #if !defined(BOOST_NO_CXX11_HDR_INITIALIZER_LIST) @@ -1382,6 +1383,20 @@ namespace boost { iterator erase(const_iterator); size_type erase(const key_type&); iterator erase(const_iterator, const_iterator); + + template + typename boost::enable_if_c< + detail::is_transparent::value && + detail::is_transparent::value && + !boost::is_convertible::value && + !boost::is_convertible::value, + size_type>::type + erase(BOOST_FWD_REF(Key) k) + { + return table_.erase_key_unique_impl( + this->key_eq(), boost::forward(k)); + } + BOOST_UNORDERED_DEPRECATED("Use erase instead") void quick_erase(const_iterator it) { erase(it); } BOOST_UNORDERED_DEPRECATED("Use erase instead") diff --git a/test/unordered/transparent_tests.cpp b/test/unordered/transparent_tests.cpp index 7acab1a4..5206ba2e 100644 --- a/test/unordered/transparent_tests.cpp +++ b/test/unordered/transparent_tests.cpp @@ -515,6 +515,36 @@ template void test_non_transparent_equal_range() } } +template void test_transparent_erase() +{ + count_reset(); + + UnorderedMap map; + + int num_erased = 0; + + num_erased = map.erase(0); + BOOST_TEST(map.empty()); + BOOST_TEST(num_erased == 0); + BOOST_TEST(key::count_ == 0); + + map[key(0)] = 1337; + map[key(1)] = 1338; + map[key(2)] = 1339; + + int const expected_key_count = 2 * map.size(); + + BOOST_TEST(key::count_ == expected_key_count); + + num_erased = map.erase(0); + BOOST_TEST(num_erased == 1); + + num_erased = map.erase(1337); + BOOST_TEST(num_erased == 0); + + BOOST_TEST(key::count_ == expected_key_count); +} + UNORDERED_AUTO_TEST (unordered_map_transparent_count) { { typedef boost::unordered_map Date: Fri, 3 Dec 2021 08:41:42 -0800 Subject: [PATCH 05/12] Add hopefully helpful comment to the source --- include/boost/unordered/detail/implementation.hpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/include/boost/unordered/detail/implementation.hpp b/include/boost/unordered/detail/implementation.hpp index bebf7486..fe1d0023 100644 --- a/include/boost/unordered/detail/implementation.hpp +++ b/include/boost/unordered/detail/implementation.hpp @@ -3631,7 +3631,11 @@ namespace boost { node_pointer n = next_node(prev); if (!n) { return link_pointer(); - } else if (n->is_first_in_group()) { + } + // the `first_in_group()` checks are required for the multi-containers + // for the unique containers, this condition seems to be always true + // + else if (n->is_first_in_group()) { if (node_bucket(n) != bucket_index) { return link_pointer(); } else if (eq(k, this->get_key(n))) { From 8b438dea76ac88ea94c43af15be20d6206e8c763 Mon Sep 17 00:00:00 2001 From: LeonineKing1199 Date: Fri, 3 Dec 2021 08:48:20 -0800 Subject: [PATCH 06/12] Use `erase_key_unique_impl()` directly so that eventually `erase_unique()` can be deprecated --- include/boost/unordered/unordered_map.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/boost/unordered/unordered_map.hpp b/include/boost/unordered/unordered_map.hpp index 8621f913..38e826fb 100644 --- a/include/boost/unordered/unordered_map.hpp +++ b/include/boost/unordered/unordered_map.hpp @@ -1797,7 +1797,7 @@ namespace boost { typename unordered_map::size_type unordered_map::erase(const key_type& k) { - return table_.erase_key_unique(k); + return table_.erase_key_unique_impl(this->key_eq(), k); } template From 4a42c93897267f9bad4d52944a64a39704b3ad48 Mon Sep 17 00:00:00 2001 From: LeonineKing1199 Date: Fri, 3 Dec 2021 10:07:57 -0800 Subject: [PATCH 07/12] Fix erroneous usage of `table::hash()` impl which implicitly copy-constructs the `const_key_type` --- include/boost/unordered/detail/implementation.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/boost/unordered/detail/implementation.hpp b/include/boost/unordered/detail/implementation.hpp index fe1d0023..f4874d57 100644 --- a/include/boost/unordered/detail/implementation.hpp +++ b/include/boost/unordered/detail/implementation.hpp @@ -4064,7 +4064,7 @@ namespace boost { if (!this->size_) return 0; - std::size_t key_hash = this->hash(k); + std::size_t key_hash = policy::apply_hash(this->hash_function(), k); std::size_t bucket_index = this->hash_to_bucket(key_hash); link_pointer prev = From e4d0693eb9cb89cec236d20cb5ea5684479dfe6e Mon Sep 17 00:00:00 2001 From: LeonineKing1199 Date: Fri, 3 Dec 2021 10:08:30 -0800 Subject: [PATCH 08/12] Fix erroneous placement of heterogeneous `erase()` from `multimap` to `map` --- include/boost/unordered/unordered_map.hpp | 27 ++++++++++++----------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/include/boost/unordered/unordered_map.hpp b/include/boost/unordered/unordered_map.hpp index 38e826fb..1577797d 100644 --- a/include/boost/unordered/unordered_map.hpp +++ b/include/boost/unordered/unordered_map.hpp @@ -709,6 +709,20 @@ namespace boost { iterator erase(const_iterator); size_type erase(const key_type&); iterator erase(const_iterator, const_iterator); + + template + typename boost::enable_if_c< + detail::is_transparent::value && + detail::is_transparent::value && + !boost::is_convertible::value && + !boost::is_convertible::value, + size_type>::type + erase(BOOST_FWD_REF(Key) k) + { + return table_.erase_key_unique_impl( + this->key_eq(), boost::forward(k)); + } + BOOST_UNORDERED_DEPRECATED("Use erase instead") void quick_erase(const_iterator it) { erase(it); } BOOST_UNORDERED_DEPRECATED("Use erase instead") @@ -1384,19 +1398,6 @@ namespace boost { size_type erase(const key_type&); iterator erase(const_iterator, const_iterator); - template - typename boost::enable_if_c< - detail::is_transparent::value && - detail::is_transparent::value && - !boost::is_convertible::value && - !boost::is_convertible::value, - size_type>::type - erase(BOOST_FWD_REF(Key) k) - { - return table_.erase_key_unique_impl( - this->key_eq(), boost::forward(k)); - } - BOOST_UNORDERED_DEPRECATED("Use erase instead") void quick_erase(const_iterator it) { erase(it); } BOOST_UNORDERED_DEPRECATED("Use erase instead") From 52f154ec0266e1c0e635c786d875a08a46f8789f Mon Sep 17 00:00:00 2001 From: LeonineKing1199 Date: Fri, 3 Dec 2021 10:17:50 -0800 Subject: [PATCH 09/12] Flesh out test suite for heterogeneous `erase()` --- test/unordered/transparent_tests.cpp | 77 +++++++++++++++++++++++++++- 1 file changed, 75 insertions(+), 2 deletions(-) diff --git a/test/unordered/transparent_tests.cpp b/test/unordered/transparent_tests.cpp index 5206ba2e..ff782632 100644 --- a/test/unordered/transparent_tests.cpp +++ b/test/unordered/transparent_tests.cpp @@ -48,6 +48,11 @@ struct transparent_key_equal was_called_ = true; return k1.x_ == x; } + bool operator()(key const& k1, int const x) const + { + was_called_ = true; + return k1.x_ == x; + } }; bool transparent_key_equal::was_called_; @@ -515,13 +520,34 @@ template void test_non_transparent_equal_range() } } +template struct convertible_to_iterator +{ + operator typename UnorderedMap::iterator() + { + return typename UnorderedMap::iterator(); + } +}; + +typedef boost::unordered_map + transparent_unordered_map; + +transparent_unordered_map::iterator erase_overload_compile_test() +{ + convertible_to_iterator c; + transparent_unordered_map map; + transparent_unordered_map::iterator pos = map.begin(); + pos = c; + return map.erase(c); +} + template void test_transparent_erase() { count_reset(); UnorderedMap map; - int num_erased = 0; + unsigned long num_erased = 0; num_erased = map.erase(0); BOOST_TEST(map.empty()); @@ -532,19 +558,62 @@ template void test_transparent_erase() map[key(1)] = 1338; map[key(2)] = 1339; - int const expected_key_count = 2 * map.size(); + BOOST_TEST(map.size() == 3); + BOOST_TEST(map.find(0) != map.end()); + int const expected_key_count = static_cast(2 * map.size()); BOOST_TEST(key::count_ == expected_key_count); num_erased = map.erase(0); + BOOST_TEST(key::count_ == expected_key_count); BOOST_TEST(num_erased == 1); + BOOST_TEST(map.size() == 2); + BOOST_TEST(map.find(0) == map.end()); + BOOST_TEST(key::count_ == expected_key_count); num_erased = map.erase(1337); BOOST_TEST(num_erased == 0); + BOOST_TEST(map.size() == 2); BOOST_TEST(key::count_ == expected_key_count); } +template void test_non_transparent_erase() +{ + count_reset(); + + UnorderedMap map; + + unsigned long num_erased = 0; + + num_erased = map.erase(0); + BOOST_TEST(map.empty()); + BOOST_TEST(num_erased == 0); + BOOST_TEST(key::count_ == 1); + + map[key(0)] = 1337; + map[key(1)] = 1338; + map[key(2)] = 1339; + + BOOST_TEST(map.size() == 3); + BOOST_TEST(map.find(0) != map.end()); + + int key_count = 2 + static_cast(2 * map.size()); + BOOST_TEST(key::count_ == key_count); + + num_erased = map.erase(0); + BOOST_TEST(key::count_ == ++key_count); + BOOST_TEST(num_erased == 1); + BOOST_TEST(map.size() == 2); + BOOST_TEST(map.find(0) == map.end()); + BOOST_TEST(key::count_ == ++key_count); + + num_erased = map.erase(1337); + BOOST_TEST(num_erased == 0); + BOOST_TEST(map.size() == 2); + BOOST_TEST(key::count_ == ++key_count); +} + UNORDERED_AUTO_TEST (unordered_map_transparent_count) { { typedef boost::unordered_map(); test_transparent_find(); test_transparent_equal_range(); + test_transparent_erase(); } { @@ -564,6 +634,7 @@ UNORDERED_AUTO_TEST (unordered_map_transparent_count) { test_non_transparent_count(); test_non_transparent_find(); test_non_transparent_equal_range(); + test_non_transparent_erase(); } { @@ -575,6 +646,7 @@ UNORDERED_AUTO_TEST (unordered_map_transparent_count) { test_non_transparent_count(); test_non_transparent_find(); test_non_transparent_equal_range(); + test_non_transparent_erase(); } { @@ -586,6 +658,7 @@ UNORDERED_AUTO_TEST (unordered_map_transparent_count) { test_non_transparent_count(); test_non_transparent_find(); test_non_transparent_equal_range(); + test_non_transparent_erase(); } } From b8d3aa2a686031498710822255e7a7f51e6a3c69 Mon Sep 17 00:00:00 2001 From: LeonineKing1199 Date: Fri, 3 Dec 2021 11:19:58 -0800 Subject: [PATCH 10/12] Light cleanup of test --- test/unordered/transparent_tests.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/unordered/transparent_tests.cpp b/test/unordered/transparent_tests.cpp index ff782632..089747ba 100644 --- a/test/unordered/transparent_tests.cpp +++ b/test/unordered/transparent_tests.cpp @@ -532,6 +532,9 @@ typedef boost::unordered_map transparent_unordered_map; +// test that in the presence of the member function template `erase()`, we still +// invoke the correct iterator overloads when the type is implicitly convertible +// transparent_unordered_map::iterator erase_overload_compile_test() { convertible_to_iterator c; @@ -565,11 +568,9 @@ template void test_transparent_erase() BOOST_TEST(key::count_ == expected_key_count); num_erased = map.erase(0); - BOOST_TEST(key::count_ == expected_key_count); BOOST_TEST(num_erased == 1); BOOST_TEST(map.size() == 2); BOOST_TEST(map.find(0) == map.end()); - BOOST_TEST(key::count_ == expected_key_count); num_erased = map.erase(1337); BOOST_TEST(num_erased == 0); From f5b03fb2e8415ca9b5d73bbdc9b05a5cd987b747 Mon Sep 17 00:00:00 2001 From: LeonineKing1199 Date: Mon, 6 Dec 2021 08:30:57 -0800 Subject: [PATCH 11/12] Pull out expressions with side-effects from the testing assertions --- test/unordered/transparent_tests.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/test/unordered/transparent_tests.cpp b/test/unordered/transparent_tests.cpp index 089747ba..22377ec0 100644 --- a/test/unordered/transparent_tests.cpp +++ b/test/unordered/transparent_tests.cpp @@ -603,16 +603,21 @@ template void test_non_transparent_erase() BOOST_TEST(key::count_ == key_count); num_erased = map.erase(0); - BOOST_TEST(key::count_ == ++key_count); + ++key_count; + BOOST_TEST(key::count_ == key_count); BOOST_TEST(num_erased == 1); BOOST_TEST(map.size() == 2); + BOOST_TEST(map.find(0) == map.end()); - BOOST_TEST(key::count_ == ++key_count); + ++key_count; + + BOOST_TEST(key::count_ == key_count); num_erased = map.erase(1337); + ++key_count; BOOST_TEST(num_erased == 0); BOOST_TEST(map.size() == 2); - BOOST_TEST(key::count_ == ++key_count); + BOOST_TEST(key::count_ == key_count); } UNORDERED_AUTO_TEST (unordered_map_transparent_count) { From afb83a6cb9f36ffe2ce896ccbf8c8f7ab119654d Mon Sep 17 00:00:00 2001 From: LeonineKing1199 Date: Mon, 6 Dec 2021 08:52:02 -0800 Subject: [PATCH 12/12] Refactor `erase()` tests to use `BOOST_TEST_EQ` where applicable --- test/unordered/transparent_tests.cpp | 42 ++++++++++++++-------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/test/unordered/transparent_tests.cpp b/test/unordered/transparent_tests.cpp index 22377ec0..bbcb240b 100644 --- a/test/unordered/transparent_tests.cpp +++ b/test/unordered/transparent_tests.cpp @@ -554,29 +554,29 @@ template void test_transparent_erase() num_erased = map.erase(0); BOOST_TEST(map.empty()); - BOOST_TEST(num_erased == 0); - BOOST_TEST(key::count_ == 0); + BOOST_TEST_EQ(num_erased, 0); + BOOST_TEST_EQ(key::count_, 0); map[key(0)] = 1337; map[key(1)] = 1338; map[key(2)] = 1339; - BOOST_TEST(map.size() == 3); + BOOST_TEST_EQ(map.size(), 3); BOOST_TEST(map.find(0) != map.end()); int const expected_key_count = static_cast(2 * map.size()); - BOOST_TEST(key::count_ == expected_key_count); + BOOST_TEST_EQ(key::count_, expected_key_count); num_erased = map.erase(0); - BOOST_TEST(num_erased == 1); - BOOST_TEST(map.size() == 2); + BOOST_TEST_EQ(num_erased, 1); + BOOST_TEST_EQ(map.size(), 2); BOOST_TEST(map.find(0) == map.end()); num_erased = map.erase(1337); - BOOST_TEST(num_erased == 0); - BOOST_TEST(map.size() == 2); + BOOST_TEST_EQ(num_erased, 0); + BOOST_TEST_EQ(map.size(), 2); - BOOST_TEST(key::count_ == expected_key_count); + BOOST_TEST_EQ(key::count_, expected_key_count); } template void test_non_transparent_erase() @@ -589,35 +589,35 @@ template void test_non_transparent_erase() num_erased = map.erase(0); BOOST_TEST(map.empty()); - BOOST_TEST(num_erased == 0); - BOOST_TEST(key::count_ == 1); + BOOST_TEST_EQ(num_erased, 0); + BOOST_TEST_EQ(key::count_, 1); map[key(0)] = 1337; map[key(1)] = 1338; map[key(2)] = 1339; - BOOST_TEST(map.size() == 3); + BOOST_TEST_EQ(map.size(), 3); BOOST_TEST(map.find(0) != map.end()); int key_count = 2 + static_cast(2 * map.size()); - BOOST_TEST(key::count_ == key_count); + BOOST_TEST_EQ(key::count_, key_count); num_erased = map.erase(0); ++key_count; - BOOST_TEST(key::count_ == key_count); - BOOST_TEST(num_erased == 1); - BOOST_TEST(map.size() == 2); + BOOST_TEST_EQ(key::count_, key_count); + BOOST_TEST_EQ(num_erased, 1); + BOOST_TEST_EQ(map.size(), 2); BOOST_TEST(map.find(0) == map.end()); ++key_count; - - BOOST_TEST(key::count_ == key_count); + + BOOST_TEST_EQ(key::count_, key_count); num_erased = map.erase(1337); ++key_count; - BOOST_TEST(num_erased == 0); - BOOST_TEST(map.size() == 2); - BOOST_TEST(key::count_ == key_count); + BOOST_TEST_EQ(num_erased, 0); + BOOST_TEST_EQ(map.size(), 2); + BOOST_TEST_EQ(key::count_, key_count); } UNORDERED_AUTO_TEST (unordered_map_transparent_count) {