14

Suppose I have the following code:

class B { /* */ };

class A {
    vector<B*> vb;
public:
    void add(B* b) { vb.push_back(b); }
};


int main() {

    A a;
    B* b(new B());
    a.add(b);
}

Suppose that in this case, all raw pointers B* can be handled through unique_ptr<B>.

Surprisingly, I wasn't able to find how to convert this code using unique_ptr. After a few tries, I came up with the following code, which compiles:

class A {
    vector<unique_ptr<B>> vb;
public:
    void add(unique_ptr<B> b) { vb.push_back(move(b)); }
};


int main() {

    A a;
    unique_ptr<B> b(new B());
    a.add(move(b));
}

So my simple question: is this the way to do it and in particular, is move(b) the only way to do it? (I was thinking of rvalue references but I don't fully understand them.)

And if you have a link with complete explanations of move semantics, unique_ptr, etc. that I was not able to find, don't hesitate to share it.

EDIT According to http://thbecker.net/articles/rvalue_references/section_01.html, my code seems to be OK.

Actually, std::move is just syntactic sugar. With object x of class X, move(x) is just the same as:

static_cast <X&&>(x)

These 2 move functions are needed because casting to a rvalue reference:

  1. prevents function "add" from passing by value
  2. makes push_back use the default move constructor of B

Apparently, I do not need the second std::move in my main() if I change my "add" function to pass by reference (ordinary lvalue ref).

I would like some confirmation of all this, though...

curiousguy
  • 8,038
  • 2
  • 40
  • 58
Bérenger
  • 2,678
  • 2
  • 21
  • 42
  • I think this may be useful for you: http://stackoverflow.com/questions/2876641/so-can-unique-ptr-be-used-safely-in-stl-collections – Kiril Kirov Sep 22 '12 at 20:49
  • @KirilKirov Thanks, the question gave interesting links (see EDIT) – Bérenger Sep 23 '12 at 12:09
  • You could avoid the `std::move` in main if you changed your add that way, but that would be bad, because the caller of `add` would have no idea if they still owned the object or not without looking at the source of `add`. – Puppy Sep 23 '12 at 13:16
  • @DeadMG Argh. I knew I would do something silly. Thanks. BTW I was thinking of passing my parameter by rvalue reference, i.e. "add(B&& b)", in order to suppress the "move" in its implementation, which seems redundant in this case. But doing this (passing by rvalue ref instead of by value), I get the same results and in particular, I can't suppress this "move". Any reason why ? – Bérenger Sep 23 '12 at 18:04
  • Because all variables are lvalues, including those which are references to rvalues. – fredoverflow Jun 22 '14 at 19:16
  • Also very useful: http://stackoverflow.com/questions/8114276/how-do-i-pass-a-unique-ptr-argument-to-a-constructor-or-a-function – kevinarpe May 03 '15 at 12:03

4 Answers4

11

I am somewhat surprised that this is not answered very clearly and explicitly here, nor on any place I easily stumbled upon. While I'm pretty new to this stuff, I think the following can be said.

The situation is a calling function that builds a unique_ptr<T> value (possibly by casting the result from a call to new), and wants to pass it to some function that will take ownership of the object pointed to (by storing it in a data structure for instance, as happens here into a vector). To indicate that ownership has been obtained by the caller, and it is ready to relinquish it, passing a unique_ptr<T> value is in place. Ther are as far as I can see three reasonable modes of passing such a value.

  1. Passing by value, as in add(unique_ptr<B> b) in the question.
  2. Passing by non-const lvalue reference, as in add(unique_ptr<B>& b)
  3. Passing by rvalue reference, as in add(unique_ptr<B>&& b)

Passing by const lvalue reference would not be reasonable, since it does not allow the called function to take ownership (and const rvalue reference would be even more silly than that; I'm not even sure it is allowed).

As far as valid code goes, options 1 and 3 are almost equivalent: they force the caller to write an rvalue as argument to the call, possibly by wrapping a variable in a call to std::move (if the argument is already an rvalue, i.e., unnamed as in a cast from the result of new, this is not necessary). In option 2 however, passing an rvalue (possibly from std::move) is not allowed, and the function must be called with a named unique_ptr<T> variable (when passing a cast from new, one has to assign to a variable first).

When std::move is indeed used, the variable holding the unique_ptr<T> value in the caller is conceptually dereferenced (converted to rvalue, respectively cast to rvalue reference), and ownership is given up at this point. In option 1. the dereferencing is real, and the value is moved to a temporary that is passed to the called function (if the calles function would inspect the variable in the caller, it would find it hold a null pointer already). Ownership has been transferred, and there is no way the caller could decide to not accept it (doing nothing with the argument causes the pointed-to value to be destroyed at function exit; calling the release method on the argument would prevent this, but would just result in a memory leak). Surprisingly, options 2. and 3. are semantically equivalent during the function call, although they require different syntax for the caller. If the called function would pass the argument to another function taking an rvalue (such as the push_back method), std::move must be inserted in both cases, which will transfer ownership at that point. Should the called function forget to do anything with the argument, then the caller will find himself still owning the object if holding a name for it (as is obligatory in option 2); this in spite of that fact that in case 3, since the function prototype asked the caller to agree to the release of ownership (by either calling std::move or supplying a temporary). In summary the methods do

  1. Forces caller to give up ownership, and be sure to actually claim it.
  2. Force caller to possess ownership, and be prepared (by supplying a non const reference) to give it up; however this is not explicit (no call of std::move required or even allowed), nor is taking away ownership assured. I would consider this method rather unclear in its intention, unless it is explicitly intended that taking ownership or not is at discretion of the called function (some use can be imagined, but callers need to be aware)
  3. Forces caller to explicitly indicate giving up ownership, as in 1. (but actual transfer of ownership is delayed until after the moment of function call).

Option 3 is fairly clear in its intention; provided ownership is actually taken, it is for me the best solution. It is slightly more efficient than 1 in that no pointer values are moved to temporaries (the calls to std::move are in fact just casts and cost nothing); this might be especially relevant if the pointer is handed through several intermediate functions before its contents is actually being moved.

Here is some code to experiment with.

class B
{
  unsigned long val;
public:
  B(const unsigned long& x) : val(x)
  { std::cout << "storing " << x << std::endl;}
  ~B() { std::cout << "dropping " << val << std::endl;}
};

typedef std::unique_ptr<B> B_ptr;

class A {
  std::vector<B_ptr> vb;
public:
  void add(B_ptr&& b)
  { vb.push_back(std::move(b)); } // or even better use emplace_back
};


void f() {
    A a;
    B_ptr b(new B(123)),c;
    a.add(std::move(b));
    std::cout << "---" <<std::endl;
    a.add(B_ptr(new B(4567))); // unnamed argument does not need std::move
}

As written, output is

storing 123
---
storing 4567
dropping 123
dropping 4567

Note that values are destroyed in the ordered stored in the vector. Try changing the prototype of the method add (adapting other code if necessary to make it compile), and whether or not it actually passes on its argument b. Several permutations of the lines of output can be obtained.

Marc van Leeuwen
  • 3,605
  • 23
  • 38
9

Yes, this is how it should be done. You are explicitly transferring ownership from main to A. This is basically the same as your previous code, except it's more explicit and vastly more reliable.

Puppy
  • 144,682
  • 38
  • 256
  • 465
  • some extended discussion: [Herb Sutter's GOTW #91](http://herbsutter.com/2013/06/05/gotw-91-solution-smart-pointer-parameters/) and also [Scott Meyers on passing move-only types](http://scottmeyers.blogspot.com/2014/07/should-move-only-types-ever-be-passed.html) specifically "by value" vs. "by r-value reference" – Chris Beck Jan 19 '16 at 05:29
0

So my simple question: is this the way to do it and in particular, is this "move(b)" the only way to do it? (I was thinking of rvalue references but I don't fully understand it so...)

And if you have a link with complete explanations of move semantics, unique_ptr... that I was not able to find, don't hesitate.

Shameless plug, search for the heading "Moving into members". It describes exactly your scenario.

Community
  • 1
  • 1
fredoverflow
  • 256,549
  • 94
  • 388
  • 662
0

Your code in main could be simplified a little, since C++14:

a.add( make_unique<B>() );

where you can put arguments for B's constructor inside the inner parentheses.


You could also consider a class member function that takes ownership of a raw pointer:

void take(B *ptr) { vb.emplace_back(ptr); }

and the corresponding code in main would be:

a.take( new B() );

Another option is to use perfect forwarding for adding vector members:

template<typename... Args>
void emplace(Args&&... args)
{ 
    vb.emplace_back( std::make_unique<B>(std::forward<Args>(args)...) );
}

and the code in main:

a.emplace();

where, as before, you could put constructor arguments for B inside the parentheses.

Link to working example

M.M
  • 138,810
  • 21
  • 208
  • 365