From 8f8ea09ce8c2bc65294d17663edac8296d850ed5 Mon Sep 17 00:00:00 2001 From: Daniel James Date: Sun, 7 Oct 2012 08:19:01 +0000 Subject: [PATCH] Unordered: Fix bug when erasing a range, refs #7471. [SVN r80894] --- doc/changes.qbk | 4 +++ include/boost/unordered/detail/equivalent.hpp | 4 +-- include/boost/unordered/detail/table.hpp | 11 +++++++- test/unordered/erase_equiv_tests.cpp | 26 +++++++++++++++++++ test/unordered/erase_tests.cpp | 14 ++++++++++ 5 files changed, 56 insertions(+), 3 deletions(-) diff --git a/doc/changes.qbk b/doc/changes.qbk index d09360a2..f4e42f63 100644 --- a/doc/changes.qbk +++ b/doc/changes.qbk @@ -3,6 +3,9 @@ / Distributed under the Boost Software License, Version 1.0. (See accompanying / file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) ] +[template ticket[number]''''''#[number]''''''] + [section:changes Change Log] [h2 Review Version] @@ -211,6 +214,7 @@ C++11 support has resulted in some breaking changes: * Faster assign, which assigns to existing nodes where possible, rather than creating entirely new nodes and copy constructing. +* Fixed bug in `erase_range` ([ticket 7471]). * Reverted some of the internal changes to how nodes are created, especially for C++11 compilers. 'construct' and 'destroy' should work a little better for C++11 allocators. diff --git a/include/boost/unordered/detail/equivalent.hpp b/include/boost/unordered/detail/equivalent.hpp index c112b398..3558b1cb 100644 --- a/include/boost/unordered/detail/equivalent.hpp +++ b/include/boost/unordered/detail/equivalent.hpp @@ -676,9 +676,9 @@ namespace boost { namespace unordered { namespace detail { if(begin == group2) { link_pointer end1 = group1->group_prev_; - link_pointer end2 = group2->group_prev_; + link_pointer end2 = end->group_prev_; group1->group_prev_ = end2; - group2->group_prev_ = end1; + end->group_prev_ = end1; } } } diff --git a/include/boost/unordered/detail/table.hpp b/include/boost/unordered/detail/table.hpp index a1c4dd69..af376fe7 100644 --- a/include/boost/unordered/detail/table.hpp +++ b/include/boost/unordered/detail/table.hpp @@ -618,7 +618,16 @@ namespace boost { namespace unordered { namespace detail { { for(;;) { n = static_cast(n->next_); - if (n == end) return; + if (n == end) { + if (n) { + std::size_t new_bucket_index = + policy::to_bucket(bucket_count_, n->hash_); + if (bucket_index != new_bucket_index) { + get_bucket(new_bucket_index)->next_ = prev; + } + } + return; + } std::size_t new_bucket_index = policy::to_bucket(bucket_count_, n->hash_); diff --git a/test/unordered/erase_equiv_tests.cpp b/test/unordered/erase_equiv_tests.cpp index d487a075..c81a9ad9 100644 --- a/test/unordered/erase_equiv_tests.cpp +++ b/test/unordered/erase_equiv_tests.cpp @@ -12,6 +12,7 @@ #include "../helpers/test.hpp" #include "../helpers/list.hpp" +#include "../helpers/invariants.hpp" #include #include #include @@ -51,12 +52,21 @@ struct collision2_hash int operator()(int x) const { return x & 1; } }; +// For testing erase in lots of buckets. +struct collision3_hash +{ + int operator()(int x) const { return x; } +}; + typedef boost::unordered_multimap, test::allocator1 > > collide_map; typedef boost::unordered_multimap, test::allocator2 > > collide_map2; +typedef boost::unordered_multimap, + test::allocator2 > > collide_map3; typedef collide_map::value_type collide_value; typedef test::list collide_list; @@ -66,6 +76,7 @@ UNORDERED_AUTO_TEST(empty_range_tests) x.erase(x.begin(), x.end()); x.erase(x.begin(), x.begin()); x.erase(x.end(), x.end()); + test::check_equivalent_keys(x); } UNORDERED_AUTO_TEST(single_item_tests) @@ -76,10 +87,13 @@ UNORDERED_AUTO_TEST(single_item_tests) collide_map x(init.begin(), init.end()); x.erase(x.begin(), x.begin()); BOOST_TEST(x.count(1) == 1 && x.size() == 1); + test::check_equivalent_keys(x); x.erase(x.end(), x.end()); BOOST_TEST(x.count(1) == 1 && x.size() == 1); + test::check_equivalent_keys(x); x.erase(x.begin(), x.end()); BOOST_TEST(x.count(1) == 0 && x.size() == 0); + test::check_equivalent_keys(x); } UNORDERED_AUTO_TEST(two_equivalent_item_tests) @@ -92,6 +106,7 @@ UNORDERED_AUTO_TEST(two_equivalent_item_tests) collide_map x(init.begin(), init.end()); x.erase(x.begin(), x.end()); BOOST_TEST(x.count(1) == 0 && x.size() == 0); + test::check_equivalent_keys(x); } { @@ -100,6 +115,7 @@ UNORDERED_AUTO_TEST(two_equivalent_item_tests) x.erase(x.begin(), boost::next(x.begin())); BOOST_TEST(x.count(1) == 1 && x.size() == 1 && x.begin()->first == 1 && x.begin()->second == value); + test::check_equivalent_keys(x); } { @@ -108,6 +124,7 @@ UNORDERED_AUTO_TEST(two_equivalent_item_tests) x.erase(boost::next(x.begin()), x.end()); BOOST_TEST(x.count(1) == 1 && x.size() == 1 && x.begin()->first == 1 && x.begin()->second == value); + test::check_equivalent_keys(x); } } @@ -129,6 +146,8 @@ bool general_erase_range_test(Container& x, std::size_t start, std::size_t end) collide_list l(x.begin(), x.end()); l.erase(boost::next(l.begin(), start), boost::next(l.begin(), end)); x.erase(boost::next(x.begin(), start), boost::next(x.begin(), end)); + + test::check_equivalent_keys(x); return compare(l, x); } @@ -191,4 +210,11 @@ UNORDERED_AUTO_TEST(exhaustive_collide2_tests) std::cout<<"\n"; } +UNORDERED_AUTO_TEST(exhaustive_collide3_tests) +{ + std::cout<<"exhaustive_collide3_tests:\n"; + exhaustive_erase_tests((collide_map3*) 0, 8, 4); + std::cout<<"\n"; +} + RUN_TESTS() diff --git a/test/unordered/erase_tests.cpp b/test/unordered/erase_tests.cpp index c634b890..4135af4d 100644 --- a/test/unordered/erase_tests.cpp +++ b/test/unordered/erase_tests.cpp @@ -15,6 +15,7 @@ #include "../helpers/tracker.hpp" #include "../helpers/equivalent.hpp" #include "../helpers/helpers.hpp" +#include "../helpers/invariants.hpp" #include @@ -32,6 +33,7 @@ void erase_tests1(Container*, test::random_generator generator) test::random_values v(1000, generator); Container x(v.begin(), v.end()); + int iterations = 0; for(BOOST_DEDUCED_TYPENAME test::random_values::iterator it = v.begin(); it != v.end(); ++it) { @@ -41,6 +43,7 @@ void erase_tests1(Container*, test::random_generator generator) BOOST_TEST(x.size() == old_size - count); BOOST_TEST(x.count(test::get_key(*it)) == 0); BOOST_TEST(x.find(test::get_key(*it)) == x.end()); + if (++iterations % 20 == 0) test::check_equivalent_keys(x); } } @@ -51,6 +54,7 @@ void erase_tests1(Container*, test::random_generator generator) test::random_values v(1000, generator); Container x(v.begin(), v.end()); std::size_t size = x.size(); + int iterations = 0; while(size > 0 && !x.empty()) { BOOST_DEDUCED_TYPENAME Container::key_type @@ -62,6 +66,7 @@ void erase_tests1(Container*, test::random_generator generator) BOOST_TEST(pos == x.begin()); BOOST_TEST(x.count(key) == count - 1); BOOST_TEST(x.size() == size); + if (++iterations % 20 == 0) test::check_equivalent_keys(x); } BOOST_TEST(x.empty()); } @@ -73,6 +78,7 @@ void erase_tests1(Container*, test::random_generator generator) test::random_values v(1000, generator); Container x(v.begin(), v.end()); std::size_t size = x.size(); + int iterations = 0; while(size > 0 && !x.empty()) { using namespace std; @@ -96,6 +102,7 @@ void erase_tests1(Container*, test::random_generator generator) next == boost::next(prev)); BOOST_TEST(x.count(key) == count - 1); BOOST_TEST(x.size() == size); + if (++iterations % 20 == 0) test::check_equivalent_keys(x); } BOOST_TEST(x.empty()); } @@ -116,12 +123,15 @@ void erase_tests1(Container*, test::random_generator generator) BOOST_TEST(x.erase(x.end(), x.end()) == x.end()); BOOST_TEST(x.erase(x.begin(), x.begin()) == x.begin()); BOOST_TEST(x.size() == size); + test::check_equivalent_keys(x); BOOST_TEST(x.erase(x.begin(), x.end()) == x.end()); BOOST_TEST(x.empty()); BOOST_TEST(x.begin() == x.end()); + test::check_equivalent_keys(x); BOOST_TEST(x.erase(x.begin(), x.end()) == x.begin()); + test::check_equivalent_keys(x); } std::cerr<<"quick_erase(begin()).\n"; @@ -131,6 +141,7 @@ void erase_tests1(Container*, test::random_generator generator) test::random_values v(1000, generator); Container x(v.begin(), v.end()); std::size_t size = x.size(); + int iterations = 0; while(size > 0 && !x.empty()) { BOOST_DEDUCED_TYPENAME Container::key_type @@ -140,6 +151,7 @@ void erase_tests1(Container*, test::random_generator generator) --size; BOOST_TEST(x.count(key) == count - 1); BOOST_TEST(x.size() == size); + if (++iterations % 20 == 0) test::check_equivalent_keys(x); } BOOST_TEST(x.empty()); } @@ -151,6 +163,7 @@ void erase_tests1(Container*, test::random_generator generator) test::random_values v(1000, generator); Container x(v.begin(), v.end()); std::size_t size = x.size(); + int iterations = 0; while(size > 0 && !x.empty()) { using namespace std; @@ -174,6 +187,7 @@ void erase_tests1(Container*, test::random_generator generator) next == boost::next(prev)); BOOST_TEST(x.count(key) == count - 1); BOOST_TEST(x.size() == size); + if (++iterations % 20 == 0) test::check_equivalent_keys(x); } BOOST_TEST(x.empty()); }