From 5f0ce60b394b94407c56f777788d6ab64923d086 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ion=20Gazta=C3=B1aga?= Date: Tue, 30 Apr 2019 18:05:50 +0200 Subject: [PATCH] Fixes #88 ("Implement C++17 MoveAssignable requirements for self-move assignments") --- doc/container.qbk | 6 +-- include/boost/container/deque.hpp | 47 ++++++++-------- include/boost/container/detail/tree.hpp | 65 ++++++++++++----------- include/boost/container/list.hpp | 43 +++++++-------- include/boost/container/slist.hpp | 43 +++++++-------- include/boost/container/stable_vector.hpp | 49 ++++++++--------- include/boost/container/string.hpp | 44 +++++++-------- 7 files changed, 151 insertions(+), 146 deletions(-) diff --git a/doc/container.qbk b/doc/container.qbk index db62dc8..ada8306 100644 --- a/doc/container.qbk +++ b/doc/container.qbk @@ -1267,9 +1267,9 @@ use [*Boost.Container]? There are several reasons for that: [section:release_notes Release Notes] [section:release_notes_boost_1_71_00 Boost 1.71 Release] - - * [@https://github.com/boostorg/container/pull/109 GitHub #109: ['"Get rid of integer overflow in copy_move_algo.hpp (-fsanitize=integer)"]]. - * [@https://github.com/boostorg/container/pull/110 GitHub #110: ['"Avoid gcc 9 deprecated copy warnings in new_allocator.hpp"]]. + * [@https://github.com/boostorg/container/issues/88 GitHub #88: ['"Implement C++17 MoveAssignable requirements for self-move assignments"]]. + * [@https://github.com/boostorg/container/pull/109 GitHub #109: ['"Get rid of integer overflow in copy_move_algo.hpp (-fsanitize=integer)"]]. + * [@https://github.com/boostorg/container/pull/110 GitHub #110: ['"Avoid gcc 9 deprecated copy warnings in new_allocator.hpp"]]. * [@https://github.com/boostorg/container/issues/112 GitHub #112: ['"vector::resize() compilation error with msvc-10..12: data is not a member of boost::detail::aligned_storage"]]. * [@https://github.com/boostorg/container/issues/114 GitHub #114: ['"Fix small_vector noexcept specification"]]. * [@https://github.com/boostorg/container/issues/116 GitHub #116: ['"MSVC + boost 1.70 compilation error when windows.h is already included (detail/thread_mutex.hpp)"]]. diff --git a/include/boost/container/deque.hpp b/include/boost/container/deque.hpp index ec19b80..4ac7115 100644 --- a/include/boost/container/deque.hpp +++ b/include/boost/container/deque.hpp @@ -811,7 +811,7 @@ class deque : protected deque_base::type, //! Complexity: Linear to the number of elements in x. deque& operator= (BOOST_COPY_ASSIGN_REF(deque) x) { - if (&x != this){ + if (BOOST_LIKELY(&x != this)){ allocator_type &this_alloc = this->alloc(); const allocator_type &x_alloc = x.alloc(); dtl::bool_::type, BOOST_NOEXCEPT_IF(allocator_traits_type::propagate_on_container_move_assignment::value || allocator_traits_type::is_always_equal::value) { - 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; - dtl::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 - dtl::move_alloc(this_alloc, x_alloc, flag); - dtl::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())); + if (BOOST_LIKELY(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; + dtl::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 + dtl::move_alloc(this_alloc, x_alloc, flag); + dtl::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 08965e5..42e3564 100644 --- a/include/boost/container/detail/tree.hpp +++ b/include/boost/container/detail/tree.hpp @@ -788,7 +788,7 @@ class tree tree& operator=(BOOST_COPY_ASSIGN_REF(tree) x) { - if (&x != this){ + if (BOOST_LIKELY(this != &x)) { NodeAlloc &this_alloc = this->get_stored_allocator(); const NodeAlloc &x_alloc = x.get_stored_allocator(); dtl::bool_:: @@ -822,39 +822,40 @@ class tree allocator_traits_type::is_always_equal::value) && boost::container::dtl::is_nothrow_move_assignable::value) { - 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())); + if (BOOST_LIKELY(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 - (::boost::move(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 + (::boost::move(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/list.hpp b/include/boost/container/list.hpp index 9195d1b..a17a906 100644 --- a/include/boost/container/list.hpp +++ b/include/boost/container/list.hpp @@ -367,7 +367,7 @@ class list //! Complexity: Linear to the number of elements in x. list& operator=(BOOST_COPY_ASSIGN_REF(list) x) { - if (&x != this){ + if (BOOST_LIKELY(this != &x)) { NodeAlloc &this_alloc = this->node_alloc(); const NodeAlloc &x_alloc = x.node_alloc(); dtl::bool_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())); + if (BOOST_LIKELY(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/slist.hpp b/include/boost/container/slist.hpp index 702f00a..a9c29f8 100644 --- a/include/boost/container/slist.hpp +++ b/include/boost/container/slist.hpp @@ -395,7 +395,7 @@ class slist //! Complexity: Linear to the number of elements in x. slist& operator= (BOOST_COPY_ASSIGN_REF(slist) x) { - if (&x != this){ + if (BOOST_LIKELY(this != &x)) { NodeAlloc &this_alloc = this->node_alloc(); const NodeAlloc &x_alloc = x.node_alloc(); dtl::bool_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())); + if (BOOST_LIKELY(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 25d0d37..87525f9 100644 --- a/include/boost/container/stable_vector.hpp +++ b/include/boost/container/stable_vector.hpp @@ -823,7 +823,7 @@ class stable_vector stable_vector& operator=(BOOST_COPY_ASSIGN_REF(stable_vector) x) { STABLE_VECTOR_CHECK_INVARIANT; - if (&x != this){ + if (BOOST_LIKELY(this != &x)) { node_allocator_type &this_alloc = this->priv_node_alloc(); const node_allocator_type &x_alloc = x.priv_node_alloc(); dtl::bool_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; - dtl::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){ - STABLE_VECTOR_CHECK_INVARIANT - //Destroy objects but retain memory in case x reuses it in the future - this->clear(); - //Move allocator if needed - dtl::move_alloc(this_alloc, x_alloc, flag); - //Take resources - this->index.swap(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())); + if (BOOST_LIKELY(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; + dtl::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){ + STABLE_VECTOR_CHECK_INVARIANT + //Destroy objects but retain memory in case x reuses it in the future + this->clear(); + //Move allocator if needed + dtl::move_alloc(this_alloc, x_alloc, flag); + //Take resources + this->index.swap(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; } diff --git a/include/boost/container/string.hpp b/include/boost/container/string.hpp index 46ce035..0943637 100644 --- a/include/boost/container/string.hpp +++ b/include/boost/container/string.hpp @@ -865,7 +865,7 @@ class basic_string //! Complexity: Linear to the elements x contains. basic_string& operator=(BOOST_COPY_ASSIGN_REF(basic_string) x) { - if (&x != this){ + if (BOOST_LIKELY(this != &x)) { allocator_type &this_alloc = this->alloc(); const allocator_type &x_alloc = x.alloc(); dtl::bool_alloc(); - allocator_type &x_alloc = x.alloc(); - const bool propagate_alloc = allocator_traits_type:: - propagate_on_container_move_assignment::value; - dtl::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 - dtl::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()); + if (BOOST_LIKELY(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; + dtl::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 + dtl::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; }