diff --git a/doc/intrusive.qbk b/doc/intrusive.qbk index a6df2e0..cb63a77 100644 --- a/doc/intrusive.qbk +++ b/doc/intrusive.qbk @@ -3894,6 +3894,7 @@ to be inserted in intrusive containers are allocated using `std::vector` or `std * [@https://github.com/boostorg/intrusive/pull/57 GitHub #57: ['UB: comparing unrelated pointers]] * [@https://github.com/boostorg/intrusive/issues/59 GitHub #59: ['Add noexcept support to the library]] * [@https://github.com/boostorg/intrusive/issues/60 GitHub #60: ['Licensing question for math.hpp]] + * [@https://github.com/boostorg/intrusive/pull/61 GitHub #61: ['Fix bstree_algorithms<>::swap_nodes]] * [@https://github.com/boostorg/intrusive/issues/63 GitHub #63: ['nop splice removes element]] [endsect] diff --git a/include/boost/intrusive/bstree_algorithms.hpp b/include/boost/intrusive/bstree_algorithms.hpp index b51c5b0..350b2e4 100644 --- a/include/boost/intrusive/bstree_algorithms.hpp +++ b/include/boost/intrusive/bstree_algorithms.hpp @@ -1,6 +1,7 @@ ///////////////////////////////////////////////////////////////////////////// // -// (C) Copyright Ion Gaztanaga 2007-2014 +// (C) Copyright Ion Gaztanaga 2007-2021 +// (C) Copyright Daniel Steck 2021 // // Distributed under the Boost Software License, Version 1.0. // (See accompanying file LICENSE_1_0.txt or copy at @@ -396,38 +397,47 @@ class bstree_algorithms : public bstree_algorithms_base NodeTraits::set_parent(node1, NodeTraits::get_parent(node2)); NodeTraits::set_parent(node2, temp); - //Now adjust adjacent nodes for newly inserted node 1 + //Now adjust child nodes for newly inserted node 1 if((temp = NodeTraits::get_left(node1))){ NodeTraits::set_parent(temp, node1); } if((temp = NodeTraits::get_right(node1))){ NodeTraits::set_parent(temp, node1); } - if((temp = NodeTraits::get_parent(node1)) && - //The header has been already updated so avoid it - temp != header2){ - if(NodeTraits::get_left(temp) == node2){ - NodeTraits::set_left(temp, node1); - } - if(NodeTraits::get_right(temp) == node2){ - NodeTraits::set_right(temp, node1); - } - } - //Now adjust adjacent nodes for newly inserted node 2 + //Now adjust child nodes for newly inserted node 2 if((temp = NodeTraits::get_left(node2))){ NodeTraits::set_parent(temp, node2); } if((temp = NodeTraits::get_right(node2))){ NodeTraits::set_parent(temp, node2); } - if((temp = NodeTraits::get_parent(node2)) && - //The header has been already updated so avoid it - temp != header1){ - if(NodeTraits::get_left(temp) == node1){ - NodeTraits::set_left(temp, node2); + + //Finally adjust parent nodes + if ((temp = NodeTraits::get_parent(node1)) == NodeTraits::get_parent(node2)) { + // special logic for the case where the nodes are siblings + const node_ptr left = NodeTraits::get_left(temp); + NodeTraits::set_left(temp, NodeTraits::get_right(temp)); + NodeTraits::set_right(temp, left); + } else { + if ((temp = NodeTraits::get_parent(node1)) && + //The header has been already updated so avoid it + temp != header2) { + if (NodeTraits::get_left(temp) == node2) { + NodeTraits::set_left(temp, node1); + } + if (NodeTraits::get_right(temp) == node2) { + NodeTraits::set_right(temp, node1); + } } - if(NodeTraits::get_right(temp) == node1){ - NodeTraits::set_right(temp, node2); + if ((temp = NodeTraits::get_parent(node2)) && + //The header has been already updated so avoid it + temp != header1) { + if (NodeTraits::get_left(temp) == node1) { + NodeTraits::set_left(temp, node2); + } + if (NodeTraits::get_right(temp) == node1) { + NodeTraits::set_right(temp, node2); + } } } } diff --git a/include/boost/intrusive/detail/config_begin.hpp b/include/boost/intrusive/detail/config_begin.hpp index 8bd57a3..b261ca9 100644 --- a/include/boost/intrusive/detail/config_begin.hpp +++ b/include/boost/intrusive/detail/config_begin.hpp @@ -17,6 +17,7 @@ #ifdef BOOST_MSVC #pragma warning (push) + #pragma warning (disable : 4619) // there is no warning number 'XXXX' #pragma warning (disable : 4275) // non DLL-interface classkey "identifier" used as base for DLL-interface classkey "identifier" #pragma warning (disable : 4251) // "identifier" : class "type" needs to have dll-interface to be used by clients of class "type2" #pragma warning (disable : 4675) // "method" should be declared "static" and have exactly one parameter diff --git a/test/generic_assoc_test.hpp b/test/generic_assoc_test.hpp index 93d5db7..c12849e 100644 --- a/test/generic_assoc_test.hpp +++ b/test/generic_assoc_test.hpp @@ -1,7 +1,8 @@ ///////////////////////////////////////////////////////////////////////////// // // (C) Copyright Olaf Krzikalla 2004-2006. -// (C) Copyright Ion Gaztanaga 2006-2013. +// (C) Copyright Ion Gaztanaga 2006-2021. +// (C) Copyright Daniel Steck 2021 // // Distributed under the Boost Software License, Version 1.0. // (See accompanying file LICENSE_1_0.txt or copy at @@ -18,6 +19,7 @@ #include #include "test_macros.hpp" #include "test_container.hpp" +#include namespace boost{ namespace intrusive{ @@ -40,6 +42,9 @@ struct test_generic_assoc static void test_root(value_cont_type&); static void test_clone(value_cont_type&); static void test_insert_erase_burst(); + static void test_swap_nodes(); + template + static void test_perfect_binary_tree_of_height_2(value_cont_type &values, Assoc &assoc); static void test_container_from_end(value_cont_type&, detail::true_type); static void test_container_from_end(value_cont_type&, detail::false_type) {} static void test_splay_up(value_cont_type&, detail::true_type); @@ -130,6 +135,226 @@ void test_generic_assoc::test_insert_erase_burst() } } +// Perfect binary tree of height 2 +// 3 | +// / \ | +// 1 5 | +// / \ / \ | +// 0 2 4 6 | +template +template +void test_generic_assoc::test_perfect_binary_tree_of_height_2 + (value_cont_type &values, Assoc &assoc) +{ + //value_cont_type values; + const std::size_t MaxValues = 7; + BOOST_TEST(values.size() == MaxValues); + for(std::size_t i = 0; i != MaxValues; ++i){ + (&values[i])->value_ = i; + } + + typedef typename Assoc::iterator iterator; + + BOOST_TEST( assoc.empty() ); + assoc.clear(); + + const iterator it3 = assoc.insert_before(assoc.end(), values[3]); + const iterator it1 = assoc.insert_before(it3, values[1]); + const iterator it5 = assoc.insert_before(assoc.end(), values[5]); + assoc.insert_before(it1, values[0]); + assoc.insert_before(it3, values[2]); + assoc.insert_before(it5, values[4]); + assoc.insert_before(assoc.end(), values[6]); +} + + +template +void test_generic_assoc::test_swap_nodes() +{ +// Perfect binary tree of height 2 +// 3 | +// / \ | +// 1 5 | +// / \ / \ | +// 0 2 4 6 | + + typedef typename ContainerDefiner::template container + <>::type assoc_type; + typedef typename assoc_type::value_traits value_traits_t; + typedef typename assoc_type::node_algorithms node_algorithms_t; + const std::size_t MaxValues = 7; + + { //Unrelated swap + value_cont_type values(MaxValues); + assoc_type testset; + test_perfect_binary_tree_of_height_2(values, testset); + + node_algorithms_t::swap_nodes + ( value_traits_t::to_node_ptr(values[0]) + , value_traits_t::to_node_ptr(values[4]) + ); + + BOOST_TEST( (&*boost::next(testset.begin(), 0) == &values[4]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 1) == &values[1]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 2) == &values[2]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 3) == &values[3]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 4) == &values[0]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 5) == &values[5]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 6) == &values[6]) ); + + node_algorithms_t::swap_nodes + ( value_traits_t::to_node_ptr(values[4]) + , value_traits_t::to_node_ptr(values[0]) + ); + + BOOST_TEST( (&*boost::next(testset.begin(), 0) == &values[0]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 1) == &values[1]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 2) == &values[2]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 3) == &values[3]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 4) == &values[4]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 5) == &values[5]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 6) == &values[6]) ); + + testset.check(); + } + + { //sibling leaf nodes + value_cont_type values(MaxValues); + assoc_type testset; + test_perfect_binary_tree_of_height_2(values, testset); + + node_algorithms_t::swap_nodes + ( value_traits_t::to_node_ptr(values[0]) + , value_traits_t::to_node_ptr(values[2]) + ); + + BOOST_TEST( (&*boost::next(testset.begin(), 0) == &values[2]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 1) == &values[1]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 2) == &values[0]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 3) == &values[3]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 4) == &values[4]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 5) == &values[5]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 6) == &values[6]) ); + + node_algorithms_t::swap_nodes + ( value_traits_t::to_node_ptr(values[0]) + , value_traits_t::to_node_ptr(values[2]) + ); + + BOOST_TEST( (&*boost::next(testset.begin(), 0) == &values[0]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 1) == &values[1]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 2) == &values[2]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 3) == &values[3]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 4) == &values[4]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 5) == &values[5]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 6) == &values[6]) ); + + testset.check(); + } + + { //sibling nodes + value_cont_type values(MaxValues); + assoc_type testset; + test_perfect_binary_tree_of_height_2(values, testset); + + node_algorithms_t::swap_nodes + ( value_traits_t::to_node_ptr(values[1]) + , value_traits_t::to_node_ptr(values[5]) + ); + + BOOST_TEST( (&*boost::next(testset.begin(), 0) == &values[0]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 1) == &values[5]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 2) == &values[2]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 3) == &values[3]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 4) == &values[4]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 5) == &values[1]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 6) == &values[6]) ); + + node_algorithms_t::swap_nodes + ( value_traits_t::to_node_ptr(values[1]) + , value_traits_t::to_node_ptr(values[5]) + ); + + BOOST_TEST( (&*boost::next(testset.begin(), 0) == &values[0]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 1) == &values[1]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 2) == &values[2]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 3) == &values[3]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 4) == &values[4]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 5) == &values[5]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 6) == &values[6]) ); + + testset.check(); + } + + { //left child + value_cont_type values(MaxValues); + assoc_type testset; + test_perfect_binary_tree_of_height_2(values, testset); + + node_algorithms_t::swap_nodes + ( value_traits_t::to_node_ptr(values[0]) + , value_traits_t::to_node_ptr(values[1]) + ); + + BOOST_TEST( (&*boost::next(testset.begin(), 0) == &values[1]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 1) == &values[0]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 2) == &values[2]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 3) == &values[3]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 4) == &values[4]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 5) == &values[5]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 6) == &values[6]) ); + + node_algorithms_t::swap_nodes + ( value_traits_t::to_node_ptr(values[0]) + , value_traits_t::to_node_ptr(values[1]) + ); + + BOOST_TEST( (&*boost::next(testset.begin(), 0) == &values[0]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 1) == &values[1]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 2) == &values[2]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 3) == &values[3]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 4) == &values[4]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 5) == &values[5]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 6) == &values[6]) ); + + testset.check(); + } + + { //right child + value_cont_type values(MaxValues); + assoc_type testset; + test_perfect_binary_tree_of_height_2(values, testset); + + node_algorithms_t::swap_nodes + ( value_traits_t::to_node_ptr(values[1]) + , value_traits_t::to_node_ptr(values[2]) + ); + + BOOST_TEST( (&*boost::next(testset.begin(), 0) == &values[0]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 1) == &values[2]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 2) == &values[1]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 3) == &values[3]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 4) == &values[4]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 5) == &values[5]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 6) == &values[6]) ); + + node_algorithms_t::swap_nodes + ( value_traits_t::to_node_ptr(values[1]) + , value_traits_t::to_node_ptr(values[2]) + ); + + BOOST_TEST( (&*boost::next(testset.begin(), 0) == &values[0]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 1) == &values[1]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 2) == &values[2]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 3) == &values[3]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 4) == &values[4]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 5) == &values[5]) ); + BOOST_TEST( (&*boost::next(testset.begin(), 6) == &values[6]) ); + + testset.check(); + } +} + template void test_generic_assoc::test_all(value_cont_type& values) { @@ -143,6 +368,7 @@ void test_generic_assoc::test_all(value_cont_type& values) test_rebalance(values, detail::bool_< has_rebalance< assoc_type >::value >()); test_insert_before(values, detail::bool_< has_insert_before< assoc_type >::value >()); test_insert_erase_burst(); + test_swap_nodes(); test_container_from_iterator(values, detail::bool_< assoc_type::has_container_from_iterator >()); }