From 0b720f82b4e8f99d7251d3381cb69e8219176e80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ion=20Gazta=C3=B1aga?= Date: Mon, 21 Apr 2014 13:59:49 +0200 Subject: [PATCH] Fixed #9916: "Allocator propagation incorrect in the assignment operator of most". Fixed #9932: "Missing assignment operator from related static_vector". Added missing details from issue #9915 --- doc/container.qbk | 2 + include/boost/container/deque.hpp | 56 ++--- include/boost/container/detail/tree.hpp | 58 +++--- .../boost/container/detail/version_type.hpp | 6 + include/boost/container/flat_map.hpp | 19 +- include/boost/container/flat_set.hpp | 15 +- include/boost/container/list.hpp | 46 +++-- include/boost/container/map.hpp | 9 +- include/boost/container/set.hpp | 10 +- include/boost/container/slist.hpp | 44 ++-- include/boost/container/stable_vector.hpp | 51 +++-- include/boost/container/static_vector.hpp | 82 ++++---- include/boost/container/string.hpp | 52 ++--- include/boost/container/vector.hpp | 194 ++++++++++-------- test/static_vector_test.cpp | 10 +- test/static_vector_test.hpp | 1 + 16 files changed, 372 insertions(+), 283 deletions(-) diff --git a/doc/container.qbk b/doc/container.qbk index 0ecd987..bd54711 100644 --- a/doc/container.qbk +++ b/doc/container.qbk @@ -964,6 +964,8 @@ use [*Boost.Container]? There are several reasons for that: * [@https://svn.boost.org/trac/boost/ticket/9338 #9338: ['"VS2005 compiler errors in swap() definition after including container/memory_util.hpp"]]. * [@https://svn.boost.org/trac/boost/ticket/9648 #9648: ['"string construction optimization - char_traits::copy could be used ..."]]. * [@https://svn.boost.org/trac/boost/ticket/9915 #9915: ['"Documentation issues regarding vector constructors and resize methods - value/default initialization"]]. + * [@https://svn.boost.org/trac/boost/ticket/9916 #9916: ['"Allocator propagation incorrect in the assignment operator of most"]]. + * [@https://svn.boost.org/trac/boost/ticket/9932 #9932: ['"Missing assignment operator from related static_vector"]]. [endsect] diff --git a/include/boost/container/deque.hpp b/include/boost/container/deque.hpp index 479a9c0..b15ff1d 100644 --- a/include/boost/container/deque.hpp +++ b/include/boost/container/deque.hpp @@ -542,7 +542,7 @@ class deque : protected deque_base //! and inserts n value initialized values. //! //! Throws: If allocator_type's default constructor - //! throws or T's value initialization or copy constructor throws. + //! throws or T's value initialization throws. //! //! Complexity: Linear to n. explicit deque(size_type n) @@ -704,35 +704,39 @@ class deque : protected deque_base return *this; } - //! Effects: Move assignment. All mx's values are transferred to *this. + //! Effects: Move assignment. All x's values are transferred to *this. //! - //! Postcondition: x.empty(). *this contains a the elements x had - //! before the function. + //! Throws: If allocator_traits_type::propagate_on_container_move_assignment + //! is false and (allocation throws or value_type's move constructor throws) //! - //! Throws: If allocator_type's copy constructor throws. - //! - //! Complexity: Linear. + //! Complexity: Constant if allocator_traits_type:: + //! propagate_on_container_move_assignment is true or + //! this->get>allocator() == x.get_allocator(). Linear otherwise. deque& operator= (BOOST_RV_REF(deque) x) + BOOST_CONTAINER_NOEXCEPT_IF(allocator_traits_type::propagate_on_container_move_assignment) { - if (&x != this){ - allocator_type &this_alloc = this->alloc(); - allocator_type &x_alloc = x.alloc(); - //If allocators are equal we can just swap pointers - if(this_alloc == x_alloc){ - //Destroy objects but retain memory in case x reuses it in the future - this->clear(); - this->swap_members(x); - //Move allocator if needed - container_detail::bool_ flag; - container_detail::move_alloc(this_alloc, x_alloc, flag); - container_detail::move_alloc(this->ptr_alloc(), x.ptr_alloc(), flag); - } - //If unequal allocators, then do a one by one move - else{ - this->assign( boost::make_move_iterator(x.begin()) - , boost::make_move_iterator(x.end())); - } + BOOST_ASSERT(this != &x); + allocator_type &this_alloc = this->alloc(); + allocator_type &x_alloc = x.alloc(); + const bool propagate_alloc = allocator_traits_type:: + propagate_on_container_move_assignment::value; + container_detail::bool_ flag; + const bool allocators_equal = this_alloc == x_alloc; (void)allocators_equal; + //Resources can be transferred if both allocators are + //going to be equal after this function (either propagated or already equal) + if(propagate_alloc || allocators_equal){ + //Destroy objects but retain memory in case x reuses it in the future + this->clear(); + //Move allocator if needed + container_detail::move_alloc(this_alloc, x_alloc, flag); + container_detail::move_alloc(this->ptr_alloc(), x.ptr_alloc(), flag); + //Nothrow swap + this->swap_members(x); + } + //Else do a one by one move + else{ + this->assign( boost::make_move_iterator(x.begin()) + , boost::make_move_iterator(x.end())); } return *this; } diff --git a/include/boost/container/detail/tree.hpp b/include/boost/container/detail/tree.hpp index 16db841..55952c8 100644 --- a/include/boost/container/detail/tree.hpp +++ b/include/boost/container/detail/tree.hpp @@ -685,35 +685,39 @@ class tree tree& operator=(BOOST_RV_REF(tree) x) { - if (&x != this){ - NodeAlloc &this_alloc = this->get_stored_allocator(); - const NodeAlloc &x_alloc = x.get_stored_allocator(); - //If allocators are equal we can just swap pointers - if(this_alloc == x_alloc){ - //Destroy and swap pointers - this->clear(); - this->icont() = ::boost::move(x.icont()); - //Move allocator if needed - this->AllocHolder::move_assign_alloc(x); - } - //If unequal allocators, then do a one by one move - else{ - //Transfer all the nodes to a temporary tree - //If anything goes wrong, all the nodes will be destroyed - //automatically - Icont other_tree(::boost::move(this->icont())); + BOOST_ASSERT(this != &x); + NodeAlloc &this_alloc = this->node_alloc(); + NodeAlloc &x_alloc = x.node_alloc(); + const bool propagate_alloc = allocator_traits:: + propagate_on_container_move_assignment::value; + const bool allocators_equal = this_alloc == x_alloc; (void)allocators_equal; + //Resources can be transferred if both allocators are + //going to be equal after this function (either propagated or already equal) + if(propagate_alloc || allocators_equal){ + //Destroy + this->clear(); + //Move allocator if needed + this->AllocHolder::move_assign_alloc(x); + //Obtain resources + this->icont() = boost::move(x.icont()); + } + //Else do a one by one move + else{ + //Transfer all the nodes to a temporary tree + //If anything goes wrong, all the nodes will be destroyed + //automatically + Icont other_tree(::boost::move(this->icont())); - //Now recreate the source tree reusing nodes stored by other_tree - this->icont().clone_from - (x.icont() - , RecyclingCloner(*this, other_tree) - , Destroyer(this->node_alloc())); + //Now recreate the source tree reusing nodes stored by other_tree + this->icont().clone_from + (x.icont() + , RecyclingCloner(*this, other_tree) + , Destroyer(this->node_alloc())); - //If there are remaining nodes, destroy them - NodePtr p; - while((p = other_tree.unlink_leftmost_without_rebalance())){ - AllocHolder::destroy_node(p); - } + //If there are remaining nodes, destroy them + NodePtr p; + while((p = other_tree.unlink_leftmost_without_rebalance())){ + AllocHolder::destroy_node(p); } } return *this; diff --git a/include/boost/container/detail/version_type.hpp b/include/boost/container/detail/version_type.hpp index 66cafc9..548fe3b 100644 --- a/include/boost/container/detail/version_type.hpp +++ b/include/boost/container/detail/version_type.hpp @@ -81,7 +81,13 @@ struct version template struct version : public container_detail::integral_constant::value> +{}; + +template +struct is_version { + static const bool value = + is_same< typename version::type, integral_constant >::value; }; } //namespace container_detail { diff --git a/include/boost/container/flat_map.hpp b/include/boost/container/flat_map.hpp index 0f70e0f..63d2d5e 100644 --- a/include/boost/container/flat_map.hpp +++ b/include/boost/container/flat_map.hpp @@ -135,6 +135,7 @@ class flat_map typedef Key key_type; typedef T mapped_type; typedef std::pair value_type; + typedef ::boost::container::allocator_traits allocator_traits_type; typedef typename boost::container::allocator_traits::pointer pointer; typedef typename boost::container::allocator_traits::const_pointer const_pointer; typedef typename boost::container::allocator_traits::reference reference; @@ -275,11 +276,15 @@ class flat_map //! Effects: Move constructs a flat_map. //! Constructs *this using x's resources. //! - //! Complexity: Construct. + //! Throws: If allocator_traits_type::propagate_on_container_move_assignment + //! is false and (allocation throws or value_type's move constructor throws) //! - //! Postcondition: x is emptied. - flat_map& operator=(BOOST_RV_REF(flat_map) mx) - { m_flat_tree = boost::move(mx.m_flat_tree); return *this; } + //! Complexity: Constant if allocator_traits_type:: + //! propagate_on_container_move_assignment is true or + //! this->get>allocator() == x.get_allocator(). Linear otherwise. + flat_map& operator=(BOOST_RV_REF(flat_map) x) + BOOST_CONTAINER_NOEXCEPT_IF(allocator_traits_type::propagate_on_container_move_assignment) + { m_flat_tree = boost::move(x.m_flat_tree); return *this; } //! Effects: Returns a copy of the Allocator that //! was passed to the object's constructor. @@ -1029,6 +1034,7 @@ class flat_multimap typedef Key key_type; typedef T mapped_type; typedef std::pair value_type; + typedef ::boost::container::allocator_traits allocator_traits_type; typedef typename boost::container::allocator_traits::pointer pointer; typedef typename boost::container::allocator_traits::const_pointer const_pointer; typedef typename boost::container::allocator_traits::reference reference; @@ -1169,8 +1175,9 @@ class flat_multimap //! Effects: this->swap(x.get()). //! //! Complexity: Constant. - flat_multimap& operator=(BOOST_RV_REF(flat_multimap) mx) - { m_flat_tree = boost::move(mx.m_flat_tree); return *this; } + flat_multimap& operator=(BOOST_RV_REF(flat_multimap) x) + BOOST_CONTAINER_NOEXCEPT_IF(allocator_traits_type::propagate_on_container_move_assignment) + { m_flat_tree = boost::move(x.m_flat_tree); return *this; } //! Effects: Returns a copy of the Allocator that //! was passed to the object's constructor. diff --git a/include/boost/container/flat_set.hpp b/include/boost/container/flat_set.hpp index 0a1ece6..4bde4be 100644 --- a/include/boost/container/flat_set.hpp +++ b/include/boost/container/flat_set.hpp @@ -72,6 +72,7 @@ class flat_set typedef Key value_type; typedef Compare key_compare; typedef Compare value_compare; + typedef ::boost::container::allocator_traits allocator_traits_type; typedef typename ::boost::container::allocator_traits::pointer pointer; typedef typename ::boost::container::allocator_traits::const_pointer const_pointer; typedef typename ::boost::container::allocator_traits::reference reference; @@ -181,11 +182,15 @@ class flat_set flat_set& operator=(BOOST_COPY_ASSIGN_REF(flat_set) x) { return static_cast(this->base_t::operator=(static_cast(x))); } - //! Effects: Makes *this a copy of the previous value of mx. + //! Throws: If allocator_traits_type::propagate_on_container_move_assignment + //! is false and (allocation throws or value_type's move constructor throws) //! - //! Complexity: Linear in x.size(). - flat_set& operator=(BOOST_RV_REF(flat_set) mx) - { return static_cast(this->base_t::operator=(boost::move(static_cast(mx)))); } + //! Complexity: Constant if allocator_traits_type:: + //! propagate_on_container_move_assignment is true or + //! this->get>allocator() == x.get_allocator(). Linear otherwise. + flat_set& operator=(BOOST_RV_REF(flat_set) x) + BOOST_CONTAINER_NOEXCEPT_IF(allocator_traits_type::propagate_on_container_move_assignment) + { return static_cast(this->base_t::operator=(boost::move(static_cast(x)))); } #ifdef BOOST_CONTAINER_DOXYGEN_INVOKED //! Effects: Returns a copy of the Allocator that @@ -725,6 +730,7 @@ class flat_multiset typedef Key value_type; typedef Compare key_compare; typedef Compare value_compare; + typedef ::boost::container::allocator_traits allocator_traits_type; typedef typename ::boost::container::allocator_traits::pointer pointer; typedef typename ::boost::container::allocator_traits::const_pointer const_pointer; typedef typename ::boost::container::allocator_traits::reference reference; @@ -804,6 +810,7 @@ class flat_multiset //! @copydoc ::boost::container::flat_set::operator=(flat_set &&) flat_multiset& operator=(BOOST_RV_REF(flat_multiset) mx) + BOOST_CONTAINER_NOEXCEPT_IF(allocator_traits_type::propagate_on_container_move_assignment) { return static_cast(this->base_t::operator=(boost::move(static_cast(mx)))); } #if defined(BOOST_CONTAINER_DOXYGEN_INVOKED) diff --git a/include/boost/container/list.hpp b/include/boost/container/list.hpp index 079526b..4c01481 100644 --- a/include/boost/container/list.hpp +++ b/include/boost/container/list.hpp @@ -332,32 +332,40 @@ class list return *this; } - //! Effects: Move assignment. All mx's values are transferred to *this. + //! Effects: Move assignment. All x's values are transferred to *this. //! //! Postcondition: x.empty(). *this contains a the elements x had //! before the function. //! - //! Throws: If allocator_type's copy constructor throws. + //! Throws: If allocator_traits_type::propagate_on_container_move_assignment + //! is false and (allocation throws or value_type's move constructor throws) //! - //! Complexity: Constant. + //! Complexity: Constant if allocator_traits_type:: + //! propagate_on_container_move_assignment is true or + //! this->get>allocator() == x.get_allocator(). Linear otherwise. list& operator=(BOOST_RV_REF(list) x) + BOOST_CONTAINER_NOEXCEPT_IF(allocator_traits_type::propagate_on_container_move_assignment) { - if (&x != this){ - NodeAlloc &this_alloc = this->node_alloc(); - NodeAlloc &x_alloc = x.node_alloc(); - //If allocators are equal we can just swap pointers - if(this_alloc == x_alloc){ - //Destroy and swap pointers - this->clear(); - this->icont() = boost::move(x.icont()); - //Move allocator if needed - this->AllocHolder::move_assign_alloc(x); - } - //If unequal allocators, then do a one by one move - else{ - this->assign( boost::make_move_iterator(x.begin()) - , boost::make_move_iterator(x.end())); - } + BOOST_ASSERT(this != &x); + NodeAlloc &this_alloc = this->node_alloc(); + NodeAlloc &x_alloc = x.node_alloc(); + const bool propagate_alloc = allocator_traits_type:: + propagate_on_container_move_assignment::value; + const bool allocators_equal = this_alloc == x_alloc; (void)allocators_equal; + //Resources can be transferred if both allocators are + //going to be equal after this function (either propagated or already equal) + if(propagate_alloc || allocators_equal){ + //Destroy + this->clear(); + //Move allocator if needed + this->AllocHolder::move_assign_alloc(x); + //Obtain resources + this->icont() = boost::move(x.icont()); + } + //Else do a one by one move + else{ + this->assign( boost::make_move_iterator(x.begin()) + , boost::make_move_iterator(x.end())); } return *this; } diff --git a/include/boost/container/map.hpp b/include/boost/container/map.hpp index 49a6cec..6412f69 100644 --- a/include/boost/container/map.hpp +++ b/include/boost/container/map.hpp @@ -89,6 +89,7 @@ class map ////////////////////////////////////////////// typedef Key key_type; + typedef ::boost::container::allocator_traits allocator_traits_type; typedef T mapped_type; typedef std::pair value_type; typedef typename boost::container::allocator_traits::pointer pointer; @@ -232,8 +233,14 @@ class map //! Effects: this->swap(x.get()). //! - //! Complexity: Constant. + //! Throws: If allocator_traits_type::propagate_on_container_move_assignment + //! is false and (allocation throws or value_type's move constructor throws) + //! + //! Complexity: Constant if allocator_traits_type:: + //! propagate_on_container_move_assignment is true or + //! this->get>allocator() == x.get_allocator(). Linear otherwise. map& operator=(BOOST_RV_REF(map) x) + BOOST_CONTAINER_NOEXCEPT_IF(allocator_traits_type::propagate_on_container_move_assignment) { return static_cast(this->base_t::operator=(boost::move(static_cast(x)))); } #if defined(BOOST_CONTAINER_DOXYGEN_INVOKED) diff --git a/include/boost/container/set.hpp b/include/boost/container/set.hpp index ffb5d00..acbfae0 100644 --- a/include/boost/container/set.hpp +++ b/include/boost/container/set.hpp @@ -76,6 +76,7 @@ class set typedef Key value_type; typedef Compare key_compare; typedef Compare value_compare; + typedef ::boost::container::allocator_traits allocator_traits_type; typedef typename ::boost::container::allocator_traits::pointer pointer; typedef typename ::boost::container::allocator_traits::const_pointer const_pointer; typedef typename ::boost::container::allocator_traits::reference reference; @@ -184,8 +185,14 @@ class set //! Effects: this->swap(x.get()). //! - //! Complexity: Constant. + //! Throws: If allocator_traits_type::propagate_on_container_move_assignment + //! is false and (allocation throws or value_type's move constructor throws) + //! + //! Complexity: Constant if allocator_traits_type:: + //! propagate_on_container_move_assignment is true or + //! this->get>allocator() == x.get_allocator(). Linear otherwise. set& operator=(BOOST_RV_REF(set) x) + BOOST_CONTAINER_NOEXCEPT_IF(allocator_traits_type::propagate_on_container_move_assignment) { return static_cast(this->base_t::operator=(boost::move(static_cast(x)))); } #if defined(BOOST_CONTAINER_DOXYGEN_INVOKED) @@ -678,6 +685,7 @@ class multiset typedef Key value_type; typedef Compare key_compare; typedef Compare value_compare; + typedef ::boost::container::allocator_traits allocator_traits_type; typedef typename ::boost::container::allocator_traits::pointer pointer; typedef typename ::boost::container::allocator_traits::const_pointer const_pointer; typedef typename ::boost::container::allocator_traits::reference reference; diff --git a/include/boost/container/slist.hpp b/include/boost/container/slist.hpp index 81d8c53..252bc38 100644 --- a/include/boost/container/slist.hpp +++ b/include/boost/container/slist.hpp @@ -358,27 +358,35 @@ class slist //! Postcondition: this->size() == x.size(). *this contains a copy //! of each of x's elements. //! - //! Throws: If memory allocation throws or T's copy constructor throws. + //! Throws: If allocator_traits_type::propagate_on_container_move_assignment + //! is false and (allocation throws or value_type's move constructor throws) //! - //! Complexity: Linear to the number of elements in x. + //! Complexity: Constant if allocator_traits_type:: + //! propagate_on_container_move_assignment is true or + //! this->get>allocator() == x.get_allocator(). Linear otherwise. slist& operator= (BOOST_RV_REF(slist) x) + BOOST_CONTAINER_NOEXCEPT_IF(allocator_traits_type::propagate_on_container_move_assignment) { - if (&x != this){ - NodeAlloc &this_alloc = this->node_alloc(); - NodeAlloc &x_alloc = x.node_alloc(); - //If allocators a re equal we can just swap pointers - if(this_alloc == x_alloc){ - //Destroy and swap pointers - this->clear(); - this->icont() = boost::move(x.icont()); - //Move allocator if needed - this->AllocHolder::move_assign_alloc(x); - } - //If unequal allocators, then do a one by one move - else{ - this->assign( boost::make_move_iterator(x.begin()) - , boost::make_move_iterator(x.end())); - } + BOOST_ASSERT(this != &x); + NodeAlloc &this_alloc = this->node_alloc(); + NodeAlloc &x_alloc = x.node_alloc(); + const bool propagate_alloc = allocator_traits_type:: + propagate_on_container_move_assignment::value; + const bool allocators_equal = this_alloc == x_alloc; (void)allocators_equal; + //Resources can be transferred if both allocators are + //going to be equal after this function (either propagated or already equal) + if(propagate_alloc || allocators_equal){ + //Destroy + this->clear(); + //Move allocator if needed + this->AllocHolder::move_assign_alloc(x); + //Obtain resources + this->icont() = boost::move(x.icont()); + } + //Else do a one by one move + else{ + this->assign( boost::make_move_iterator(x.begin()) + , boost::make_move_iterator(x.end())); } return *this; } diff --git a/include/boost/container/stable_vector.hpp b/include/boost/container/stable_vector.hpp index 950f288..69bfbaa 100644 --- a/include/boost/container/stable_vector.hpp +++ b/include/boost/container/stable_vector.hpp @@ -743,35 +743,42 @@ class stable_vector //! Postcondition: x.empty(). *this contains a the elements x had //! before the function. //! - //! Throws: If allocator_type's copy constructor throws. + //! Throws: If allocator_traits_type::propagate_on_container_move_assignment + //! is false and (allocation throws or T's move constructor throws) //! - //! Complexity: Linear. + //! Complexity: Constant if allocator_traits_type:: + //! propagate_on_container_move_assignment is true or + //! this->get>allocator() == x.get_allocator(). Linear otherwise. stable_vector& operator=(BOOST_RV_REF(stable_vector) x) + BOOST_CONTAINER_NOEXCEPT_IF(allocator_traits_type::propagate_on_container_move_assignment) { - if (&x != this){ - node_allocator_type &this_alloc = this->priv_node_alloc(); - node_allocator_type &x_alloc = x.priv_node_alloc(); - //If allocators are equal we can just swap pointers - if(this_alloc == x_alloc){ - //Destroy objects but retain memory - this->clear(); - this->index = boost::move(x.index); - this->priv_swap_members(x); - //Move allocator if needed - container_detail::bool_ flag; - container_detail::move_alloc(this->priv_node_alloc(), x.priv_node_alloc(), flag); - } - //If unequal allocators, then do a one by one move - else{ - this->assign( boost::make_move_iterator(x.begin()) - , boost::make_move_iterator(x.end())); - } + //for move constructor, no aliasing (&x != this) is assummed. + BOOST_ASSERT(this != &x); + node_allocator_type &this_alloc = this->priv_node_alloc(); + node_allocator_type &x_alloc = x.priv_node_alloc(); + const bool propagate_alloc = allocator_traits_type:: + propagate_on_container_move_assignment::value; + container_detail::bool_ flag; + const bool allocators_equal = this_alloc == x_alloc; (void)allocators_equal; + //Resources can be transferred if both allocators are + //going to be equal after this function (either propagated or already equal) + if(propagate_alloc || allocators_equal){ + //Destroy objects but retain memory in case x reuses it in the future + this->clear(); + //Move allocator if needed + container_detail::move_alloc(this_alloc, x_alloc, flag); + //Take resources + this->index = boost::move(x.index); + this->priv_swap_members(x); + } + //Else do a one by one move + else{ + this->assign( boost::make_move_iterator(x.begin()) + , boost::make_move_iterator(x.end())); } return *this; } - //! Effects: Assigns the n copies of val to *this. //! //! Throws: If memory allocation throws or T's copy constructor throws. diff --git a/include/boost/container/static_vector.hpp b/include/boost/container/static_vector.hpp index 583e7f5..6734163 100644 --- a/include/boost/container/static_vector.hpp +++ b/include/boost/container/static_vector.hpp @@ -2,6 +2,7 @@ // // Copyright (c) 2012-2013 Adam Wulkiewicz, Lodz, Poland. // Copyright (c) 2011-2013 Andrew Hundt. +// Copyright (c) 2013-2014 Ion Gaztanaga // // Use, modification and distribution is subject to the Boost Software License, // Version 1.0. (See accompanying file LICENSE_1_0.txt or copy at @@ -231,45 +232,9 @@ public: //! @par Complexity //! Linear O(N). template - static_vector(static_vector const& other) : base_t(other) {} - - //! @brief Copy assigns Values stored in the other static_vector to this one. - //! - //! @param other The static_vector which content will be copied to this one. - //! - //! @par Throws - //! If Value's copy constructor or copy assignment throws. - //! - //! @par Complexity - //! Linear O(N). - static_vector & operator=(BOOST_COPY_ASSIGN_REF(static_vector) other) - { - base_t::operator=(static_cast(other)); - return *this; - } - - //! @pre other.size() <= capacity() - //! - //! @brief Copy assigns Values stored in the other static_vector to this one. - //! - //! @param other The static_vector which content will be copied to this one. - //! - //! @par Throws - //! If Value's copy constructor or copy assignment throws. - //! - //! @par Complexity - //! Linear O(N). - template -// TEMPORARY WORKAROUND -#if defined(BOOST_NO_RVALUE_REFERENCES) - static_vector & operator=(::boost::rv< static_vector > const& other) -#else - static_vector & operator=(static_vector const& other) -#endif - { - base_t::operator=(static_cast const&>(other)); - return *this; - } + static_vector(static_vector const& other) + : base_t(other) + {} //! @brief Move constructor. Moves Values stored in the other static_vector to this one. //! @@ -302,6 +267,38 @@ public: : base_t(boost::move(static_cast::base_t&>(other))) {} + //! @brief Copy assigns Values stored in the other static_vector to this one. + //! + //! @param other The static_vector which content will be copied to this one. + //! + //! @par Throws + //! If Value's copy constructor or copy assignment throws. + //! + //! @par Complexity + //! Linear O(N). + static_vector & operator=(BOOST_COPY_ASSIGN_REF(static_vector) other) + { + return static_cast(base_t::operator=(static_cast(other))); + } + + //! @pre other.size() <= capacity() + //! + //! @brief Copy assigns Values stored in the other static_vector to this one. + //! + //! @param other The static_vector which content will be copied to this one. + //! + //! @par Throws + //! If Value's copy constructor or copy assignment throws. + //! + //! @par Complexity + //! Linear O(N). + template + static_vector & operator=(static_vector const& other) + { + return static_cast(base_t::operator= + (static_cast::base_t const&>(other))); + } + //! @brief Move assignment. Moves Values stored in the other static_vector to this one. //! //! @param other The static_vector which content will be moved to this one. @@ -314,8 +311,7 @@ public: //! Linear O(N). static_vector & operator=(BOOST_RV_REF(static_vector) other) { - base_t::operator=(boost::move(static_cast(other))); - return *this; + return static_cast(base_t::operator=(boost::move(static_cast(other)))); } //! @pre other.size() <= capacity() @@ -333,8 +329,8 @@ public: template static_vector & operator=(BOOST_RV_REF_BEG static_vector BOOST_RV_REF_END other) { - base_t::operator=(boost::move(static_cast::base_t&>(other))); - return *this; + return static_cast(base_t::operator= + (boost::move(static_cast::base_t&>(other)))); } #ifdef BOOST_CONTAINER_DOXYGEN_INVOKED diff --git a/include/boost/container/string.hpp b/include/boost/container/string.hpp index 67cbc38..9861653 100644 --- a/include/boost/container/string.hpp +++ b/include/boost/container/string.hpp @@ -733,34 +733,38 @@ class basic_string return *this; } - //! Effects: Move constructor. Moves mx's resources to *this. + //! Effects: Move constructor. Moves x's resources to *this. //! //! Throws: If allocator_traits_type::propagate_on_container_move_assignment - //! is true, when allocator_type's move assignment throws. - //! If allocator_traits_type::propagate_on_container_move_assignment - //! is false, when allocator_type's allocation throws. + //! is false and allocation throws //! - //! Complexity: Constant if allocator_traits_type::propagate_on_container_move_assignment. - //! is true, linear otherwise - basic_string& operator=(BOOST_RV_REF(basic_string) x) BOOST_CONTAINER_NOEXCEPT + //! Complexity: Constant if allocator_traits_type:: + //! propagate_on_container_move_assignment is true or + //! this->get>allocator() == x.get_allocator(). Linear otherwise. + basic_string& operator=(BOOST_RV_REF(basic_string) x) + BOOST_CONTAINER_NOEXCEPT_IF(allocator_traits_type::propagate_on_container_move_assignment) { - if (&x != this){ - allocator_type &this_alloc = this->alloc(); - allocator_type &x_alloc = x.alloc(); - //If allocators are equal we can just swap pointers - if(this_alloc == x_alloc){ - //Destroy objects but retain memory in case x reuses it in the future - this->clear(); - this->swap_data(x); - //Move allocator if needed - container_detail::bool_ flag; - container_detail::move_alloc(this_alloc, x_alloc, flag); - } - //If unequal allocators, then do a one by one move - else{ - this->assign( x.begin(), x.end()); - } + //for move constructor, no aliasing (&x != this) is assummed. + BOOST_ASSERT(this != &x); + allocator_type &this_alloc = this->alloc(); + allocator_type &x_alloc = x.alloc(); + const bool propagate_alloc = allocator_traits_type:: + propagate_on_container_move_assignment::value; + container_detail::bool_ flag; + const bool allocators_equal = this_alloc == x_alloc; (void)allocators_equal; + //Resources can be transferred if both allocators are + //going to be equal after this function (either propagated or already equal) + if(propagate_alloc || allocators_equal){ + //Destroy objects but retain memory in case x reuses it in the future + this->clear(); + //Move allocator if needed + container_detail::move_alloc(this_alloc, x_alloc, flag); + //Nothrow swap + this->swap_data(x); + } + //Else do a one by one move + else{ + this->assign( x.begin(), x.end()); } return *this; } diff --git a/include/boost/container/vector.hpp b/include/boost/container/vector.hpp index 2792bff..29590d2 100644 --- a/include/boost/container/vector.hpp +++ b/include/boost/container/vector.hpp @@ -65,7 +65,7 @@ template class vec_iterator { public: - typedef std::random_access_iterator_tag iterator_category; + typedef std::random_access_iterator_tag iterator_category; typedef typename boost::intrusive::pointer_traits::element_type value_type; typedef typename boost::intrusive::pointer_traits::difference_type difference_type; typedef typename if_c @@ -256,10 +256,7 @@ struct vector_value_traits //!This struct deallocates and allocated memory template < class Allocator - , class AllocatorVersion = container_detail::integral_constant - < unsigned - , boost::container::container_detail::version::value - > + , class AllocatorVersion = typename container_detail::version::type > struct vector_alloc_holder : public Allocator @@ -370,8 +367,8 @@ struct vector_alloc_holder void move_from_empty(vector_alloc_holder &x) BOOST_CONTAINER_NOEXCEPT { + //this->m_size was previously initialized this->m_start = x.m_start; - this->m_size = x.m_size; this->m_capacity = x.m_capacity; x.m_start = pointer(); x.m_size = x.m_capacity = 0; @@ -538,9 +535,8 @@ template class vector { #ifndef BOOST_CONTAINER_DOXYGEN_INVOKED - typedef container_detail::integral_constant - ::value > alloc_version; + + typedef typename container_detail::version::type alloc_version; boost::container::container_detail::vector_alloc_holder m_holder; typedef allocator_traits allocator_traits_type; @@ -717,27 +713,30 @@ class vector , x.size(), container_detail::to_raw_pointer(this->m_holder.start())); } - //! Effects: Move constructor. Moves mx's resources to *this. + //! Effects: Move constructor. Moves x's resources to *this. //! //! Throws: Nothing //! //! Complexity: Constant. - vector(BOOST_RV_REF(vector) mx) BOOST_CONTAINER_NOEXCEPT - : m_holder(boost::move(mx.m_holder)) + vector(BOOST_RV_REF(vector) x) BOOST_CONTAINER_NOEXCEPT + : m_holder(boost::move(x.m_holder)) {} #if !defined(BOOST_CONTAINER_DOXYGEN_INVOKED) - //! Effects: Move constructor. Moves mx's resources to *this. + //! Effects: Move constructor. Moves x's resources to *this. //! //! Throws: If T's move constructor or allocation throws //! //! Complexity: Linear. //! - //! Note: Non-standard extension + //! Note: Non-standard extension to support static_vector template - vector(BOOST_RV_REF_BEG vector BOOST_RV_REF_END mx) - : m_holder(boost::move(mx.m_holder)) + vector(BOOST_RV_REF_BEG vector BOOST_RV_REF_END x + , typename container_detail::enable_if_c + < container_detail::is_version::value>::type * = 0 + ) + : m_holder(boost::move(x.m_holder)) {} #endif //!defined(BOOST_CONTAINER_DOXYGEN_INVOKED) @@ -759,25 +758,24 @@ class vector } //! Effects: Move constructor using the specified allocator. - //! Moves mx's resources to *this if a == allocator_type(). + //! Moves x's resources to *this if a == allocator_type(). //! Otherwise copies values from x to *this. //! //! Throws: If allocation or T's copy constructor throws. //! - //! Complexity: Constant if a == mx.get_allocator(), linear otherwise. - vector(BOOST_RV_REF(vector) mx, const allocator_type &a) - : m_holder(a) + //! Complexity: Constant if a == x.get_allocator(), linear otherwise. + vector(BOOST_RV_REF(vector) x, const allocator_type &a) + : m_holder(container_detail::uninitialized_size, a, x.size()) { - if(mx.m_holder.alloc() == a){ - this->m_holder.move_from_empty(mx.m_holder); + if(x.m_holder.alloc() == a){ + this->m_holder.move_from_empty(x.m_holder); } else{ - const size_type n = mx.size(); + const size_type n = x.size(); this->m_holder.first_allocation_same_allocator_type(n); ::boost::container::uninitialized_move_alloc_n_source - ( this->m_holder.alloc(), container_detail::to_raw_pointer(mx.m_holder.start()) + ( this->m_holder.alloc(), container_detail::to_raw_pointer(x.m_holder.start()) , n, container_detail::to_raw_pointer(this->m_holder.start())); - this->m_holder.m_size = n; } } @@ -805,30 +803,32 @@ class vector vector& operator=(BOOST_COPY_ASSIGN_REF(vector) x) { if (&x != this){ - this->priv_copy_assign(x, alloc_version()); + this->priv_copy_assign(x); } return *this; } - //! Effects: Move assignment. All mx's values are transferred to *this. + //! Effects: Move assignment. All x's values are transferred to *this. //! //! Postcondition: x.empty(). *this contains a the elements x had //! before the function. //! - //! Throws: Nothing + //! Throws: If allocator_traits_type::propagate_on_container_move_assignment + //! is false and (allocation throws or value_type's move constructor throws) //! - //! Complexity: Linear. + //! Complexity: Constant if allocator_traits_type:: + //! propagate_on_container_move_assignment is true or + //! this->get>allocator() == x.get_allocator(). Linear otherwise. vector& operator=(BOOST_RV_REF(vector) x) - //iG BOOST_CONTAINER_NOEXCEPT_IF(!allocator_type::propagate_on_container_move_assignment::value || is_nothrow_move_assignable::value);) - BOOST_CONTAINER_NOEXCEPT + BOOST_CONTAINER_NOEXCEPT_IF(allocator_traits_type::propagate_on_container_move_assignment) { - this->priv_move_assign(boost::move(x), alloc_version()); + this->priv_move_assign(boost::move(x)); return *this; } #if !defined(BOOST_CONTAINER_DOXYGEN_INVOKED) - //! Effects: Move assignment. All mx's values are transferred to *this. + //! Effects: Move assignment. All x's values are transferred to *this. //! //! Postcondition: x.empty(). *this contains a the elements x had //! before the function. @@ -836,10 +836,37 @@ class vector //! Throws: If move constructor/assignment of T throws or allocation throws //! //! Complexity: Linear. - template - vector& operator=(BOOST_RV_REF_BEG vector BOOST_RV_REF_END x) + //! + //! Note: Non-standard extension to support static_vector + template + typename container_detail::enable_if_c + < container_detail::is_version::value && + !container_detail::is_same::value + , vector& >::type + operator=(BOOST_RV_REF_BEG vector BOOST_RV_REF_END x) { - this->priv_move_assign(boost::move(x), alloc_version()); + this->priv_move_assign(boost::move(x)); + return *this; + } + + //! Effects: Copy assignment. All x's values are copied to *this. + //! + //! Postcondition: x.empty(). *this contains a the elements x had + //! before the function. + //! + //! Throws: If move constructor/assignment of T throws or allocation throws + //! + //! Complexity: Linear. + //! + //! Note: Non-standard extension to support static_vector + template + typename container_detail::enable_if_c + < container_detail::is_version::value && + !container_detail::is_same::value + , vector& >::type + operator=(const vector &x) + { + this->priv_copy_assign(x); return *this; } @@ -898,7 +925,7 @@ class vector #endif ) { - //For Fwd iterators the standard only requires EmplaceConstructible and assignble from *first + //For Fwd iterators the standard only requires EmplaceConstructible and assignable from *first //so we can't do any backwards allocation const size_type input_sz = static_cast(std::distance(first, last)); const size_type old_capacity = this->capacity(); @@ -1124,7 +1151,7 @@ class vector { this->priv_resize(new_size, value_init); } //! Effects: Inserts or erases elements at the end such that - //! the size becomes n. New elements are value initialized. + //! the size becomes n. New elements are default initialized. //! //! Throws: If memory allocation throws, or T's copy/move or default initialization throws. //! @@ -1413,7 +1440,7 @@ class vector //! Requires: position must be a valid iterator of *this. //! - //! Effects: Insert a new element before position with mx's resources. + //! Effects: Insert a new element before position with x's resources. //! //! Throws: If memory allocation throws. //! @@ -1537,7 +1564,7 @@ class vector //! Throws: Nothing. //! //! Complexity: Constant. - void swap(vector& x) BOOST_CONTAINER_NOEXCEPT_IF((!container_detail::is_same::value)) + void swap(vector& x) BOOST_CONTAINER_NOEXCEPT_IF((!container_detail::is_version::value)) { //Just swap internals in case of !allocator_v0. Otherwise, deep swap this->m_holder.swap(x.m_holder); @@ -1554,9 +1581,13 @@ class vector //! //! Complexity: Linear //! - //! Note: non-standard extension. + //! Note: Non-standard extension to support static_vector template - void swap(vector & x) + void swap(vector & x + , typename container_detail::enable_if_c + < container_detail::is_version::value && + !container_detail::is_same::value >::type * = 0 + ) { this->m_holder.swap(x.m_holder); } #endif //#ifndef BOOST_CONTAINER_DOXYGEN_INVOKED @@ -1631,36 +1662,15 @@ class vector private: - template + template void priv_move_assign(BOOST_RV_REF_BEG vector BOOST_RV_REF_END x - , AllocVersion , typename container_detail::enable_if_c - < container_detail::is_same::value && - !container_detail::is_same::value - >::type * = 0) + < container_detail::is_version::value >::type * = 0) { - if(this->capacity() < x.size()){ + if(!container_detail::is_same::value && + this->capacity() < x.size()){ throw_bad_alloc(); } - this->priv_move_assign_impl(boost::move(x), AllocVersion()); - } - - template - void priv_move_assign(BOOST_RV_REF_BEG vector BOOST_RV_REF_END x - , AllocVersion - , typename container_detail::enable_if_c - < !container_detail::is_same::value || - container_detail::is_same::value - >::type * = 0) - { this->priv_move_assign_impl(boost::move(x), AllocVersion()); } - - template - void priv_move_assign_impl(BOOST_RV_REF_BEG vector BOOST_RV_REF_END x - , AllocVersion - , typename container_detail::enable_if_c - < container_detail::is_same::value - >::type * = 0) - { T* const this_start = container_detail::to_raw_pointer(m_holder.start()); T* const other_start = container_detail::to_raw_pointer(x.m_holder.start()); const size_type this_sz = m_holder.m_size; @@ -1669,40 +1679,46 @@ class vector this->m_holder.m_size = other_sz; } - template - void priv_move_assign_impl(BOOST_RV_REF_BEG vector BOOST_RV_REF_END x - , AllocVersion + template + void priv_move_assign(BOOST_RV_REF_BEG vector BOOST_RV_REF_END x , typename container_detail::enable_if_c - < !container_detail::is_same::value - >::type * = 0) + < !container_detail::is_version::value && + container_detail::is_same::value>::type * = 0) { //for move constructor, no aliasing (&x != this) is assummed. + BOOST_ASSERT(this != &x); allocator_type &this_alloc = this->m_holder.alloc(); allocator_type &x_alloc = x.m_holder.alloc(); - //If allocators are equal we can just swap pointers - if(this_alloc == x_alloc){ + const bool propagate_alloc = allocator_traits_type:: + propagate_on_container_move_assignment::value; + container_detail::bool_ flag; + const bool allocators_equal = this_alloc == x_alloc; (void)allocators_equal; + //Resources can be transferred if both allocators are + //going to be equal after this function (either propagated or already equal) + if(propagate_alloc || allocators_equal){ //Destroy objects but retain memory in case x reuses it in the future this->clear(); - this->m_holder.swap(x.m_holder); //Move allocator if needed - container_detail::bool_ flag; container_detail::move_alloc(this_alloc, x_alloc, flag); + //Nothrow swap + this->m_holder.swap(x.m_holder); } - //If unequal allocators, then do a one by one move + //Else do a one by one move else{ - //TO-DO: optimize this - this->assign( boost::make_move_iterator(container_detail::to_raw_pointer(x.m_holder.start())) - , boost::make_move_iterator(container_detail::to_raw_pointer(x.m_holder.start() + x.m_holder.m_size))); + this->assign( boost::make_move_iterator(x.begin()) + , boost::make_move_iterator(x.end())); } } - template - void priv_copy_assign(const vector &x, AllocVersion + template + void priv_copy_assign(const vector &x , typename container_detail::enable_if_c - < container_detail::is_same::value - >::type * = 0) + < container_detail::is_version::value >::type * = 0) { + if(!container_detail::is_same::value && + this->capacity() < x.size()){ + throw_bad_alloc(); + } T* const this_start = container_detail::to_raw_pointer(m_holder.start()); T* const other_start = container_detail::to_raw_pointer(x.m_holder.start()); const size_type this_sz = m_holder.m_size; @@ -1711,11 +1727,11 @@ class vector this->m_holder.m_size = other_sz; } - template - void priv_copy_assign(const vector &x, AllocVersion + template + void priv_copy_assign(const vector &x , typename container_detail::enable_if_c - < !container_detail::is_same::value - >::type * = 0) + < !container_detail::is_version::value && + container_detail::is_same::value >::type * = 0) { allocator_type &this_alloc = this->m_holder.alloc(); const allocator_type &x_alloc = x.m_holder.alloc(); diff --git a/test/static_vector_test.cpp b/test/static_vector_test.cpp index 4521aef..6cbb315 100644 --- a/test/static_vector_test.cpp +++ b/test/static_vector_test.cpp @@ -486,7 +486,7 @@ void test_swap_and_move_nd() } { static_vector v1, v2, v3; - static_vector s1, s2; + static_vector s1, s2, s3; for (size_t i = 0 ; i < N/2 ; ++i ) { @@ -501,17 +501,19 @@ void test_swap_and_move_nd() } s1.swap(v1); + s3 = v2; s2 = boost::move(v2); - static_vector s3(boost::move(v3)); + static_vector s4(boost::move(v3)); BOOST_TEST(v1.size() == N/3); BOOST_TEST(s1.size() == N/2); //iG moving does not imply emptying source //BOOST_TEST(v2.size() == 0); BOOST_TEST(s2.size() == N/2); + BOOST_TEST(s3.size() == N/2); //iG moving does not imply emptying source //BOOST_TEST(v3.size() == 0); - BOOST_TEST(s3.size() == N/2); + BOOST_TEST(s4.size() == N/2); for (size_t i = 0 ; i < N/3 ; ++i ) BOOST_TEST(v1[i] == T(100 + i)); for (size_t i = 0 ; i < N/2 ; ++i ) @@ -519,6 +521,7 @@ void test_swap_and_move_nd() BOOST_TEST(s1[i] == T(i)); BOOST_TEST(s2[i] == T(i)); BOOST_TEST(s3[i] == T(i)); + BOOST_TEST(s4[i] == T(i)); } } { @@ -528,6 +531,7 @@ void test_swap_and_move_nd() BOOST_TEST_THROWS(s.swap(v), std::bad_alloc); v.resize(N, T(0)); BOOST_TEST_THROWS(s = boost::move(v), std::bad_alloc); + BOOST_TEST_THROWS(s = v, std::bad_alloc); v.resize(N, T(0)); BOOST_TEST_THROWS(small_vector_t s2(boost::move(v)), std::bad_alloc); } diff --git a/test/static_vector_test.hpp b/test/static_vector_test.hpp index f2b3a65..1686955 100644 --- a/test/static_vector_test.hpp +++ b/test/static_vector_test.hpp @@ -13,6 +13,7 @@ #include +#define BOOST_SP_DISABLE_THREADS #include #include "movable_int.hpp"