using swap to implement move assignment
Asked Answered
B

1

17

Something occurred to me which I think it completely reasonable, but I'd like people's opinion on it in case I'm just completely missing something. So firstly, my understanding of T& operator=(T&& rhs) is that we don't care what the contents of rhs are when we are done, just that the contents have been moved into this and that rhs is safely destructable.

That being said, a common exception safe implementation of the copy-assignment operator, assuming that swaps are cheap, looks like this:

T& operator=(const T& rhs) {
    if(this != &rhs) {
        T(rhs).swap(*this);
    }
    return *this;
}

So one natural way to implement the move-assignment operator would be like this:

T& operator=(T&& rhs) {
    if(this != &rhs) {
        T(std::move(rhs)).swap(*this);
    }
    return *this;
}

But then it occured to me, rhs doesn't have to be empty! So, why not just do a plain swap?

T& operator=(T&& rhs) {
    rhs.swap(*this); // is this good enough?
    return *this;
}

I think that this satisfies what the move-assignment operator needs to do... but like I said, this just occurred to me, so I assume that it's possible that I'm missing something.

The only "downside" that I can think of is that things owned by this will potentially live longer using the plain swap when compared to the version which does a move-construct/swap.

Thoughts?

Buttery answered 26/8, 2015 at 18:58 Comment(6)
https://mcmap.net/q/746642/-questions-about-the-move-assignment-operator/560648?Hautrhin
I prefer the move assignment at the end of this answer: https://mcmap.net/q/15523/-what-is-the-copy-and-swap-idiomAtiana
note that you can achieve both swaps at once by using T& operator=(T rhs) { rhs.swap(*this); return *this; }Radiophotograph
we don't care what the contents of rhs are when we are done yes we do: it must be in a valid state for the destructor. For example, we must not keep a pointer to be deleted when that pointer was passed to another object.Alejandraalejandrina
@Walter. You should read the whole sentence you quoted. Specifically the part after the comma where I say "...just that the contents have been moved into this and that rhs is safely destructable."Buttery
You never need to guard if(this != &rhs). Both T(std::move(rhs)).swap(*this); and rhs.swap(*this); are safe for self assignment.Samuelson
L
8

"what the move-assignment operator needs to do"

I always find the best way to treat a && variable is to say that "nobody cares what state I leave this variable in". You may move from it. You may copy from it. You may swap into it. There are many more things you can do.

Your code is correct. The && can be left in any state whatsoever (as long as it is destructible).

The object will be destructed sooner or later (probably sooner) and the implementer should be aware of this. This is important if the destructor will delete raw pointers, which would lead to double-deletion if the same raw pointer is in multiple objects.

Lezlielg answered 26/8, 2015 at 19:27 Comment(13)
Sometimes you have to leave rhs in a consistent state. For instance in the move c'tor and assigment of std::unique_ptr you have to set rhs.ptr to nullptr either explicitly or by calling rhs.release() to prevent double deletion.Roveover
@MartinTrenkmann, your comment might seem to (unintentionally) imply that users of unique_ptr would need to call release on it. But that's not true. unique_ptr is implemented to do "the right thing".Lezlielg
@MartinTrenkmann, I think my comment that "The && can be left in any state whatsoever" still stands. But I guess the developer should be aware that the && object will be destructed (quite soon probably) and therefore the developer should take account of the side effects of that destruction.Lezlielg
@MartinTrenkmann, in other words, I'm trying to identify if you believe there is an error in my answer. And if so, how my answer can be improvedLezlielg
The question is about implementing move assignment and especially the state of rhs, so my comment was from an implementers point of view. In this context your statement "nobody cares what state I leave this variable in" is not correct. On the other hand, as a user of someones move c'tor or assigment, you are right, you shouldn't care about rhs, it is just not usable anymore, but this was not the question in my opinion.Roveover
I've added something to the answer, but I'm not entirely satisfied with it. I'm trying to make the point that the && doesn't need to be logically "empty". We just need to be confident that the (inevitable) destructor call won't have any nasty side effects.Lezlielg
I guess it depends somewhat on the class semantics; for example we would expect unique_ptr<T> x,y; .... x = std::move(y); to free the old object in x .Radiophotograph
"nobody cares what state I leave this variable in" wrong. The state must be a valid state for the destructor. For example, you cannot leave a pointer to point to memory to be deleted when you have passed that address into another object.Alejandraalejandrina
@Walter, that's exactly what I said in my answer. Even the first version of my answer said "(as long as it is destructible)"Lezlielg
Everyone, my broad point, on which I'm sure we all agree, is that it doesn't matter (for correctness) whether the moved-from object is "empty" (i.e. destruction does nothing) or whether it is a deep copy of the target object (i.e. destruction still has no undesirable side effects). This is the kind of answer that is needed for the original question. Feel free to post another answer if you think I've made this point badlyLezlielg
@AaronMcDaid Yes, you say it, but only later and in brackets, not in the original wrong statement. You should qualify that statement.Alejandraalejandrina
Another benefit of implementing moves as swaps is that it results in correct order of destruction for the compiler-generated move assignment operator when you have a simple struct of movable types. If you were to instead do any destruction in the move assignment operator, the compiler-generated move assignment operator would then destruct objects in the wrong order.Savagery
@Savagery Which is why several of us recommend operator= take the parameter by value. Then the destructors run in the expected order.Looming

© 2022 - 2024 — McMap. All rights reserved.