improve unit tests and fix some bugs

include/boost/heap/binomial_heap.hpp

@@ -181,7 +181,7 @@ class binomial_heap :
                                        detail::list_iterator_converter< node_type, node_list_type >,
                                        true,
                                        true,
-                                       value_compare >
+                                       typename super_t::internal_compare >
             ordered_iterator;
     };
 #endif
@@ -459,8 +459,13 @@ class binomial_heap :
                 increase( handle );
             else
                 decrease( handle );
-        } else
-            decrease( handle );
+        } else {
+            // Node is a root: it has no parent to compare with.  The heap order below
+            // it may have changed in either direction, so sift it down to restore order,
+            // then re-scan for the new top element.
+            siftdown( this_node );
+            update_top_element();
+        }
     }
 
     /**
@@ -565,13 +570,13 @@ class binomial_heap :
     /// \copydoc boost::heap::fibonacci_heap::ordered_begin
     ordered_iterator ordered_begin( void ) const
     {
-        return ordered_iterator( trees.begin(), trees.end(), top_element, super_t::value_comp() );
+        return ordered_iterator( trees.begin(), trees.end(), top_element, super_t::get_internal_cmp() );
     }
 
     /// \copydoc boost::heap::fibonacci_heap::ordered_end
     ordered_iterator ordered_end( void ) const
     {
-        return ordered_iterator( nullptr, super_t::value_comp() );
+        return ordered_iterator( nullptr, super_t::get_internal_cmp() );
     }
 
     /**

include/boost/heap/detail/heap_comparison.hpp

@@ -154,6 +154,9 @@ struct heap_compare_iteration
         if ( left_size > right_size )
             return false;
 
+        if ( lhs.empty() )
+            return false;
+
         typename Heap1::ordered_iterator it1     = lhs.ordered_begin();
         typename Heap1::ordered_iterator it1_end = lhs.ordered_end();
         typename Heap1::ordered_iterator it2     = rhs.ordered_begin();
@@ -169,7 +172,7 @@ struct heap_compare_iteration
             ++it2;
 
             if ( it1 == it1_end && it2 == it2_end )
-                return true;
+                return false; // all elements compared equal — heaps are equivalent, so lhs < rhs is false
 
             if ( it1 == it1_end || it2 == it2_end )
                 return false;
@@ -190,6 +193,9 @@ struct heap_compare_copy
         if ( left_size > right_size )
             return false;
 
+        if ( lhs.empty() )
+            return false;
+
         Heap1 lhs_copy( lhs );
         Heap2 rhs_copy( rhs );
 

include/boost/heap/detail/mutable_heap.hpp

@@ -145,6 +145,10 @@ class priority_queue_mutable_wrapper
         q_( rhs.q_ ),
         objects( rhs.objects )
     {
+        // q_ was copy-constructed from rhs.q_, which holds iterators into rhs.objects (now
+        // dangling after we copied objects into our own list).  Clear q_ and rebuild it from
+        // our own objects list so that every iterator points into this->objects.
+        q_.clear();
         for ( typename object_list::iterator it = objects.begin(); it != objects.end(); ++it )
             q_.push( it );
     }
@@ -225,25 +229,55 @@ class priority_queue_mutable_wrapper
 
         typedef const_list_iterator                          iterator;
         typedef typename q_type::ordered_iterator_dispatcher ordered_iterator_dispatcher;
+        typedef typename q_type::internal_compare            internal_compare_type;
+
+        // Comparator for unvisited_nodes that respects stability by comparing internal values
+        // (which includes stability counters for stable heaps)
+        struct compare_by_internal_value : public internal_compare_type
+        {
+            const q_type* q;
+
+            compare_by_internal_value( const q_type*                q_ptr = nullptr,
+                                       internal_compare_type const& cmp   = internal_compare_type() ) :
+                internal_compare_type( cmp ),
+                q( q_ptr )
+            {}
+
+            bool operator()( const_list_iterator const& lhs, const_list_iterator const& rhs ) const
+            {
+                if ( q == nullptr )
+                    return false; // arbitrary for null case
+
+                size_type lhs_index = lhs->second;
+                size_type rhs_index = rhs->second;
+
+                typename q_type::internal_type const& lhs_internal
+                    = ordered_iterator_dispatcher::get_internal_value( q, lhs_index );
+                typename q_type::internal_type const& rhs_internal
+                    = ordered_iterator_dispatcher::get_internal_value( q, rhs_index );
+
+                return internal_compare_type::operator()( lhs_internal, rhs_internal );
+            }
+        };
 
         friend class boost::iterator_core_access;
 
     public:
         ordered_iterator( void ) :
             adaptor_type( 0 ),
-            unvisited_nodes( indirect_cmp() ),
+            unvisited_nodes( compare_by_internal_value() ),
             q_( nullptr )
         {}
 
         ordered_iterator( const priority_queue_mutable_wrapper* q, indirect_cmp const& cmp ) :
             adaptor_type( 0 ),
-            unvisited_nodes( cmp ),
+            unvisited_nodes( compare_by_internal_value( &( q->q_ ), q->q_.get_internal_cmp() ) ),
             q_( q )
         {}
 
         ordered_iterator( const_list_iterator it, const priority_queue_mutable_wrapper* q, indirect_cmp const& cmp ) :
             adaptor_type( it ),
-            unvisited_nodes( cmp ),
+            unvisited_nodes( compare_by_internal_value( &( q->q_ ), q->q_.get_internal_cmp() ) ),
             q_( q )
         {
             if ( it != q->objects.end() )
@@ -300,7 +334,7 @@ class priority_queue_mutable_wrapper
 
         std::priority_queue< iterator,
                              std::vector< iterator, typename boost::allocator_rebind< allocator_type, iterator >::type >,
-                             indirect_cmp >
+                             compare_by_internal_value >
                                               unvisited_nodes;
         const priority_queue_mutable_wrapper* q_;
     };

include/boost/heap/detail/tree_iterator.hpp

@@ -88,17 +88,15 @@ struct unordered_tree_iterator_storage
 template < typename ValueType, typename HandleType, typename Alloc, typename ValueCompare, typename ValueExtractor >
 struct ordered_tree_iterator_storage : ValueExtractor
 {
-    struct compare_values_by_handle : ValueExtractor, ValueCompare
+    struct compare_values_by_handle : ValueCompare
     {
         compare_values_by_handle( ValueCompare const& cmp ) :
             ValueCompare( cmp )
         {}
 
         bool operator()( HandleType const& lhs, HandleType const& rhs ) const
         {
-            ValueType const& lhs_value = ValueExtractor::operator()( lhs->value );
-            ValueType const& rhs_value = ValueExtractor::operator()( rhs->value );
-            return ValueCompare::operator()( lhs_value, rhs_value );
+            return ValueCompare::operator()( lhs->value, rhs->value );
         }
     };
 

include/boost/heap/fibonacci_heap.hpp

@@ -176,7 +176,7 @@ class fibonacci_heap :
                                        detail::list_iterator_converter< node, node_list_type >,
                                        true,
                                        true,
-                                       value_compare >
+                                       internal_compare >
             ordered_iterator;
     };
 
@@ -562,7 +562,7 @@ class fibonacci_heap :
      * */
     ordered_iterator ordered_begin( void ) const
     {
-        return ordered_iterator( roots.begin(), roots.end(), top_element, super_t::value_comp() );
+        return ordered_iterator( roots.begin(), roots.end(), top_element, super_t::get_internal_cmp() );
     }
 
     /**
@@ -572,7 +572,7 @@ class fibonacci_heap :
      * */
     ordered_iterator ordered_end( void ) const
     {
-        return ordered_iterator( nullptr, super_t::value_comp() );
+        return ordered_iterator( nullptr, super_t::get_internal_cmp() );
     }
 
     /**

include/boost/heap/pairing_heap.hpp

@@ -184,7 +184,7 @@ class pairing_heap :
                                        detail::pointer_to_reference< node >,
                                        false,
                                        true,
-                                       value_compare >
+                                       typename super_t::internal_compare >
             ordered_iterator;
     };
 
@@ -279,8 +279,7 @@ class pairing_heap :
 
     ~pairing_heap( void )
     {
-        while ( !empty() )
-            pop();
+        clear();
     }
 
     /// \copydoc boost::heap::priority_queue::empty
@@ -430,14 +429,28 @@ class pairing_heap :
      * */
     void update( handle_type handle )
     {
-        node_pointer n = handle.node_;
+        node_pointer n       = handle.node_;
+        bool         is_root = ( n == root );
 
         n->unlink();
-        if ( !n->children.empty() )
-            n = merge_nodes( n, merge_node_list( n->children ) );
 
-        if ( n != root )
-            merge_node( n );
+        if ( !n->children.empty() ) {
+            node_pointer merged_children = merge_node_list( n->children );
+            if ( is_root ) {
+                // Root was removed; its merged children become the new tentative root.
+                // Re-insert n by merging it with the children subtree.
+                root    = merged_children;
+                is_root = false; // n is no longer the root after re-inserting below
+            } else {
+                merge_node( merged_children );
+            }
+        } else if ( is_root ) {
+            // Root had no children; heap is now empty until n is re-inserted.
+            root    = nullptr;
+            is_root = false;
+        }
+
+        merge_node( n );
     }
 
     /**
@@ -549,13 +562,13 @@ class pairing_heap :
     /// \copydoc boost::heap::fibonacci_heap::ordered_begin
     ordered_iterator ordered_begin( void ) const
     {
-        return ordered_iterator( root, super_t::value_comp() );
+        return ordered_iterator( root, super_t::get_internal_cmp() );
     }
 
     /// \copydoc boost::heap::fibonacci_heap::ordered_begin
     ordered_iterator ordered_end( void ) const
     {
-        return ordered_iterator( nullptr, super_t::value_comp() );
+        return ordered_iterator( nullptr, super_t::get_internal_cmp() );
     }
 
 

include/boost/heap/skew_heap.hpp

@@ -86,7 +86,7 @@ struct skew_heap_node : parent_holder< skew_heap_node< value_type, store_parent_
     }
 
     skew_heap_node( value_type&& v ) :
-        value( v )
+        value( std::move( v ) )
     {
         children.fill( 0 );
     }
@@ -305,9 +305,15 @@ class skew_heap :
 
         typedef iterator const_iterator;
 
-        typedef detail::
-            tree_iterator< node, const value_type, allocator_type, value_extractor, detail::dereferencer< node >, true, true, value_compare >
-                ordered_iterator;
+        typedef detail::tree_iterator< node,
+                                       const value_type,
+                                       allocator_type,
+                                       value_extractor,
+                                       detail::dereferencer< node >,
+                                       true,
+                                       true,
+                                       typename super_t::internal_compare >
+            ordered_iterator;
 
         typedef typename detail::extract_allocator_types< typename base_maker::allocator_argument >::reference reference;
         typedef detail::node_handle< node_pointer, super_t, reference > handle_type;
@@ -529,13 +535,13 @@ class skew_heap :
     /// \copydoc boost::heap::fibonacci_heap::ordered_begin
     ordered_iterator ordered_begin( void ) const
     {
-        return ordered_iterator( root, super_t::value_comp() );
+        return ordered_iterator( root, super_t::get_internal_cmp() );
     }
 
     /// \copydoc boost::heap::fibonacci_heap::ordered_begin
     ordered_iterator ordered_end( void ) const
     {
-        return ordered_iterator( 0, super_t::value_comp() );
+        return ordered_iterator( 0, super_t::get_internal_cmp() );
     }
 
     /**

test/CMakeLists.txt

@@ -38,6 +38,7 @@ set(Tests
     priority_queue_test
     skew_heap_test
     mutable_heap_test
+    move_only_types_test
 )
 
 foreach(Test ${Tests})

test/binomial_heap_test.cpp

@@ -81,3 +81,18 @@ BOOST_AUTO_TEST_CASE( binomial_heap_leak_test )
     typedef boost::heap::binomial_heap< boost::shared_ptr< int > > pri_queue;
     run_leak_check_test< pri_queue >();
 }
+
+BOOST_AUTO_TEST_CASE( binomial_heap_edge_cases )
+{
+    typedef boost::heap::binomial_heap< int > pri_queue;
+    run_common_edge_case_tests< pri_queue >();
+    run_ordered_iterator_edge_case_tests< pri_queue >();
+    run_merge_edge_case_tests< pri_queue >();
+    run_mutable_heap_edge_case_tests< pri_queue >();
+}
+
+BOOST_AUTO_TEST_CASE( binomial_heap_stable_edge_cases )
+{
+    typedef boost::heap::binomial_heap< q_tester, boost::heap::stable< true > > stable_queue;
+    run_stable_heap_edge_case_tests< stable_queue >();
+}