diff --git a/doc/changes.qbk b/doc/changes.qbk index fdaf3f89..839b370f 100644 --- a/doc/changes.qbk +++ b/doc/changes.qbk @@ -274,5 +274,7 @@ C++11 support has resulted in some breaking changes: * Remove use of deprecated `boost::iterator`. * Remove `BOOST_NO_STD_DISTANCE` workaround. * Remove `BOOST_UNORDERED_DEPRECATED_EQUALITY` warning. +* Simpler implementation of assignment, fixes an exception safety issue + for `unordered_multiset` and `unordered_multimap`. Might be a little slower. [endsect] diff --git a/include/boost/unordered/detail/equivalent.hpp b/include/boost/unordered/detail/equivalent.hpp index a62a3571..407b5f79 100644 --- a/include/boost/unordered/detail/equivalent.hpp +++ b/include/boost/unordered/detail/equivalent.hpp @@ -385,7 +385,14 @@ namespace boost { namespace unordered { namespace detail { std::size_t key_hash, iterator pos) { - node_pointer n = a.release(); + return add_node(a.release(), key_hash, pos); + } + + inline iterator add_node( + node_pointer n, + std::size_t key_hash, + iterator pos) + { n->hash_ = key_hash; if (pos.node_) { this->add_after_node(n, pos.node_); @@ -622,30 +629,17 @@ namespace boost { namespace unordered { namespace detail { // fill_buckets template - static void fill_buckets(iterator n, table& dst, - NodeCreator& creator) + void fill_buckets(iterator n, NodeCreator& creator) { - link_pointer prev = dst.get_previous_start(); - while (n.node_) { std::size_t key_hash = n.node_->hash_; iterator group_end(n.node_->group_prev_->next_); - node_pointer first_node = creator.create(*n); - node_pointer end = first_node; - first_node->hash_ = key_hash; - prev->next_ = first_node; - ++dst.size_; - + iterator pos = this->add_node(creator.create(*n), key_hash, iterator()); for (++n; n != group_end; ++n) { - end = creator.create(*n); - end->hash_ = key_hash; - add_after_node(end, first_node); - ++dst.size_; + this->add_node(creator.create(*n), key_hash, pos); } - - prev = place_in_bucket(dst, prev, end); } } diff --git a/include/boost/unordered/detail/table.hpp b/include/boost/unordered/detail/table.hpp index f0b9a8b0..a337f45b 100644 --- a/include/boost/unordered/detail/table.hpp +++ b/include/boost/unordered/detail/table.hpp @@ -393,7 +393,7 @@ namespace boost { namespace unordered { namespace detail { if (x.size_) { create_buckets(bucket_count_); copy_nodes node_creator(node_alloc()); - table_impl::fill_buckets(x.begin(), *this, node_creator); + static_cast(this)->fill_buckets(x.begin(), node_creator); } } @@ -408,7 +408,7 @@ namespace boost { namespace unordered { namespace detail { move_nodes node_creator(node_alloc()); node_holder nodes(x); - table_impl::fill_buckets(nodes.begin(), *this, node_creator); + static_cast(this)->fill_buckets(nodes.begin(), node_creator); } } @@ -654,7 +654,7 @@ namespace boost { namespace unordered { namespace detail { // assigning to them if possible, and deleting any that are // left over. assign_nodes node_creator(*this); - table_impl::fill_buckets(x.begin(), *this, node_creator); + static_cast(this)->fill_buckets(x.begin(), node_creator); } void assign(table const& x, true_type) @@ -681,7 +681,7 @@ namespace boost { namespace unordered { namespace detail { if (x.size_) { create_buckets(bucket_count_); copy_nodes node_creator(node_alloc()); - table_impl::fill_buckets(x.begin(), *this, node_creator); + static_cast(this)->fill_buckets(x.begin(), node_creator); } } } @@ -740,7 +740,7 @@ namespace boost { namespace unordered { namespace detail { // any that are left over. move_assign_nodes
node_creator(*this); node_holder nodes(x); - table_impl::fill_buckets(nodes.begin(), *this, node_creator); + static_cast(this)->fill_buckets(nodes.begin(), node_creator); } } diff --git a/include/boost/unordered/detail/unique.hpp b/include/boost/unordered/detail/unique.hpp index f76ca5a6..a318d0b3 100644 --- a/include/boost/unordered/detail/unique.hpp +++ b/include/boost/unordered/detail/unique.hpp @@ -311,7 +311,13 @@ namespace boost { namespace unordered { namespace detail { node_constructor& a, std::size_t key_hash) { - node_pointer n = a.release(); + return add_node(a.release(), key_hash); + } + + inline iterator add_node( + node_pointer n, + std::size_t key_hash) + { n->hash_ = key_hash; bucket_pointer b = this->get_bucket(this->hash_to_bucket(key_hash)); @@ -577,19 +583,10 @@ namespace boost { namespace unordered { namespace detail { // fill_buckets template - static void fill_buckets(iterator n, table& dst, - NodeCreator& creator) + void fill_buckets(iterator n, NodeCreator& creator) { - link_pointer prev = dst.get_previous_start(); - - while (n.node_) { - node_pointer node = creator.create(*n); - node->hash_ = n.node_->hash_; - prev->next_ = node; - ++dst.size_; - ++n; - - prev = place_in_bucket(dst, prev); + for (; n.node_; ++n) { + this->add_node(creator.create(*n), n.node_->hash_); } } diff --git a/test/exception/assign_exception_tests.cpp b/test/exception/assign_exception_tests.cpp index 75be2adb..a9f4976f 100644 --- a/test/exception/assign_exception_tests.cpp +++ b/test/exception/assign_exception_tests.cpp @@ -38,21 +38,18 @@ struct self_assign_test2 : self_assign_base template struct assign_base : public test::exception_base { - const test::random_values x_values, y_values; + test::random_values x_values, y_values; T x,y; typedef BOOST_DEDUCED_TYPENAME T::hasher hasher; typedef BOOST_DEDUCED_TYPENAME T::key_equal key_equal; typedef BOOST_DEDUCED_TYPENAME T::allocator_type allocator_type; - assign_base(unsigned int count1, unsigned int count2, int tag1, int tag2, - float mlf1 = 1.0, float mlf2 = 1.0) : - x_values(count1), - y_values(count2), - x(x_values.begin(), x_values.end(), 0, hasher(tag1), key_equal(tag1), - allocator_type(tag1)), - y(y_values.begin(), y_values.end(), 0, hasher(tag2), key_equal(tag2), - allocator_type(tag2)) + assign_base(int tag1, int tag2, float mlf1 = 1.0, float mlf2 = 1.0) : + x_values(), + y_values(), + x(0, hasher(tag1), key_equal(tag1), allocator_type(tag1)), + y(0, hasher(tag2), key_equal(tag2), allocator_type(tag2)) { x.max_load_factor(mlf1); y.max_load_factor(mlf2); @@ -66,47 +63,80 @@ struct assign_base : public test::exception_base test::check_equivalent_keys(x1); // If the container is empty at the point of the exception, the - // internal structure is hidden, this exposes it. - T& y = const_cast(x1); + // internal structure is hidden, this exposes it, at the cost of + // messing up the data. if (x_values.size()) { - y.emplace(*x_values.begin()); - test::check_equivalent_keys(y); + T& x2 = const_cast(x1); + x2.emplace(*x_values.begin()); + test::check_equivalent_keys(x2); } } }; template -struct assign_test1 : assign_base +struct assign_values : assign_base { - assign_test1() : assign_base(0, 0, 0, 0) {} + assign_values(unsigned int count1, unsigned int count2, + int tag1, int tag2, float mlf1 = 1.0, float mlf2 = 1.0) : + assign_base(tag1, tag2, mlf1, mlf2) + { + this->x_values.fill(count1); + this->y_values.fill(count2); + this->x.insert(this->x_values.begin(), this->x_values.end()); + this->y.insert(this->y_values.begin(), this->y_values.end()); + } }; template -struct assign_test2 : assign_base +struct assign_test1 : assign_values { - assign_test2() : assign_base(60, 0, 0, 0) {} + assign_test1() : assign_values(0, 0, 0, 0) {} }; template -struct assign_test3 : assign_base +struct assign_test2 : assign_values { - assign_test3() : assign_base(0, 60, 0, 0) {} + assign_test2() : assign_values(60, 0, 0, 0) {} }; template -struct assign_test4 : assign_base +struct assign_test3 : assign_values { - assign_test4() : assign_base(10, 10, 1, 2) {} + assign_test3() : assign_values(0, 60, 0, 0) {} }; template -struct assign_test5 : assign_base +struct assign_test4 : assign_values { - assign_test5() : assign_base(5, 60, 0, 0, 1.0, 0.1) {} + assign_test4() : assign_values(10, 10, 1, 2) {} +}; + +template +struct assign_test5 : assign_values +{ + assign_test5() : assign_values(5, 60, 0, 0, 1.0f, 0.1f) {} +}; + +template +struct equivalent_test1 : assign_base +{ + equivalent_test1() : + assign_base(0, 0) + { + test::random_values x_values2(10); + this->x_values.insert(x_values2.begin(), x_values2.end()); + this->x_values.insert(x_values2.begin(), x_values2.end()); + test::random_values y_values2(10); + this->y_values.insert(y_values2.begin(), y_values2.end()); + this->y_values.insert(y_values2.begin(), y_values2.end()); + this->x.insert(this->x_values.begin(), this->x_values.end()); + this->y.insert(this->y_values.begin(), this->y_values.end()); + } }; EXCEPTION_TESTS( (self_assign_test1)(self_assign_test2) - (assign_test1)(assign_test2)(assign_test3)(assign_test4)(assign_test5), + (assign_test1)(assign_test2)(assign_test3)(assign_test4)(assign_test5) + (equivalent_test1), CONTAINER_SEQ) RUN_TESTS() diff --git a/test/helpers/random_values.hpp b/test/helpers/random_values.hpp index f8ba252c..b65a29b2 100644 --- a/test/helpers/random_values.hpp +++ b/test/helpers/random_values.hpp @@ -103,8 +103,16 @@ namespace test struct random_values : public test::list { - random_values(int count, test::random_generator const& generator = + random_values() {} + + explicit random_values(int count, test::random_generator const& generator = test::default_generator) + { + fill(count, generator); + } + + void fill(int count, test::random_generator const& generator = + test::default_generator) { test::unordered_generator gen(generator); gen.fill(*this, count);