7

I was trying to adapt some code and moving the content from a vector to another one using emplace_back()

#include <iostream>
#include <vector>

struct obj
{
  std::string name;

  obj():name("NO_NAME"){}
  obj(const std::string& _name):name(_name){}

  obj(obj&& tmp): name(std::move(tmp.name)) {}
  obj& operator=(obj&& tmp) = default;

};

int main(int argc, char* argv[])
{

  std::vector<obj> v;
  for( int i = 0; i < 1000; ++i )
  {
    v.emplace_back(obj("Jon"));
  }

  std::vector<obj> p;
  for( int i = 0; i < 1000; ++i )
  {
    p.emplace_back(v[i]);
  }

  return(0);
}

This code doesn't compile with g++-4.7, g++-4.6 and clang++: what it's wrong with it ?

I always got 1 main error about

call to implicitly-deleted copy constructor of obj

?

ildjarn
  • 62,044
  • 9
  • 127
  • 211
user1849534
  • 2,329
  • 4
  • 18
  • 20
  • 2
    The error message is telling you that `obj` needs a copy constructor. – Mat Nov 24 '12 at 14:00
  • @mat why it needs to construct ? it's not supposed to use the move constructor + the move assignment operator ? – user1849534 Nov 24 '12 at 14:03
  • Check the definition of [emplace_back](http://en.cppreference.com/w/cpp/container/vector/emplace_back), you need a constructor that takes what you're passing `emplace_back`. You're passing an l-value and don't have a constructor that matches. – Mat Nov 24 '12 at 14:10
  • @Mat i know that, but my focus it's on what v[i] represent, before this i was thinking that it was an Rvalue because it's a reference to an address that contains a type. – user1849534 Nov 24 '12 at 14:13
  • `v[i]` is an l-value, it can be assigned to. – Mat Nov 24 '12 at 14:15

2 Answers2

9

Although the existing answer provides a workaround using std::move that makes your program compile, it must be said that your use of emplace_back seems to be based on a misunderstanding.

The way you describe it ("I was trying to [...] moving the content from a vector to another one using emplace_back()") and the way you use it suggest that you think of emplace_back as a method to move elements into the vector, and of push_back as a method to copy elements into a vector. The code you use to fill the first instance of the vector seems to suggest this as well:

std::vector<obj> v;
for( int i = 0; i < 1000; ++i )
{
  v.emplace_back(obj("Jon"));
}

But this is not what the difference between emplace_back and push_back is about.

Firstly, even push_back will move (not copy) the elements into the vector if only it is given an rvalue, and if the element type has a move assignment operator.

Secondly, the real use case of emplace_back is to construct elements in place, i.e. you use it when you want to put objects into a vector that do not exist yet. The arguments of emplace_back are the arguments to the constructor of the object. So your loop above should really look like this:

std::vector<obj> v;
for( int i = 0; i < 1000; ++i )
{
  v.emplace_back("Jon");   // <-- just pass the string "Jon" , not obj("Jon")
}

The reason why your existing code works is that obj("Jon") is also a valid argument to the constructor (specifically, to the move constructor). But the main idea of emplace_back is that you need not create the object and then move it in. You don't benefit from that idea when you pass obj("Jon") instead of "Jon" to it.

On the other hand, in your second loop you are dealing with objects that were created before. There is no point in using emplace_back to move objects that exist already. And again, emplace_back applied to an existing object does not mean that the object is moved. It only means that it is created in-place, using the ordinary copy constructor (if that exists). If you want to move it, simply use push_back, applied to the result of std::move:

std::vector<obj> p;
for( int i = 0; i < 1000; ++i )
{
  p.push_back(std::move(v[i]));  // <-- Use push_back to move existing elements
}

Further notes
1) You can simplify the loop above using C++11 range-based for:

std::vector<obj> p;
for (auto &&obj : v)
  p.push_back(std::move(obj));

2) Regardless of whether you use an ordinary for-loop or range-based for, you move the elements one by one, which means that the source vector v will remain as a vector of 1000 empty objects. If you actually want to clear the vector in the process (but still use move semantics to transport the elements to the new vector), you can use the move constructor of the vector itself:

std::vector<obj> p(std::move(v));

This reduces the second loop to just a single line, and it makes sure the source vector is cleared.

jogojapan
  • 68,383
  • 11
  • 101
  • 131
  • thanks! a really good explanation, i just have one more thing to explain, the statement `v.emplace_back(obj("Jon"));` in the `for` cycle works because of the scope resolution mechanism that picks the right ( or wrong in my case ) constructor ? – user1849534 Nov 25 '12 at 11:07
  • Oh, it works because `obj("Jon")` is an rvalue (because it is a temporary object). So `emplace_back` calls the constructor that accepts rvalue references, `obj::obj(obj &&tmp)`. If you had first stored the object in a variable (i.e. an lvalue) like this: `obj x = obj("Jon");` and then called `emplace_back(x);` then it would not have worked because `x` is not an rvalue. Instead it would have searched for a constructor `obj(obj &tmp)` or `obj(const obj &tmp)`, but it wouldn't have found any, so I suppose it would have failed. – jogojapan Nov 25 '12 at 11:14
7

The problem is that

p.emplace_back(v[i]);

passes an lvalue to emplace_back, which means that your move constructor (which expects an rvalue reference) won't work.

If you actually want to move values from one container to another, you should explicitly call std::move:

p.emplace_back(std::move(v[i]));

(The idea behind a move constructor like obj(obj&& tmp) is that tmp should be an object that isn't going to be around for much longer. In your first loop, you pass a temporary object to emplace_back, which is fine -- a rvalue reference can bind to a temporary object and steal data from it because the temporary object is about to disappear. In your second loop, the object that you pass to emplace_back has a name: v[i]. That means it's not temporary, and could be referred to later in the program. That's why you have to use std::move to tell the compiler "yes, I really meant to steal data from this object, even though someone else might try to use it later.")


Edit: I'm assuming that your rather unusual usage of emplace_back is a relic of having to craft a little example for us. If that isn't the case, see @jogojapan's answer for a good discussion about why using a std::vector move constructor or repeated calls to push_back would make more sense for your example.

Nate Kohl
  • 35,264
  • 10
  • 43
  • 55
  • I was convinced that v[i] it's an Rvalue, since it's a reference to a position of a well defined type, strange to know that it is an lvalue. but your solution works ... – user1849534 Nov 24 '12 at 14:10
  • @user1849534: I've updated the answer to try to explain it a bit more. The gist of it is that `v[i]` is an lvalue because it's a named object that you could refer to later. – Nate Kohl Nov 24 '12 at 14:18