how to move an std::unique_ptr<> from one STL container to another?

Question

Problem

I have a template container MyContainer<std::unique_ptr<Foo>> which has a std::deque<T> and a std::vector<T> member.

Inside method, send_to_purgatory_if( predicate ), I would like to look at all items in m_taskdq and move items from m_taskdq to m_purgatory, if the predicate evaluates to true.

Issues

I have two issues that I'm struggling with:

my iterator it gets trashed if I remove items from m_taskdq from inside the loop
I am worried about the state of the std::unique_ptr<> if I do the move in two steps (problem lines 1 and 2 - by line 2, I think the std::unique_ptr<> pointed to by it is undefined?)

How should I fix this code?

    template <typename T>
    class MyContainer
    {
      typedef std::function<bool(T&)>  PREDICATE;
    
      void send_to_purgatory_if( PREDICATE p )
      {
// bad code -------------------------------------
        for( auto it=m_taskdq.begin(); it!=m_taskdq.end(); ++it )
        {
          if ( p( *it ) )
          {
            m_purgatory.emplace_back( move( *it ));  // problem line 1
            m_taskdq.erase( it );                    // problem line 2
          }
        }
// end bad code ---------------------------------
      }
    
      std::deque<  T >  m_taskdq;                                                     
      std::vector< T >  m_purgatory;
    };

Why using emplace_back ? Appending an item to a container by moving it is a job for push_back(value_type&&). emplace_back(args&&...) primary point is to forward arguments and construct directly into a container by using an already existing constructor. It's true that emplace_back decays to push_back if only one argument is used but that's really unfortunate IMHO and leads only to confusion. More info here : http://stackoverflow.com/questions/4303513/push-back-vs-emplace-back/4306581#4306581 with my answer and also the comment by lurscher — Arzar, Mar 12 '12 at 10:35
@ThomasPetit Because I am moving an std::unique_ptr<> from one container to another (as described in detail above)? Are you telling me that in my case, I can/should use push_back() instead? — kfmfe04, Mar 12 '12 at 15:01
The next question we need to ask if the erase method sends the task to heaven or hell. — msmith81886, May 05 '15 at 15:41

score 13 · Accepted Answer · answered Mar 12 '12 at 02:54

This is really a C++98 question, with a red-herring concerning move semantics. The first thing to ask is how to do this in C++98:

std::deque::erase(iterator) returns an iterator that refers to the element after the one erased. So get that working first:

 void send_to_purgatory_if( PREDICATE p )
  {
    for( auto it=m_taskdq.begin(); it!=m_taskdq.end();)
    {
      if ( p( *it ) )
      {
        m_purgatory.emplace_back(*it);
        it = m_taskdq.erase(it);
      }
      else
        ++it;
    }
  }

And now it is easy to make it work with C++11 move semantics:

 void send_to_purgatory_if( PREDICATE p )
  {
    for( auto it=m_taskdq.begin(); it!=m_taskdq.end();)
    {
      if ( p( *it ) )
      {
        m_purgatory.emplace_back(std::move(*it));
        it = m_taskdq.erase(it);
      }
      else
        ++it;
    }
  }

The unique_ptr moved from in taskdq becomes a null unique_ptr after the emplace_back, and then it gets erased in the next line. No harm, no foul.

When there is an erase, the return from the erase does a good job at incrementing the iterator. And when there is no erase, a normal iterator increment is in order.

+1 tyvm for a fantastic breakdown of the problem - trying it out now — kfmfe04, Mar 12 '12 at 02:59

how to move an std::unique_ptr<> from one STL container to another?

Problem

Issues

1 Answers1

Linked