diff --git a/doc/changes.qbk b/doc/changes.qbk index f71da60d..af8cf0a5 100644 --- a/doc/changes.qbk +++ b/doc/changes.qbk @@ -257,7 +257,16 @@ C++11 support has resulted in some breaking changes: * Fix the `pointer` typedef in iterators ([ticket 10672]). * Fix Coverity warning ([@https://github.com/boostorg/unordered/pull/2 GitHub #2]). + +[h2 Boost 1.58.0] + +* Remove unnecessary template parameter from const iterators. * Rename private `iterator` typedef in some iterator classes, as it confuses some traits classes. +* Fix move assignment with stateful, propagate_on_container_move_assign + allocators ([ticket 10777]). +* Fix rare exception safety issue in move assignment. +* Fix potential overflow when calculating number of buckets to allocate + ([@https://github.com/boostorg/unordered/pull/4 GitHub #4]). [endsect] diff --git a/include/boost/unordered/detail/buckets.hpp b/include/boost/unordered/detail/buckets.hpp index 7de4317b..8a67345c 100644 --- a/include/boost/unordered/detail/buckets.hpp +++ b/include/boost/unordered/detail/buckets.hpp @@ -45,9 +45,9 @@ namespace boost { namespace unordered { namespace iterator_detail { // all no throw template struct iterator; - template struct c_iterator; + template struct c_iterator; template struct l_iterator; - template + template struct cl_iterator; // Local Iterators @@ -64,12 +64,12 @@ namespace boost { namespace unordered { namespace iterator_detail { typename Node::value_type&> { #if !defined(BOOST_NO_MEMBER_TEMPLATE_FRIENDS) - template + template friend struct boost::unordered::iterator_detail::cl_iterator; private: #endif typedef typename Node::node_pointer node_pointer; - typedef boost::unordered::iterator_detail::iterator iterator; + typedef boost::unordered::iterator_detail::iterator n_iterator; node_pointer ptr_; std::size_t bucket_; std::size_t bucket_count_; @@ -80,7 +80,7 @@ namespace boost { namespace unordered { namespace iterator_detail { l_iterator() BOOST_NOEXCEPT : ptr_() {} - l_iterator(iterator x, std::size_t b, std::size_t c) BOOST_NOEXCEPT + l_iterator(n_iterator x, std::size_t b, std::size_t c) BOOST_NOEXCEPT : ptr_(x.node_), bucket_(b), bucket_count_(c) {} value_type& operator*() const { @@ -114,7 +114,7 @@ namespace boost { namespace unordered { namespace iterator_detail { } }; - template + template struct cl_iterator : public boost::iterator< std::forward_iterator_tag, @@ -128,7 +128,7 @@ namespace boost { namespace unordered { namespace iterator_detail { private: typedef typename Node::node_pointer node_pointer; - typedef boost::unordered::iterator_detail::iterator iterator; + typedef boost::unordered::iterator_detail::iterator n_iterator; node_pointer ptr_; std::size_t bucket_; std::size_t bucket_count_; @@ -139,7 +139,7 @@ namespace boost { namespace unordered { namespace iterator_detail { cl_iterator() BOOST_NOEXCEPT : ptr_() {} - cl_iterator(iterator x, std::size_t b, std::size_t c) BOOST_NOEXCEPT : + cl_iterator(n_iterator x, std::size_t b, std::size_t c) BOOST_NOEXCEPT : ptr_(x.node_), bucket_(b), bucket_count_(c) {} cl_iterator(boost::unordered::iterator_detail::l_iterator< @@ -192,11 +192,11 @@ namespace boost { namespace unordered { namespace iterator_detail { typename Node::value_type&> { #if !defined(BOOST_NO_MEMBER_TEMPLATE_FRIENDS) - template + template friend struct boost::unordered::iterator_detail::c_iterator; template friend struct boost::unordered::iterator_detail::l_iterator; - template + template friend struct boost::unordered::iterator_detail::cl_iterator; template friend struct boost::unordered::detail::table; @@ -223,7 +223,7 @@ namespace boost { namespace unordered { namespace iterator_detail { } value_type* operator->() const { - return &node_->value(); + return node_->value_ptr(); } iterator& operator++() { @@ -246,7 +246,7 @@ namespace boost { namespace unordered { namespace iterator_detail { } }; - template + template struct c_iterator : public boost::iterator< std::forward_iterator_tag, @@ -268,7 +268,7 @@ namespace boost { namespace unordered { namespace iterator_detail { private: #endif typedef typename Node::node_pointer node_pointer; - typedef boost::unordered::iterator_detail::iterator iterator; + typedef boost::unordered::iterator_detail::iterator n_iterator; node_pointer node_; public: @@ -280,14 +280,14 @@ namespace boost { namespace unordered { namespace iterator_detail { explicit c_iterator(typename Node::link_pointer x) BOOST_NOEXCEPT : node_(static_cast(x)) {} - c_iterator(iterator const& x) BOOST_NOEXCEPT : node_(x.node_) {} + c_iterator(n_iterator const& x) BOOST_NOEXCEPT : node_(x.node_) {} value_type const& operator*() const { return node_->value(); } value_type const* operator->() const { - return &node_->value(); + return node_->value_ptr(); } c_iterator& operator++() { diff --git a/include/boost/unordered/detail/table.hpp b/include/boost/unordered/detail/table.hpp index 2bb5d450..b356ca22 100644 --- a/include/boost/unordered/detail/table.hpp +++ b/include/boost/unordered/detail/table.hpp @@ -191,11 +191,11 @@ namespace boost { namespace unordered { namespace detail { typedef boost::unordered::iterator_detail:: iterator iterator; typedef boost::unordered::iterator_detail:: - c_iterator c_iterator; + c_iterator c_iterator; typedef boost::unordered::iterator_detail:: l_iterator l_iterator; typedef boost::unordered::iterator_detail:: - cl_iterator cl_iterator; + cl_iterator cl_iterator; //////////////////////////////////////////////////////////////////////// // Members @@ -343,7 +343,7 @@ namespace boost { namespace unordered { namespace detail { return policy::new_bucket_count( boost::unordered::detail::double_to_size(floor( static_cast(size) / - static_cast(mlf_))) + 1); + static_cast(mlf_)) + 1)); } //////////////////////////////////////////////////////////////////////// @@ -500,9 +500,11 @@ namespace boost { namespace unordered { namespace detail { op2.commit(); } + // Only call with nodes allocated with the currect allocator, or + // one that is equal to it. (Can't assert because other's + // allocators might have already been moved). void move_buckets_from(table& other) { - BOOST_ASSERT(node_alloc() == other.node_alloc()); BOOST_ASSERT(!buckets_); buckets_ = other.buckets_; bucket_count_ = other.bucket_count_; @@ -710,15 +712,25 @@ namespace boost { namespace unordered { namespace detail { void move_assign(table& x, true_type) { delete_buckets(); + set_hash_functions new_func_this(*this, x); allocators_.move_assign(x.allocators_); - move_assign_no_alloc(x); + // No throw from here. + mlf_ = x.mlf_; + max_load_ = x.max_load_; + move_buckets_from(x); + new_func_this.commit(); } void move_assign(table& x, false_type) { if (node_alloc() == x.node_alloc()) { delete_buckets(); - move_assign_no_alloc(x); + set_hash_functions new_func_this(*this, x); + // No throw from here. + mlf_ = x.mlf_; + max_load_ = x.max_load_; + move_buckets_from(x); + new_func_this.commit(); } else { set_hash_functions new_func_this(*this, x); @@ -743,16 +755,6 @@ namespace boost { namespace unordered { namespace detail { table_impl::fill_buckets(nodes.begin(), *this, node_creator); } } - - void move_assign_no_alloc(table& x) - { - set_hash_functions new_func_this(*this, x); - // No throw from here. - mlf_ = x.mlf_; - max_load_ = x.max_load_; - move_buckets_from(x); - new_func_this.commit(); - } // Accessors diff --git a/test/objects/minimal.hpp b/test/objects/minimal.hpp index af7d0387..85132aa3 100644 --- a/test/objects/minimal.hpp +++ b/test/objects/minimal.hpp @@ -19,6 +19,12 @@ #pragma warning(disable:4100) // unreferenced formal parameter #endif +#if !BOOST_WORKAROUND(BOOST_MSVC, == 1500) +#define BOOST_UNORDERED_CHECK_ADDR_OPERATOR_NOT_USED 1 +#else +#define BOOST_UNORDERED_CHECK_ADDR_OPERATOR_NOT_USED 0 +#endif + namespace test { namespace minimal @@ -29,7 +35,9 @@ namespace minimal class default_assignable; class assignable; - struct ampersand_operator_used {}; + struct ampersand_operator_used { + ampersand_operator_used() { BOOST_TEST(false); } + }; template class hash; template class equal_to; @@ -48,6 +56,7 @@ namespace minimal public: destructible(constructor_param const&) {} ~destructible() {} + void dummy_member() const {} private: destructible(destructible const&); destructible& operator=(destructible const&); @@ -59,6 +68,7 @@ namespace minimal copy_constructible(constructor_param const&) {} copy_constructible(copy_constructible const&) {} ~copy_constructible() {} + void dummy_member() const {} private: copy_constructible& operator=(copy_constructible const&); copy_constructible() {} @@ -78,11 +88,15 @@ namespace minimal { } + void dummy_member() const {} private: copy_constructible_equality_comparable& operator=( copy_constructible_equality_comparable const&); copy_constructible_equality_comparable() {} - ampersand_operator_used operator&() const { return ampersand_operator_used(); } +#if BOOST_UNORDERED_CHECK_ADDR_OPERATOR_NOT_USED + ampersand_operator_used operator&() const { + return ampersand_operator_used(); } +#endif }; bool operator==( @@ -121,9 +135,12 @@ namespace minimal { } - private: + void dummy_member() const {} + +#if BOOST_UNORDERED_CHECK_ADDR_OPERATOR_NOT_USED ampersand_operator_used operator&() const { return ampersand_operator_used(); } +#endif }; class assignable @@ -133,11 +150,13 @@ namespace minimal assignable(assignable const&) {} assignable& operator=(assignable const&) { return *this; } ~assignable() {} - + void dummy_member() const {} private: assignable() {} - // TODO: This messes up a concept check in the tests. - //ampersand_operator_used operator&() const { return ampersand_operator_used(); } +#if BOOST_UNORDERED_CHECK_ADDR_OPERATOR_NOT_USED + ampersand_operator_used operator&() const { + return ampersand_operator_used(); } +#endif }; struct movable_init {}; @@ -153,6 +172,7 @@ namespace minimal movable1(BOOST_RV_REF(movable1)) {} movable1& operator=(BOOST_RV_REF(movable1)) { return *this; } ~movable1() {} + void dummy_member() const {} }; #if !defined(BOOST_NO_CXX11_RVALUE_REFERENCES) @@ -164,6 +184,7 @@ namespace minimal movable2(movable2&&) {} ~movable2() {} movable2& operator=(movable2&&) { return *this; } + void dummy_member() const {} private: movable2() {} movable2(movable2 const&); @@ -184,8 +205,10 @@ namespace minimal ~hash() {} std::size_t operator()(T const&) const { return 0; } - private: - ampersand_operator_used operator&() const { return ampersand_operator_used(); } +#if BOOST_UNORDERED_CHECK_ADDR_OPERATOR_NOT_USED + ampersand_operator_used operator&() const { + return ampersand_operator_used(); } +#endif }; template @@ -199,8 +222,10 @@ namespace minimal ~equal_to() {} bool operator()(T const&, T const&) const { return true; } - private: - ampersand_operator_used operator&() const { return ampersand_operator_used(); } +#if BOOST_UNORDERED_CHECK_ADDR_OPERATOR_NOT_USED + ampersand_operator_used operator&() const { + return ampersand_operator_used(); } +#endif }; template class ptr; @@ -288,9 +313,10 @@ namespace minimal bool operator>(ptr const& x) const { return ptr_ > x.ptr_; } bool operator<=(ptr const& x) const { return ptr_ <= x.ptr_; } bool operator>=(ptr const& x) const { return ptr_ >= x.ptr_; } - private: - // TODO: - //ampersand_operator_used operator&() const { return ampersand_operator_used(); } +#if BOOST_UNORDERED_CHECK_ADDR_OPERATOR_NOT_USED + ampersand_operator_used operator&() const { + return ampersand_operator_used(); } +#endif }; template @@ -325,9 +351,10 @@ namespace minimal bool operator>(const_ptr const& x) const { return ptr_ > x.ptr_; } bool operator<=(const_ptr const& x) const { return ptr_ <= x.ptr_; } bool operator>=(const_ptr const& x) const { return ptr_ >= x.ptr_; } - private: - // TODO: - //ampersand_operator_used operator&() const { return ampersand_operator_used(); } +#if BOOST_UNORDERED_CHECK_ADDR_OPERATOR_NOT_USED + ampersand_operator_used operator&() const { + return ampersand_operator_used(); } +#endif }; template @@ -387,8 +414,10 @@ namespace minimal #else private: allocator& operator=(allocator const&); #endif - private: - ampersand_operator_used operator&() const { return ampersand_operator_used(); } +#if BOOST_UNORDERED_CHECK_ADDR_OPERATOR_NOT_USED + ampersand_operator_used operator&() const { + return ampersand_operator_used(); } +#endif }; template diff --git a/test/unordered/compile_map.cpp b/test/unordered/compile_map.cpp index 317607c7..ebd944ea 100644 --- a/test/unordered/compile_map.cpp +++ b/test/unordered/compile_map.cpp @@ -202,6 +202,7 @@ UNORDERED_AUTO_TEST(test2) unordered_unique_test(map, map_value); unordered_map_test(map, assignable, assignable); unordered_copyable_test(map, assignable, map_value, hash, equal_to); + unordered_map_member_test(map, map_value); boost::unordered_map< test::minimal::assignable, @@ -226,6 +227,7 @@ UNORDERED_AUTO_TEST(test2) unordered_equivalent_test(multimap, map_value); unordered_map_test(multimap, assignable, assignable); unordered_copyable_test(multimap, assignable, map_value, hash, equal_to); + unordered_map_member_test(multimap, map_value); } RUN_TESTS() diff --git a/test/unordered/compile_set.cpp b/test/unordered/compile_set.cpp index f023c797..5c622902 100644 --- a/test/unordered/compile_set.cpp +++ b/test/unordered/compile_set.cpp @@ -183,6 +183,7 @@ UNORDERED_AUTO_TEST(test2) unordered_unique_test(set, assignable); unordered_set_test(set, assignable); unordered_copyable_test(set, assignable, assignable, hash, equal_to); + unordered_set_member_test(set, assignable); std::cout<<"Test unordered_multiset.\n"; @@ -195,6 +196,7 @@ UNORDERED_AUTO_TEST(test2) unordered_equivalent_test(multiset, assignable); unordered_set_test(multiset, assignable); unordered_copyable_test(multiset, assignable, assignable, hash, equal_to); + unordered_set_member_test(multiset, assignable); } UNORDERED_AUTO_TEST(movable1_tests) diff --git a/test/unordered/compile_tests.hpp b/test/unordered/compile_tests.hpp index b48a6efd..38d7dda2 100644 --- a/test/unordered/compile_tests.hpp +++ b/test/unordered/compile_tests.hpp @@ -554,3 +554,23 @@ void unordered_movable_test(X& x, Key& k, T& /* t */, Hash& hf, Pred& eq) sink(a8); sink(a10); } + +template +void unordered_set_member_test(X& x, T& t) +{ + X x1(x); + x1.insert(t); + x1.begin()->dummy_member(); + x1.cbegin()->dummy_member(); +} + +template +void unordered_map_member_test(X& x, T& t) +{ + X x1(x); + x1.insert(t); + x1.begin()->first.dummy_member(); + x1.cbegin()->first.dummy_member(); + x1.begin()->second.dummy_member(); + x1.cbegin()->second.dummy_member(); +}