std::move inside move assignment operator

Question

I read in another question that when implementing a move constructor it is good practice to std::move each member in the initializer list because if the member happens to be another object then that objects move constructor will be called. Like so...

//Move constructor
Car::Car(Car && obj)
    : 
    prBufferLength(std::move(obj.prBufferLength)), 
    prBuffer(std::move(obj.prBuffer)) 
{
    obj.prBuffer = nullptr;
    obj.prBufferLength = 0;
}

However in all the sample move assignment operators I've seen, there has been no mention of using std::move for the same reasons. If the member is an object then should std::move be used? Like so...

//Move assignment
Car Car::operator=(Car && obj)  
{
    delete[] prBuffer;

    prBufferLength = std::move(obj.prBufferLength);
    prBuffer = std::move(obj.prBuffer);

    obj.prBuffer = nullptr;
    obj.prBufferLength = 0;
    return *this;
}

UPDATE:

I appreciate there is no need to use std::move in the example I have chosen (poorly) however I'm interested in if the members were objects.

I'm certainly interested in some clarification on this point as well. — goji, Nov 15 '12 at 20:40
`I read in another question`, could you link that question? In your example there is no reason to use `std::move`. — Jesse Good, Nov 15 '12 at 20:45
Not exactly related to the question, but I strongly recommend that you use `std::vector` instead of manually allocating an array. — Zyx 2000, Nov 15 '12 at 21:18

score 3 · Accepted Answer · answered Nov 15 '12 at 21:05

3

After reading the linked question, I can see the advice in the second most-upvoted answer is to use std::move in the initializer list for the move constructor because no matter if it is a primitive type or not, it will do the right thing. I somewhat disagree with that and think you should only call std::move where appropriate, but this is were personal preferences come in.

Also, for your move assignment operator, the way you have it is fine although I think the unnecessary call to std::move should be removed personally. Another option is to use std::swap which will do the right thing for you.

Car Car::operator=(Car && obj)  
{
    std::swap(this->prBufferLength, obj.prBufferLength);
    std::swap(this->prBuffer, obj.prBuffer); 
    return *this;
}

The difference between the above move assignment operator and your move assignment operator is that the deallocation of memory is delayed while your version deallocates the memory right away, this might be important in some situations.

answered Nov 15 '12 at 21:05

Jesse Good

50,901
14
124
166

1

`swap` will _not_ do the right thing here -- the moved-from object is potentially very heavy here, which violates the principle of least surprise. – ildjarn Nov 16 '12 at 01:08
@ildjarn: Interesting point. However, since it is stated that a moved from object has a "valid but unspecified state", I don't think it violates the principle of least surprise. – Jesse Good Nov 16 '12 at 01:41
I would certainly find it surprising if instantiations of standard container objects did not become empty when moved from, as a matter of standard practice. I.e., unlike usually, I'm not making this statement from the perspective of the standard, but from the perspective of established practice (meh). – ildjarn Nov 16 '12 at 01:57
The requirement for moved-from objects to be in a valid but unspecified state only applies to standard library types and is only the minimum requirement, user-defined types are free to define different guarantees if they wish. Some stdlib types provide stricter guarantees e.g. a moved-from `std::shared_ptr` is guaranteed to be empty – Jonathan Wakely Aug 26 '13 at 10:42

score 1 · Answer 2 · answered Nov 15 '12 at 20:43

1

It looks like prBuffer is a pointer and prBufferLength is some kind of integral type, so move isn't going to make any difference in this particular case as they are both fundamental types.

If prBuffer was a std::string for example, then you should use move to force the use of a move constructor or move assignment operator.

answered Nov 15 '12 at 20:43

Alex Korban

14,916
5
44
55

Yes sorry, I should have mentioned in the question that I was currently using fundamental types, thanks for clarification – TomP89 Nov 15 '12 at 20:45

Josh Heitzman · Answer 3 · 2012-11-15T21:15:39.257

Like a lot of things in C++ and life there isn't a definitive yes or no answer to the question.

That said, as a general rule if incoming member will be cleared / reset / emptied and assigning the incoming member to the destination member will result in an assignment operator being called, then you will want to use std::move so that the move assignment operator will be called instead of the copy assignment operator.

If an assignment operator will not be called (i.e. just a shallow copy will be done) then using std::move is not necessary.

score 1 · Answer 4 · edited May 23 '17 at 11:59

If your member objects would benefit from moving, you should certainly move them. Another strategy, which was demonstrated in the answer you linked to, is swapping. Swapping is like moving, but it's moving in both directions. If your class manages a resource, this has the effect of the object that was passed in (the rvalue) recieving the no longer wanted data. That data is then destroyed by that object's destructor. For example, your move assignment operator could be written like this:

Car Car::operator=(Car && obj)  
{
    // don't need this, it will be handled by obj's destructor
    // delete[] prBuffer; 

    using std::swap;
    swap(prBuffer, obj.prBuffer);
    swap(prBufferLength, obj.prBufferLength);

    return *this;
}

Also take a look at the Copy-and-Swap idiom. Which allows you to use the same assignment operator for both move and copy, but has the slight drawback that self-assignment results in an unnecessary copy.

Actually, deferring to the object's destructor as you suggested is not a very good idea. If `obj` happens to be an lvalue which is explicitly being moved from, it will continue to exist after the call. This can lead to subtle problems, e.g. if `prBuffer` points to objects which have exclusive access to some resources. — Alex Korban, Nov 16 '12 at 00:17

score 0 · Answer 5 · answered Nov 15 '12 at 21:00

0

I believe a (the?) better way to implement move assignment is to use the move constructor to create a new temporary object, and then swap it with the current object, just as with copy assignment.

It not only avoids code duplication but also prevents you from making mistakes accidentally by e.g. forgetting to move a member.

answered Nov 15 '12 at 21:00

user541686

205,094
128
528
886

Copy and Swap is the way to implement the copy constructor. The move constructor should simply swap, no temporary needed. – Mooing Duck Nov 15 '12 at 21:29
@MooingDuck: Great point, I should've thought twice before saying that! – user541686 Nov 15 '12 at 21:33
Erm, @MooingDuck, how do you use copy&anything to implement copy construction? What does the copy? I think you mean copy&swap is a good way to implement copy _assignment_ which is what Mehrdad said in the first place, along with talking about move assignment not a move constructor. – Jonathan Wakely Nov 15 '12 at 23:53
@JonathanWakely: Correct: I did mean copy assignment rather than copy constructor. Good catch. Incorrect: Mehrdad said copy and swap was the better way to implement a _move_ assignment, which is just silly. – Mooing Duck Nov 16 '12 at 00:06
1

@MooingDuck: Why is that silly? Copy-and-swap is a great way to implement both the move-assignment operator and the copy-assignment operator in one. – Benjamin Lindley Nov 16 '12 at 03:15
@MooingDuck: I think I said ***move** and swap* should be used to implement a *move* assignment, *not* "*copy* and swap" as you just wrote. But I'm guessing that was just a typo on your part? – user541686 Nov 16 '12 at 03:38
1

@BenjaminLindley: I'm not sure if MooingDuck understood what I was saying correctly or not, but either way, I never meant to say that you're supposed to make a *copy* inside a move assignment operator. Rather, I was saying you should *move*-construct the object into a temporary in the move assignment operator, then swap it with the current object. Which is still silly, because there is no need for the local temporary at all -- I could just swap with the R-value reference, which is a temporary itself. That said, this will *only* work assuming you've *not* implemented `swap` in terms of `move`. – user541686 Nov 16 '12 at 03:40
1

@Mehrdad: What I'm saying is that the copy-and-swap idiom works for both the copy-assignment operator and the move-assignment operator. It just happens that when called with an rvalue, it doesn't actually copy, but it is still the copy-and-swap idiom. e.g. http://ideone.com/u1W7Fm – Benjamin Lindley Nov 16 '12 at 03:56
1

@BenjaminLindley: `X& operator=(X other)` is not a move-assignment operator; it merely inhibits (and therefore defeats the purpose of) the move assignment operator. – user541686 Nov 16 '12 at 04:15
Simply swapping with the rvalue leaves the rvalue with your old state, sometimes that's not acceptable (e.g. a smart pointer probably wants to release its target as soon as it's assigned to, not when the rvalue is destroyed). And `X& operator=(X)` doesn't defeat the purpose of a move assignment operator if you have a move constructor and swap, it performs an equivalent job _fulfilling_ the purpose of the move assignment i.e. can be just as efficient – Jonathan Wakely Nov 16 '12 at 09:19
@JonathanWakely: Ah, so then it's better to make a local temporary like I said originally in my answer, right? – user541686 Nov 16 '12 at 10:19
1

Sometimes that's better (e.g. types like smart pointers which have postconditions saying that they are empty after a move), sometimes it's not a problem to move your old state into the rvalue (if the type's state is simply "unspecified" after a move). As long as you know which you're doing and if it's appropriate they're both valid implementations – Jonathan Wakely Nov 16 '12 at 11:45

std::move inside move assignment operator

5 Answers5

Linked