Issue #48: added std::move for pop() function of stack/queue/pqueue

Fixed stack overflow case in pqueue unit tests

diff --git a/cds/container/basket_queue.h b/cds/container/basket_queue.h
index 53ef4bcff594a339be8bc877c953a71afae0a04a..1e443c12166462a2034d924e65eeea5ca2e46794 100644 (file)
--- a/cds/container/basket_queue.h
+++ b/cds/container/basket_queue.h
@@ -373,12 +373,12 @@ namespace cds { namespace container {
         /// Dequeues a value from the queue
         /**
             If queue is not empty, the function returns \p true, \p dest contains copy of
-            dequeued value. The assignment operator for type \ref value_type is invoked.
+            dequeued value. The assignment operator for \p value_type is invoked.
             If queue is empty, the function returns \p false, \p dest is unchanged.
         */
         bool dequeue( value_type& dest )
         {
-            return dequeue_with( [&dest]( value_type& src ) { dest = src;  } );
+            return dequeue_with( [&dest]( value_type& src ) { dest = std::move( src );});
         }
 
         /// Dequeues a value using a functor

diff --git a/cds/container/fcdeque.h b/cds/container/fcdeque.h
index d14328b60381e31864ef0aedc0dac21058bf26ce..7dccc157cdedf821e3aa36bfb77e26bc17f9b586 100644 (file)
--- a/cds/container/fcdeque.h
+++ b/cds/container/fcdeque.h
@@ -417,7 +417,7 @@ namespace cds { namespace container {
                 assert( pRec->pValPop );
                 pRec->bEmpty = m_Deque.empty();
                 if ( !pRec->bEmpty ) {
-                    *(pRec->pValPop) = m_Deque.front();
+                    *(pRec->pValPop) = std::move( m_Deque.front());
                     m_Deque.pop_front();
                 }
                 break;
@@ -425,7 +425,7 @@ namespace cds { namespace container {
                 assert( pRec->pValPop );
                 pRec->bEmpty = m_Deque.empty();
                 if ( !pRec->bEmpty ) {
-                    *(pRec->pValPop) = m_Deque.back();
+                    *(pRec->pValPop) = std::move( m_Deque.back());
                     m_Deque.pop_back();
                 }
                 break;
@@ -454,20 +454,28 @@ namespace cds { namespace container {
             for ( fc_iterator it = itBegin, itPrev = itEnd; it != itEnd; ++it ) {
                 switch ( it->op() ) {
                 case op_push_front:
+                    if ( itPrev != itEnd
+                        && (itPrev->op() == op_pop_front || (m_Deque.empty() && itPrev->op() == op_pop_back)) )
+                    {
+                        collide( *it, *itPrev );
+                        itPrev = itEnd;
+                    }
+                    else
+                        itPrev = it;
+                    break;
                 case op_push_front_move:
                     if ( itPrev != itEnd
                       && (itPrev->op() == op_pop_front || ( m_Deque.empty() && itPrev->op() == op_pop_back )))
                     {
-                        collide( *it, *itPrev );
+                        collide_move( *it, *itPrev );
                         itPrev = itEnd;
                     }
                     else
                         itPrev = it;
                     break;
                 case op_push_back:
-                case op_push_back_move:
                     if ( itPrev != itEnd
-                        && (itPrev->op() == op_pop_back || ( m_Deque.empty() && itPrev->op() == op_pop_front )))
+                        && (itPrev->op() == op_pop_back || (m_Deque.empty() && itPrev->op() == op_pop_front)) )
                     {
                         collide( *it, *itPrev );
                         itPrev = itEnd;
@@ -475,24 +483,84 @@ namespace cds { namespace container {
                     else
                         itPrev = it;
                     break;
-                case op_pop_front:
+                case op_push_back_move:
                     if ( itPrev != itEnd
-                        && ( itPrev->op() == op_push_front || itPrev->op() == op_push_front_move
-                          || ( m_Deque.empty() && ( itPrev->op() == op_push_back || itPrev->op() == op_push_back_move ))))
+                        && (itPrev->op() == op_pop_back || ( m_Deque.empty() && itPrev->op() == op_pop_front )))
                     {
-                        collide( *itPrev, *it );
+                        collide_move( *it, *itPrev );
                         itPrev = itEnd;
                     }
                     else
                         itPrev = it;
                     break;
+                case op_pop_front:
+                    if ( itPrev != itEnd ) {
+                        if ( m_Deque.empty() ) {
+                            switch ( itPrev->op() ) {
+                            case op_push_back:
+                                collide( *itPrev, *it );
+                                itPrev = itEnd;
+                                break;
+                            case op_push_back_move:
+                                collide_move( *itPrev, *it );
+                                itPrev = itEnd;
+                                break;
+                            default:
+                                itPrev = it;
+                                break;
+                            }
+                        }
+                        else {
+                            switch ( itPrev->op() ) {
+                            case op_push_front:
+                                collide( *itPrev, *it );
+                                itPrev = itEnd;
+                                break;
+                            case op_push_front_move:
+                                collide_move( *itPrev, *it );
+                                itPrev = itEnd;
+                                break;
+                            default:
+                                itPrev = it;
+                                break;
+                            }
+                        }
+                    }
+                    else
+                        itPrev = it;
+                    break;
                 case op_pop_back:
-                    if ( itPrev != itEnd
-                        && ( itPrev->op() == op_push_back || itPrev->op() == op_push_back_move
-                        || ( m_Deque.empty() && ( itPrev->op() == op_push_front || itPrev->op() == op_push_front_move ))))
-                    {
-                        collide( *itPrev, *it );
-                        itPrev = itEnd;
+                    if ( itPrev != itEnd ) {
+                        if ( m_Deque.empty() ) {
+                            switch ( itPrev->op() ) {
+                            case op_push_front:
+                                collide( *itPrev, *it );
+                                itPrev = itEnd;
+                                break;
+                            case op_push_front_move:
+                                collide_move( *itPrev, *it );
+                                itPrev = itEnd;
+                                break;
+                            default:
+                                itPrev = it;
+                                break;
+                            }
+                        }
+                        else {
+                            switch ( itPrev->op() ) {
+                            case op_push_back:
+                                collide( *itPrev, *it );
+                                itPrev = itEnd;
+                                break;
+                            case op_push_back_move:
+                                collide_move( *itPrev, *it );
+                                itPrev = itEnd;
+                                break;
+                            default:
+                                itPrev = it;
+                                break;
+                            }
+                        }
                     }
                     else
                         itPrev = it;
@@ -513,6 +581,15 @@ namespace cds { namespace container {
             m_FlatCombining.operation_done( recPop );
             m_FlatCombining.internal_statistics().onCollide();
         }
+
+        void collide_move( fc_record& recPush, fc_record& recPop )
+        {
+            *(recPop.pValPop) = std::move( *(recPush.pValPush));
+            recPop.bEmpty = false;
+            m_FlatCombining.operation_done( recPush );
+            m_FlatCombining.operation_done( recPop );
+            m_FlatCombining.internal_statistics().onCollide();
+        }
         //@endcond
     };
 

diff --git a/cds/container/fcpriority_queue.h b/cds/container/fcpriority_queue.h
index b8c38cdab0b96da8d286454a9869c010a624f12e..779849b5fd73c438af7c0d7a084304707f36ed3a 100644 (file)
--- a/cds/container/fcpriority_queue.h
+++ b/cds/container/fcpriority_queue.h
@@ -303,7 +303,7 @@ namespace cds { namespace container {
                 assert( pRec->pValPop );
                 pRec->bEmpty = m_PQueue.empty();
                 if ( !pRec->bEmpty ) {
-                    *(pRec->pValPop) = m_PQueue.top();
+                    *(pRec->pValPop) = std::move( m_PQueue.top());
                     m_PQueue.pop();
                 }
                 break;

diff --git a/cds/container/fcqueue.h b/cds/container/fcqueue.h
index 8c58a45d18755646c0bf0a94bdbe04fc76c5e9d9..dff95edea937173e8b044efd083dffcfdbf567ad 100644 (file)
--- a/cds/container/fcqueue.h
+++ b/cds/container/fcqueue.h
@@ -343,7 +343,7 @@ namespace cds { namespace container {
                 assert( pRec->pValDeq );
                 pRec->bEmpty = m_Queue.empty();
                 if ( !pRec->bEmpty ) {
-                    *(pRec->pValDeq) = m_Queue.front();
+                    *(pRec->pValDeq) = std::move( m_Queue.front());
                     m_Queue.pop();
                 }
                 break;

diff --git a/cds/container/fcstack.h b/cds/container/fcstack.h
index 5fb2428100a9f4e8dced2bbea2ff42512662af1f..eb7c8de513e99abe743e272f2142065c0d3b7a6b 100644 (file)
--- a/cds/container/fcstack.h
+++ b/cds/container/fcstack.h
@@ -321,7 +321,7 @@ namespace cds { namespace container {
                 assert( pRec->pValPop );
                 pRec->bEmpty = m_Stack.empty();
                 if ( !pRec->bEmpty ) {
-                    *(pRec->pValPop) = m_Stack.top();
+                    *(pRec->pValPop) = std::move( m_Stack.top());
                     m_Stack.pop();
                 }
                 break;

diff --git a/cds/container/moir_queue.h b/cds/container/moir_queue.h
index 7e9037277e04a43e6a718161593d016318e8aaf4..40acf9c66cc005c1b03118a4c3ebdd6dbc5c01d9 100644 (file)
--- a/cds/container/moir_queue.h
+++ b/cds/container/moir_queue.h
@@ -221,7 +221,7 @@ namespace cds { namespace container {
         */
         bool dequeue( value_type& dest )
         {
-            return dequeue_with( [&dest]( value_type& src ) { dest = src;  } );
+            return dequeue_with( [&dest]( value_type& src ) { dest = std::move( src ); });
         }
 
         /// Dequeues a value using a functor

diff --git a/cds/container/mspriority_queue.h b/cds/container/mspriority_queue.h
index b1321b5f7d48772ff71ea8fd627bb270ff62b9de..4b916f042d128e2891734c6253e4b2a43aebb9b2 100644 (file)
--- a/cds/container/mspriority_queue.h
+++ b/cds/container/mspriority_queue.h
@@ -251,7 +251,7 @@ namespace cds { namespace container {
         */
         bool pop( value_type& dest )
         {
-            return pop_with( [&dest]( value_type& src ) { move_policy()(dest, src); } );
+            return pop_with( [&dest]( value_type& src ) { move_policy()(dest, std::move(src)); });
         }
 
         /// Extracts an item with high priority

diff --git a/cds/container/msqueue.h b/cds/container/msqueue.h
index f3d558f198aeb7e41da94c0909804b0e8d4ef3fe..a26dff20289b240a120ffb10f1367c7f8a6e603f 100644 (file)
--- a/cds/container/msqueue.h
+++ b/cds/container/msqueue.h
@@ -336,7 +336,7 @@ namespace cds { namespace container {
         */
         bool dequeue( value_type& dest )
         {
-            return dequeue_with( [&dest]( value_type& src ) { dest = src;  } );
+            return dequeue_with( [&dest]( value_type& src ) { dest = std::move( src );});
         }
 
         /// Dequeues a value using a functor

diff --git a/cds/container/optimistic_queue.h b/cds/container/optimistic_queue.h
index b118dce2aa3b167eebc39f1b99b1e22db5ffd2f0..0bb01cb4e24364255095e7f9b0a7dc3f23249ccb 100644 (file)
--- a/cds/container/optimistic_queue.h
+++ b/cds/container/optimistic_queue.h
@@ -339,7 +339,7 @@ namespace cds { namespace container {
         */
         bool dequeue( value_type& dest )
         {
-            return dequeue_with( [&dest]( value_type& src ) { dest = src; } );
+            return dequeue_with( [&dest]( value_type& src ) { dest = std::move( src ); });
         }
 
         /// Dequeues a value using a functor

diff --git a/cds/container/rwqueue.h b/cds/container/rwqueue.h
index ae52b9f0927debfb7e379acf3d36ba7248a9e7aa..de0c8e0cdcc7a66dbe4426119d5b0769d431b021 100644 (file)
--- a/cds/container/rwqueue.h
+++ b/cds/container/rwqueue.h
@@ -306,7 +306,7 @@ namespace cds { namespace container {
         */
         bool dequeue( value_type& dest )
         {
-            return dequeue_with( [&dest]( value_type& src ) { dest = src; } );
+            return dequeue_with( [&dest]( value_type& src ) { dest = std::move( src ); });
         }
 
         /// Dequeues a value using a functor

diff --git a/cds/container/segmented_queue.h b/cds/container/segmented_queue.h
index d29d567a1360fb605df8ef4a52bd07c92fd15d4f..fa981e2636b7d1a5ec4cc4b400ecacd511c9eac7 100644 (file)
--- a/cds/container/segmented_queue.h
+++ b/cds/container/segmented_queue.h
@@ -337,7 +337,7 @@ namespace cds { namespace container {
         */
         bool dequeue( value_type& dest )
         {
-            return dequeue_with( [&dest]( value_type& src ) { dest = src; });
+            return dequeue_with( [&dest]( value_type& src ) { dest = std::move( src );});
         }
 
         /// Dequeues a value using a functor

diff --git a/cds/container/treiber_stack.h b/cds/container/treiber_stack.h
index ae9cb1f50c78dff3c28fb680f5b279be0ee50b71..6c7f07026c3c4ebd5378d4d284d147fcf7676d46 100644 (file)
--- a/cds/container/treiber_stack.h
+++ b/cds/container/treiber_stack.h
@@ -347,25 +347,17 @@ namespace cds { namespace container {
         */
         bool pop( value_type& val )
         {
-            return pop_with( [&val]( value_type& src ) { val = src; } );
+            return pop_with( [&val]( value_type& src ) { val = std::move(src); } );
         }
 
         /// Pops an item from the stack with functor
         /**
+            \p Func can be used to copy/move popped item from the stack.
             \p Func interface is:
             \code
             void func( value_type& src );
             \endcond
             where \p src - item popped.
-
-            The \p %pop_with can be used to move item from the stack to user-provided storage:
-            \code
-            cds::container::TreiberStack<cds::gc::HP, std::string > myStack;
-            //...
-
-            std::string dest;
-            myStack.pop_with( [&dest]( std::string& src ) { dest = std::move( src ); } );
-            \endcode
         */
         template <typename Func>
         bool pop_with( Func f )

diff --git a/cds/container/tsigas_cycle_queue.h b/cds/container/tsigas_cycle_queue.h
index c4661898a6e8a55bad57e25a0f1ba6ee5373848e..f87da37c74a2a5729107de574d6f6b329f112f00 100644 (file)
--- a/cds/container/tsigas_cycle_queue.h
+++ b/cds/container/tsigas_cycle_queue.h
@@ -362,7 +362,7 @@ namespace cds { namespace container {
         */
         bool dequeue( value_type& dest )
         {
-            return dequeue_with( [&dest]( value_type& src ) { dest = src; } );
+            return dequeue_with( [&dest]( value_type& src ) { dest = std::move( src );});
         }
 
         /// Synonym for \p dequeue() function

diff --git a/cds/container/vyukov_mpmc_cycle_queue.h b/cds/container/vyukov_mpmc_cycle_queue.h
index 29a126fa6a5a41f1bae81998485a372c87c2397d..783a912c81a582867e599743c38514a4f9881dc4 100644 (file)
--- a/cds/container/vyukov_mpmc_cycle_queue.h
+++ b/cds/container/vyukov_mpmc_cycle_queue.h
@@ -380,13 +380,13 @@ namespace cds { namespace container {
 
         /// Dequeues a value from the queue
         /**
-            If queue is not empty, the function returns \p true, \p dest contains copy of
+            If queue is not empty, the function returns \p true, \p dest contains a copy of
             dequeued value. The assignment operator for type \ref value_type is invoked.
             If queue is empty, the function returns \p false, \p dest is unchanged.
         */
         bool dequeue(value_type & dest )
         {
-            return dequeue_with( [&dest]( value_type& src ){ dest = src; } );
+            return dequeue_with( [&dest]( value_type& src ){ dest = std::move( src );});
         }
 
         /// Synonym for \p dequeue()

diff --git a/cds/opt/options.h b/cds/opt/options.h
index e78655bb02cfc2e57536cd8555ba1834cd2ef965..2b1c3ec6d5299b7d5d6b6134003bb50acba54219 100644 (file)
--- a/cds/opt/options.h
+++ b/cds/opt/options.h
@@ -851,14 +851,14 @@ namespace cds { namespace opt {
             }
         };
 
-        /// \p opt::move_policy based on assignment operator
+        /// \p opt::move_policy based on move-assignment operator
         struct assignment_move_policy
         {
-            /// <tt> dest = src </tt>
+            /// <tt> dest = std::move( src ) </tt>
             template <typename T>
-            void operator()( T& dest, T const& src ) const
+            void operator()( T& dest, T&& src ) const
             {
-                dest = src;
+                dest = std::move( src );
             }
         };
 

diff --git a/tests/test-hdr/priority_queue/hdr_intrusive_pqueue.h b/tests/test-hdr/priority_queue/hdr_intrusive_pqueue.h
index 3a917dc191955c657c55b55e547184b84fc4bf71..e038c8f908e76407d7a08f33400ae6572d69aeb0 100644 (file)
--- a/tests/test-hdr/priority_queue/hdr_intrusive_pqueue.h
+++ b/tests/test-hdr/priority_queue/hdr_intrusive_pqueue.h
@@ -201,8 +201,8 @@ namespace priority_queue {
         template <class PQueue>
         void test_msq_stat()
         {
-            PQueue pq( 0 );   // argument should be ignored for static buffer
-            test_bounded_with( pq );
+            std::unique_ptr< PQueue > pq( new PQueue(0)); // argument should be ignored for static buffer
+            test_bounded_with( *pq );
         }
         template <class PQueue>
         void test_msq_dyn()

diff --git a/tests/test-hdr/priority_queue/hdr_pqueue.h b/tests/test-hdr/priority_queue/hdr_pqueue.h
index 12eb4bb772a82fc3863c6ad874fd61116e83ed95..a9ef86c0b8a7f6cdadd532ecc2fed03bb99fdb57 100644 (file)
--- a/tests/test-hdr/priority_queue/hdr_pqueue.h
+++ b/tests/test-hdr/priority_queue/hdr_pqueue.h
@@ -258,8 +258,8 @@ namespace priority_queue {
         template <class PQueue>
         void test_msq_stat()
         {
-            PQueue pq( 0 );   // argument should be ignored for static buffer
-            test_bounded_with( pq );
+            std::unique_ptr< PQueue > pq( new PQueue( 0 )); // argument should be ignored for static buffer
+            test_bounded_with( *pq );
         }
         template <class PQueue>
         void test_msq_dyn()