Passing a container of unique_ptr to constructor?

Question

What am I missing here? Why can't I move a vector as part of class constructor? Removing const from the constructor doesn't help either.

#include <iostream>
#include <vector>
#include <memory>

using namespace std;

class Bar
{
public:
  Bar(const vector<unique_ptr<char>> vec);
  vector<unique_ptr<char>> vec_;
};

Bar::Bar(const vector<unique_ptr<char>> vec) :
  vec_(move(vec)) //not ok
{
}

int main()
{
  vector<unique_ptr<char>> vec;
  vec.push_back(unique_ptr<char>(new char('a')));
  vec.push_back(unique_ptr<char>(new char('b')));
  vec.push_back(unique_ptr<char>(new char('c')));
  vector<unique_ptr<char>> vec1 (move(vec)); //ok
  Bar bar(vec1);
  return 0;
}

It's not clear from your question what problem you are having; if the code fails to compile then post the compiler error messages — M.M, Dec 01 '14 at 00:17

score 4 · Accepted Answer · edited May 23 '17 at 12:13

The following should compile fine:

#include <iostream>
#include <vector>
#include <memory>

using namespace std;

class Bar
{
public:
  Bar(vector<unique_ptr<char>> vec);
  vector<unique_ptr<char>> vec_;
};

Bar::Bar(vector<unique_ptr<char>> vec) : // If you intend to move something,
                                         // do not make it const, as moving
                                         // from it will in most cases change
                                         // its state (and therefore cannot be
                                         // const-qualified).
  vec_(move(vec))
{
}

int main()
{
  vector<unique_ptr<char>> vec;
  vec.push_back(unique_ptr<char>(new char('a')));
  vec.push_back(unique_ptr<char>(new char('b')));
  vec.push_back(unique_ptr<char>(new char('c')));
  vector<unique_ptr<char>> vec1 (move(vec));
  Bar bar(std::move(vec1)); // Just like the line immediately above,
                            // the explicit `move` is required, otherwise
                            // you are requesting a copy, which is an error.
  return 0;
}

I have left the rest of your code unchanged, but you may want to read Why is “using namespace std;” considered bad practice?

score 4 · Answer 2 · answered Nov 30 '14 at 23:21

This code works fine for me:

#include <iostream>
#include <memory>
#include <vector>
using namespace std;

struct Bar
{
    Bar(vector<unique_ptr<char>> vec)
        : Vec(move(vec))
    { }

    vector<unique_ptr<char>> Vec;
};

int main()
{
    vector<unique_ptr<char>> vec;
    vec.push_back(unique_ptr<char>(new char('a')));
    vec.push_back(unique_ptr<char>(new char('b')));
    vec.push_back(unique_ptr<char>(new char('c')));

    vector<unique_ptr<char>> vec1(move(vec));

    Bar bar(move(vec1));
}

Note that I removed the const in the constructor parameter, and I also added an explicit move when constructing bar in main().

In fact, you had this code in main():

vector<unique_ptr<char>> vec1 (move(vec)); //ok
Bar bar(vec1);

But that code required a copy of the vec1 vector when constructing bar. Since vec1 is defined as a vector of unique_ptrs, and since unique_ptr is movable but not copyable, the compiler issued an error: it is not possible to copy the vec1 vector.

Instead, you must be explicit in calling std::move() on the vec1 argument if you want to trigger a move of the vector argument.

score 2 · Answer 3 · answered Dec 01 '14 at 00:27

This constructor (with or without const) accepts a vector by value:

Bar::Bar(const vector<unique_ptr<char>> vec)

Pass-by-value means that a vector vec is constructed by copying the argument passed. However, a unique_ptr is not copyable. Therefore, this function can only be called with an xvalue (that is, a temporary vector that is about to be destroyed anyway, or a vector which you have explicitly permitted to be moved out of by calling std::move or otherwise).

So to call this constructor:

Bar bar( vec1 );               // FAIL: not allowed to take a copy of vec1
Bar bar( std::move(vec1) );    // OK: move pointers out of vec1 in order to create bar

You could avoid this problem by having Bar::Bar take the vector by non-const reference. However this would lead to unintuitive behaviour if that constructor moved elements out of its argument, so the way you have it is perhaps best.

The second part of the problem is what Bar::Bar does once you have actually managed to call it. In this version:

Bar::Bar(const vector<unique_ptr<char>> vec) : vec_(move(vec))

it fails because you cannot move out of a const vector. To fix this, take out the word const which was serving no purpose anyway. (The first part of the problem is unaffected by whether the parameter is const or not).

Passing a container of unique_ptr to constructor?

3 Answers3