Copy/Move Assignment Confusion

Question

In a move assignment operator, should I be using std::move or std::swap?

I did:

void Swap(Image& Img) {/*Swap everything here*/}

Image& Image::operator = (const Image &Img)
{
    if (this != &Img) Img.Swap(*this); //Compiler error for copy assignment.. no match for swap taking const..
    return *this;
}

Image& Image::operator = (Image&& Img)
{
    Img.Swap(*this);
    return *this;
}

Assume I have two Images I and J and I do:

I = std::move(J);

What happens is that the data from I and J are swapped so now J has the pixels of I and vice-versa. Is this normal? I thought move assignment was supposed to steal and destroy? Such that when J is moved to I, I gets the contents of J and J gets destroyed? But yet I see these examples on the net

I can see the same code working in the move constructor but how can this work in assignment? It doesn't seem to make sense :S

Should std::move be used in move constructor too? If I use std::move in the constructor it crashes my program:

Image::Image(Image&& Img) : Pixels(), Info(), /*Default everything*/
{
    this->Swap(Img);
    Img.Pixels.clear();
}

The above constructor works.. If however, I use std::move in the constructor:

Image::Image(Image&& Img) : Pixels(std::move(Img.Pixels)), Info(std::move(Img.Info)), /*move everything*/
{
    //Do I need to set Img's values to default? Or does std::move do that?
}

That will crash my program when attempting to use the Object that was moved:

Image I = Image("C:/Image.bmp");
Image J = std::move(I);
I.Draw(...); //Crashes program if using std::move. If using the swap version of the constructor, it works. I assume it's because swap version defaults everything.

You seem to assume that after std::move & move constructor call object should be returned to some default state (whatever that might mean, C++ doesn't even have such a notion). What move constructor must do, is to return it to a state in which it is possible to `delete` it or assign a new value to it. Easiest way to achieve that is to swap moved element contents and `this` - moved element is not cleaned. Deleting it will deallocate our old storage - and if reassigned, any internall-y allocated buffers might be reused - thus saving (de)allocation. — j_kubik, Mar 11 '13 at 07:23

score 3 · Accepted Answer · answered Mar 11 '13 at 03:42

3

If you support efficient swapping and move construction, then you should only have a single assignment operator, by value:

Foo & operator=(Foo rhs) { rhs.swap(*this); }

If the user passes a const-reference, then you need to make the copy anyway. If the user passes an rvalue, then construction of the local variable rhs is cheap.

In other words, out of the three: copy/move constructor, copy/move assignment, and swap, you only need to implement two deeply (and one of those two should be the constructor).

answered Mar 11 '13 at 03:42

Kerrek SB

464,522
92
875
1,084

:o Are you serious? That's it? Wait wow.. Why?! How is that assignment operator going to know when someone passes an rvalue if it doesn't have the double Ampersand? – Brandon Mar 11 '13 at 03:47
@CantChooseUsernames: Because we assume that `Foo` has a sensible move constructor. – Kerrek SB Mar 11 '13 at 03:48
What do you mean sensible move constructor? The only two I have is above? Are they sensible? My swap just calls std::swap on all data members. – Brandon Mar 11 '13 at 03:49
@CantChooseUsernames: By "sensible" I mean "provides an advantage over copying". Note also that this by-value+swap assignment also assumes that having a `swap` function is sensible, i.e. provides an advantage. If, for example, your class only contained hundreds of int fields, then swapping wouldn't make sense, and this assignment operator would also be suboptimal. – Kerrek SB Mar 11 '13 at 03:51
In other words, *if* move construction and swapping make sense, then this is a perfectly fine way of writing the assignment operator. – Kerrek SB Mar 11 '13 at 03:51
"out of the three"... I count five. Do you mean "two of the five should be the constructors"? If not, could you edit this answer to elaborate? (I am still trying to wrap my head around C++11 best practices) – Nemo Mar 11 '13 at 03:53
^Me too lol. The poster of this answer is right though. I just tested this and it works but I don't know if there are downsides/differences to this as I've never seen this operator look like his. Everywhere else I look, I see two separate operators defined. Ex.. The second answer of this post does what Kerrek did here and seems to be perfectly fine: http://stackoverflow.com/questions/5976459/how-to-actually-implement-the-rule-of-five/5976829#5976829 – Brandon Mar 11 '13 at 03:55
@Nemo: Yeah, "five" strictly. I'm bunching the copy and move versions together in my count. Sometimes you can even have a move but not a copy (e.g. `thread`), so I didn't want to spell out too much detail... – Kerrek SB Mar 11 '13 at 04:27
That's a nice advice for when you can't follow/must implement the rule of zero :) – Luc Danton Mar 11 '13 at 15:02

Copy/Move Assignment Confusion

1 Answers1