From 0e5930b8dc0eb55150663249c0077d150300e116 Mon Sep 17 00:00:00 2001 From: Daniel James Date: Thu, 11 Aug 2011 21:18:43 +0000 Subject: [PATCH] Unordred: Implement propagate_on_container_swap. [SVN r73680] --- .../unordered/detail/allocator_helpers.hpp | 10 ++- include/boost/unordered/detail/buckets.hpp | 24 +++--- include/boost/unordered/detail/equivalent.hpp | 1 + include/boost/unordered/detail/table.hpp | 77 ++++++------------- include/boost/unordered/detail/unique.hpp | 1 + test/exception/swap_exception_tests.cpp | 14 ++-- test/objects/exception.hpp | 20 ++++- test/objects/minimal.hpp | 8 +- test/objects/test.hpp | 20 ++++- test/unordered/Jamfile.v2 | 2 +- test/unordered/swap_tests.cpp | 13 ---- 11 files changed, 95 insertions(+), 95 deletions(-) diff --git a/include/boost/unordered/detail/allocator_helpers.hpp b/include/boost/unordered/detail/allocator_helpers.hpp index 30fc94bc..bc4367dd 100644 --- a/include/boost/unordered/detail/allocator_helpers.hpp +++ b/include/boost/unordered/detail/allocator_helpers.hpp @@ -47,6 +47,12 @@ namespace boost { namespace unordered { namespace detail { }}} #endif +// TODO: Use std::addressof if available? +#include +namespace boost { namespace unordered { namespace detail { + using boost::addressof; +}}} + namespace boost { namespace unordered { namespace detail { #if BOOST_UNORDERED_USE_ALLOCATOR_TRAITS @@ -235,7 +241,7 @@ namespace boost { namespace unordered { namespace detail { ~allocator_array_constructor() { if (ptr_) { for(pointer p = ptr_; p != constructed_; ++p) - allocator_traits::destroy(alloc_, p); + allocator_traits::destroy(alloc_, addressof(*p)); allocator_traits::deallocate(alloc_, ptr_, length_); } @@ -249,7 +255,7 @@ namespace boost { namespace unordered { namespace detail { ptr_ = allocator_traits::allocate(alloc_, length_); pointer end = ptr_ + static_cast(length_); for(constructed_ = ptr_; constructed_ != end; ++constructed_) - allocator_traits::construct(alloc_, constructed_, v); + allocator_traits::construct(alloc_, addressof(*constructed_), v); } pointer get() const diff --git a/include/boost/unordered/detail/buckets.hpp b/include/boost/unordered/detail/buckets.hpp index 5710a83d..7dfde3a0 100644 --- a/include/boost/unordered/detail/buckets.hpp +++ b/include/boost/unordered/detail/buckets.hpp @@ -106,9 +106,9 @@ namespace boost { namespace unordered { namespace detail { // TODO: Need to move allocators_, not copy. But compressed_pair // doesn't support move parameters. - buckets(buckets& b, move_tag m) + buckets(buckets& b, move_tag) : buckets_(), - bucket_count_(b.bucket_count), + bucket_count_(b.bucket_count_), allocators_(b.allocators_) { } @@ -133,14 +133,20 @@ namespace boost { namespace unordered { namespace detail { this->buckets_ = constructor.release(); } - // no throw - void swap(buckets& other) + void swap(buckets& other, false_type = false_type()) { BOOST_ASSERT(node_alloc() == other.node_alloc()); std::swap(buckets_, other.buckets_); std::swap(bucket_count_, other.bucket_count_); } + void swap(buckets& other, true_type) + { + allocators_.swap(other.allocators_); + std::swap(buckets_, other.buckets_); + std::swap(bucket_count_, other.bucket_count_); + } + void move(buckets& other) { BOOST_ASSERT(node_alloc() == other.node_alloc()); @@ -189,11 +195,11 @@ namespace boost { namespace unordered { namespace detail { void delete_node(node_ptr n) { - node* raw_ptr = static_cast(&*n); + node* raw_ptr = static_cast(addressof(*n)); real_node_ptr real_ptr(node_alloc().address(*raw_ptr)); ::boost::unordered::detail::destroy(raw_ptr->value_ptr()); - allocator_traits::destroy(node_alloc(), real_ptr); + allocator_traits::destroy(node_alloc(), raw_ptr); allocator_traits::deallocate(node_alloc(), real_ptr, 1); } @@ -211,7 +217,7 @@ namespace boost { namespace unordered { namespace detail { ++end; for(bucket_ptr begin = this->buckets_; begin != end; ++begin) { - allocator_traits::destroy(bucket_alloc(), begin); + allocator_traits::destroy(bucket_alloc(), addressof(*begin)); } allocator_traits::deallocate(bucket_alloc(), this->buckets_, this->bucket_count_ + 1); @@ -580,7 +586,7 @@ namespace boost { namespace unordered { namespace detail { } if (node_constructed_) - allocator_traits::destroy(buckets_.node_alloc(), node_); + allocator_traits::destroy(buckets_.node_alloc(), addressof(*node_)); allocator_traits::deallocate(buckets_.node_alloc(), node_, 1); } @@ -594,7 +600,7 @@ namespace boost { namespace unordered { namespace detail { value_constructed_ = false; node_ = allocator_traits::allocate(buckets_.node_alloc(), 1); - allocator_traits::construct(buckets_.node_alloc(), node_, node()); + allocator_traits::construct(buckets_.node_alloc(), addressof(*node_), node()); node_->init(buckets_.bucket_alloc().address(*node_)); node_constructed_ = true; diff --git a/include/boost/unordered/detail/equivalent.hpp b/include/boost/unordered/detail/equivalent.hpp index 39b77cf3..66183a67 100644 --- a/include/boost/unordered/detail/equivalent.hpp +++ b/include/boost/unordered/detail/equivalent.hpp @@ -22,6 +22,7 @@ namespace boost { namespace unordered { namespace detail { typedef BOOST_DEDUCED_TYPENAME T::value_type value_type; typedef BOOST_DEDUCED_TYPENAME T::table_base table_base; typedef BOOST_DEDUCED_TYPENAME T::node_constructor node_constructor; + typedef BOOST_DEDUCED_TYPENAME T::node_allocator node_allocator; typedef BOOST_DEDUCED_TYPENAME T::node node; typedef BOOST_DEDUCED_TYPENAME T::node_ptr node_ptr; diff --git a/include/boost/unordered/detail/table.hpp b/include/boost/unordered/detail/table.hpp index 28fe3ba7..c28adcf9 100644 --- a/include/boost/unordered/detail/table.hpp +++ b/include/boost/unordered/detail/table.hpp @@ -176,8 +176,7 @@ namespace boost { namespace unordered { namespace detail { //////////////////////////////////////////////////////////////////////// // Constructors - table( - std::size_t num_buckets, + table(std::size_t num_buckets, hasher const& hf, key_equal const& eq, node_allocator const& a) @@ -202,8 +201,8 @@ namespace boost { namespace unordered { namespace detail { } } - table(table& x, move_tag) - : buckets(boost::move(x.node_alloc()), x.bucket_count_), + table(table& x, move_tag m) + : buckets(x, m), functions(x), size_(0), mlf_(1.0f), @@ -254,69 +253,43 @@ namespace boost { namespace unordered { namespace detail { void swap(table& x) { - if(this->node_alloc() == x.node_alloc()) { - if(this != &x) this->fast_swap(x); - } - else { - this->slow_swap(x); + // TODO: Should I actually swap the buckets even if this + // is with self? + if(this != &x) { + { + set_hash_functions op1(*this, x); + set_hash_functions op2(x, *this); + + this->buckets::swap(x, integral_constant:: + propagate_on_container_swap::value>()); + + op1.commit(); + op2.commit(); + } + + std::swap(this->size_, x.size_); + std::swap(this->mlf_, x.mlf_); + std::swap(this->max_load_, x.max_load_); } } + // Swap everything but the allocators void fast_swap(table& x) { - // These can throw, but they only affect the function objects - // that aren't in use so it is strongly exception safe, via. - // double buffering. { set_hash_functions op1(*this, x); set_hash_functions op2(x, *this); op1.commit(); op2.commit(); } - this->buckets::swap(x); // No throw - std::swap(this->size_, x.size_); - std::swap(this->mlf_, x.mlf_); - std::swap(this->max_load_, x.max_load_); - } - - void slow_swap(table& x) - { - if(this == &x) return; - - { - // These can throw, but they only affect the function objects - // that aren't in use so it is strongly exception safe, via. - // double buffering. - set_hash_functions op1(*this, x); - set_hash_functions op2(x, *this); - - // Create new buckets in separate buckets objects - // which will clean up if anything throws an exception. - // (all can throw, but with no effect as these are new objects). - - buckets b1(this->node_alloc(), x.min_buckets_for_size(x.size_)); - if (x.size_) x.copy_buckets_to(b1); - - buckets b2(x.node_alloc(), this->min_buckets_for_size(this->size_)); - if (this->size_) this->copy_buckets_to(b2); - - // Modifying the data, so no throw from now on. - - b1.swap(*this); - b2.swap(x); - op1.commit(); - op2.commit(); - } - - std::swap(this->size_, x.size_); - - this->max_load_ = !this->buckets_ ? 0 : this->calculate_max_load(); - x.max_load_ = !x.buckets_ ? 0 : x.calculate_max_load(); + partial_swap(x); } + // Swap everything but the allocators, and the functions objects. void partial_swap(table& x) { - this->buckets::swap(x); // No throw + this->buckets::swap(x, false_type()); std::swap(this->size_, x.size_); std::swap(this->mlf_, x.mlf_); std::swap(this->max_load_, x.max_load_); diff --git a/include/boost/unordered/detail/unique.hpp b/include/boost/unordered/detail/unique.hpp index 92b41a3a..d87131b1 100644 --- a/include/boost/unordered/detail/unique.hpp +++ b/include/boost/unordered/detail/unique.hpp @@ -22,6 +22,7 @@ namespace boost { namespace unordered { namespace detail { typedef BOOST_DEDUCED_TYPENAME T::value_type value_type; typedef BOOST_DEDUCED_TYPENAME T::table_base table_base; typedef BOOST_DEDUCED_TYPENAME T::node_constructor node_constructor; + typedef BOOST_DEDUCED_TYPENAME T::node_allocator node_allocator; typedef BOOST_DEDUCED_TYPENAME T::node node; typedef BOOST_DEDUCED_TYPENAME T::node_ptr node_ptr; diff --git a/test/exception/swap_exception_tests.cpp b/test/exception/swap_exception_tests.cpp index a3cfe823..570a54fa 100644 --- a/test/exception/swap_exception_tests.cpp +++ b/test/exception/swap_exception_tests.cpp @@ -27,13 +27,12 @@ struct self_swap_base : public test::exception_base void check BOOST_PREVENT_MACRO_SUBSTITUTION(T const& x) const { std::string scope(test::scope); -#if BOOST_UNORDERED_SWAP_METHOD != 2 + // TODO: In C++11 exceptions are only allowed in the swap function. BOOST_TEST( - scope == "hash::operator(hash)" || + scope == "hash::hash(hash)" || scope == "hash::operator=(hash)" || - scope == "equal_to::operator(equal_to)" || + scope == "equal_to::equal_to(equal_to)" || scope == "equal_to::operator=(equal_to)"); -#endif test::check_equivalent_keys(x); } @@ -82,13 +81,12 @@ struct swap_base : public test::exception_base void check BOOST_PREVENT_MACRO_SUBSTITUTION(data_type const& d) const { std::string scope(test::scope); -#if BOOST_UNORDERED_SWAP_METHOD != 2 + // TODO: In C++11 exceptions are only allowed in the swap function. BOOST_TEST( - scope == "hash::operator(hash)" || + scope == "hash::hash(hash)" || scope == "hash::operator=(hash)" || - scope == "equal_to::operator(equal_to)" || + scope == "equal_to::equal_to(equal_to)" || scope == "equal_to::operator=(equal_to)"); -#endif test::check_equivalent_keys(d.x); test::check_equivalent_keys(d.y); diff --git a/test/objects/exception.hpp b/test/objects/exception.hpp index d3d6f133..51123026 100644 --- a/test/objects/exception.hpp +++ b/test/objects/exception.hpp @@ -34,6 +34,16 @@ namespace exception template class allocator; object generate(object const*); + struct true_type + { + enum { value = true }; + }; + + struct false_type + { + enum { value = false }; + }; + class object { public: @@ -340,7 +350,7 @@ namespace exception } void construct(pointer p, T const& t) { - UNORDERED_SCOPE(allocator::construct(pointer, T)) { + UNORDERED_SCOPE(allocator::construct(T*, T)) { UNORDERED_EPOINT("Mock allocator construct function."); new(p) T(t); } @@ -348,7 +358,7 @@ namespace exception } #if defined(BOOST_UNORDERED_STD_FORWARD_MOVE) - template void construct(pointer p, Args&&... args) { + template void construct(T* p, Args&&... args) { UNORDERED_SCOPE(allocator::construct(pointer, Args&&...)) { UNORDERED_EPOINT("Mock allocator construct function."); new(p) T(std::forward(args)...); @@ -357,7 +367,7 @@ namespace exception } #endif - void destroy(pointer p) { + void destroy(T* p) { detail::tracker.track_destroy((void*) p, sizeof(T), tag_); p->~T(); } @@ -368,6 +378,10 @@ namespace exception } return (std::numeric_limits::max)(); } + + typedef true_type propagate_on_container_copy_assignment; + typedef true_type propagate_on_container_move_assignment; + typedef true_type propagate_on_container_swap; }; template diff --git a/test/objects/minimal.hpp b/test/objects/minimal.hpp index fbb5a869..65779032 100644 --- a/test/objects/minimal.hpp +++ b/test/objects/minimal.hpp @@ -279,15 +279,15 @@ namespace minimal ::operator delete((void*) p.ptr_); } - void construct(pointer p, T const& t) { new((void*)p.ptr_) T(t); } + void construct(T* p, T const& t) { new((void*)p) T(t); } #if defined(BOOST_UNORDERED_STD_FORWARD_MOVE) - template void construct(pointer p, Args&&... args) { - new((void*)p.ptr_) T(std::forward(args)...); + template void construct(T* p, Args&&... args) { + new((void*)p) T(std::forward(args)...); } #endif - void destroy(pointer p) { ((T*)p.ptr_)->~T(); } + void destroy(T* p) { p->~T(); } size_type max_size() const { return 1000; } diff --git a/test/objects/test.hpp b/test/objects/test.hpp index ec1cff5c..0db0fe56 100644 --- a/test/objects/test.hpp +++ b/test/objects/test.hpp @@ -27,6 +27,16 @@ namespace test template class allocator; object generate(object const*); implicitly_convertible generate(implicitly_convertible const*); + + struct true_type + { + enum { value = true }; + }; + + struct false_type + { + enum { value = false }; + }; class object : globally_counted_object { @@ -268,19 +278,19 @@ namespace test ::operator delete((void*) p); } - void construct(pointer p, T const& t) { + void construct(T* p, T const& t) { detail::tracker.track_construct((void*) p, sizeof(T), tag_); new(p) T(t); } #if defined(BOOST_UNORDERED_STD_FORWARD_MOVE) - template void construct(pointer p, Args&&... args) { + template void construct(T* p, Args&&... args) { detail::tracker.track_construct((void*) p, sizeof(T), tag_); new(p) T(std::forward(args)...); } #endif - void destroy(pointer p) { + void destroy(T* p) { detail::tracker.track_destroy((void*) p, sizeof(T), tag_); p->~T(); } @@ -298,6 +308,10 @@ namespace test { return tag_ != x.tag_; } + + typedef true_type propagate_on_container_copy_assignment; + typedef true_type propagate_on_container_move_assignment; + typedef true_type propagate_on_container_swap; }; template diff --git a/test/unordered/Jamfile.v2 b/test/unordered/Jamfile.v2 index fc343747..8c3fbbcf 100644 --- a/test/unordered/Jamfile.v2 +++ b/test/unordered/Jamfile.v2 @@ -43,5 +43,5 @@ test-suite unordered [ run load_factor_tests.cpp ] [ run rehash_tests.cpp ] [ run equality_tests.cpp ] - [ run swap_tests.cpp : : : BOOST_UNORDERED_SWAP_METHOD=2 ] + [ run swap_tests.cpp ] ; diff --git a/test/unordered/swap_tests.cpp b/test/unordered/swap_tests.cpp index 3705cce4..0e7c1a2e 100644 --- a/test/unordered/swap_tests.cpp +++ b/test/unordered/swap_tests.cpp @@ -92,18 +92,6 @@ void swap_tests2(X* ptr = 0, swap_test_impl(x, y); } -#if BOOST_UNORDERED_SWAP_METHOD == 1 - { - test::random_values vx(100, generator), vy(50, generator); - X x(vx.begin(), vx.end(), 0, hasher(), key_equal(), allocator_type(1)); - X y(vy.begin(), vy.end(), 0, hasher(), key_equal(), allocator_type(2)); - try { - swap_test_impl(x, y); - BOOST_ERROR("Using swap method 1, " - "swapping with unequal allocators didn't throw."); - } catch (std::runtime_error) {} - } -#else { test::random_values vx(50, generator), vy(100, generator); X x(vx.begin(), vx.end(), 0, hasher(), key_equal(), allocator_type(1)); @@ -120,7 +108,6 @@ void swap_tests2(X* ptr = 0, swap_test_impl(x, y); swap_test_impl(x, y); } -#endif } boost::unordered_set