feedc0de/boost_unordered
1
0
Fork 0
forked from boostorg/unordered
Code Pull Requests Activity

Fix exception safety in assignment for multimap/multiset.

Browse Source 
The assignment code seemed like a bit of a premature optimization, I
replaced it with a slightly slower but much simpler implementation.
This commit is contained in:
Daniel James
2016-05-30 11:29:20 +01:00
parent 1d4845d6b8
commit b4a3c6f460
6 changed files with 91 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
doc/changes.qbk
View File

@ -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]

28
include/boost/unordered/detail/equivalent.hpp
View File

@ -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 <class NodeCreator>
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);
}
}

10
include/boost/unordered/detail/table.hpp
View File

@ -393,7 +393,7 @@ namespace boost { namespace unordered { namespace detail {
if (x.size_) {
create_buckets(bucket_count_);
copy_nodes<node_allocator> node_creator(node_alloc());
table_impl::fill_buckets(x.begin(), *this, node_creator);
static_cast<table_impl*>(this)->fill_buckets(x.begin(), node_creator);
}
}
@ -408,7 +408,7 @@ namespace boost { namespace unordered { namespace detail {
move_nodes<node_allocator> node_creator(node_alloc());
node_holder<node_allocator> nodes(x);
table_impl::fill_buckets(nodes.begin(), *this, node_creator);
static_cast<table_impl*>(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<table> node_creator(*this);
table_impl::fill_buckets(x.begin(), *this, node_creator);
static_cast<table_impl*>(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_allocator> node_creator(node_alloc());
table_impl::fill_buckets(x.begin(), *this, node_creator);
static_cast<table_impl*>(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<table> node_creator(*this);
node_holder<node_allocator> nodes(x);
table_impl::fill_buckets(nodes.begin(), *this, node_creator);
static_cast<table_impl*>(this)->fill_buckets(nodes.begin(), node_creator);
}
}

23
include/boost/unordered/detail/unique.hpp
View File

@ -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 <class NodeCreator>
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_);
}
}

78
test/exception/assign_exception_tests.cpp
View File

@ -38,21 +38,18 @@ struct self_assign_test2 : self_assign_base<T>
template <class T>
struct assign_base : public test::exception_base
{
const test::random_values<T> x_values, y_values;
test::random_values<T> 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<T&>(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<T&>(x1);
x2.emplace(*x_values.begin());
test::check_equivalent_keys(x2);
}
}
};
template <class T>
struct assign_test1 : assign_base<T>
struct assign_values : assign_base<T>
{
assign_test1() : assign_base<T>(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<T>(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 <class T>
struct assign_test2 : assign_base<T>
struct assign_test1 : assign_values<T>
{
assign_test2() : assign_base<T>(60, 0, 0, 0) {}
assign_test1() : assign_values<T>(0, 0, 0, 0) {}
};
template <class T>
struct assign_test3 : assign_base<T>
struct assign_test2 : assign_values<T>
{
assign_test3() : assign_base<T>(0, 60, 0, 0) {}
assign_test2() : assign_values<T>(60, 0, 0, 0) {}
};
template <class T>
struct assign_test4 : assign_base<T>
struct assign_test3 : assign_values<T>
{
assign_test4() : assign_base<T>(10, 10, 1, 2) {}
assign_test3() : assign_values<T>(0, 60, 0, 0) {}
};
template <class T>
struct assign_test5 : assign_base<T>
struct assign_test4 : assign_values<T>
{
assign_test5() : assign_base<T>(5, 60, 0, 0, 1.0, 0.1) {}
assign_test4() : assign_values<T>(10, 10, 1, 2) {}
};
template <class T>
struct assign_test5 : assign_values<T>
{
assign_test5() : assign_values<T>(5, 60, 0, 0, 1.0f, 0.1f) {}
};
template <class T>
struct equivalent_test1 : assign_base<T>
{
equivalent_test1() :
assign_base<T>(0, 0)
{
test::random_values<T> 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<T> 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()

10
test/helpers/random_values.hpp
View File

@ -103,8 +103,16 @@ namespace test
struct random_values
: public test::list<BOOST_DEDUCED_TYPENAME X::value_type>
{
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<X> gen(generator);
gen.fill(*this, count);