16

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?

Evan Teran
  • 87,561
  • 32
  • 179
  • 238
  • 3
    http://stackoverflow.com/q/9847860/560648? – Lightness Races in Orbit Aug 26 '15 at 19:15
  • 2
    I prefer the move assignment at the end of this answer: http://stackoverflow.com/a/3279550/4342498 – NathanOliver Aug 26 '15 at 19:25
  • note that you can achieve both swaps at once by using `T& operator=(T rhs) { rhs.swap(*this); return *this; }` – M.M Nov 19 '15 at 09:01
  • 1
    *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. – Walter Nov 19 '15 at 09:02
  • 1
    @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.**" – Evan Teran Nov 19 '15 at 13:47

2 Answers2

6

"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.

Aaron McDaid
  • 26,501
  • 9
  • 66
  • 88
  • 2
    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. – Martin Trenkmann Nov 11 '15 at 18:58
  • 2
    @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". – Aaron McDaid Nov 17 '15 at 16:21
  • 1
    @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. – Aaron McDaid Nov 17 '15 at 16:22
  • @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 improved – Aaron McDaid Nov 17 '15 at 16:23
  • 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. – Martin Trenkmann Nov 18 '15 at 20:09
  • 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. – Aaron McDaid Nov 19 '15 at 08:55
  • 1
    I guess it depends somewhat on the class semantics; for example we would expect `unique_ptr x,y; .... x = std::move(y);` to free the old object in `x` . – M.M Nov 19 '15 at 09:03
  • *"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. – Walter Nov 19 '15 at 09:04
  • 3
    @Walter, that's exactly what I said in my answer. Even the first version of my answer said "(as long as it is destructible)" – Aaron McDaid Nov 19 '15 at 09:37
  • 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 badly – Aaron McDaid Nov 19 '15 at 09:39
  • @AaronMcDaid Yes, you say it, but only later and in brackets, not in the original wrong statement. You should qualify that statement. – Walter Nov 19 '15 at 13:41
1

For me the principle of least astonishment says that the safe bet is to leave the rhs as empty as possible, whatever it means for the given type. In other words: it would be astonishing for me that after

std::shared_ptr<A> x = f(), y = g();
x = std::move(y);

the y would hold any resources. If the move assignment would have been implemented in shared_ptr with a swap then it would be the case. Which would be surprising for most.

So I would argue that swapping satisfies the standard to the letter, but the "downside" is also quite big.

UPDATE: The language does not say anything about the state of the moved from object. But all resource handling classes (containers. smart pointers, thread handles, etc...) in the standard library are specified as being empty after moved from. Which seems to be not coincidental and a general good practice.

Notinlist
  • 16,144
  • 10
  • 57
  • 99
  • Implementing move assignment in terms of `swap` [the canonical way](https://stackoverflow.com/a/3279550/364696) would not leave `y` with meaningful resources. `x = std::move(y);` would 1) Move-construct the argument to the assignment operator (with the move constructor swapping a freshly initialized, and presumably empty instance with the received value), leaving `y` empty, then 2) Invoke the move assignment operator itself, swapping the temporary it receives with the current value of `x`. `x`'s value is left in the temporary, which is destroyed when the move-assignment operator returns. – ShadowRanger May 31 '23 at 22:31
  • @Notinlist, I think an important thing to keep in mind is that in your example, it would be pretty unreasonable to do **anything** with `y` except destruct it or assign it a new value because that's the point of being moved from. In other words, one should only move from an object that will "never" be used again. So while in this example, it would have whatever value `f()` returned... it's also true that it's not appropriate to even test if it did or did not, let alone do anything with that information. – Evan Teran Jun 02 '23 at 05:02
  • 1
    The standard explicitly says that in case of shared pointers the moved from shared pointer will be empty. The main point is: Any other solution would be surprising in case of shared pointers (and in fact many other types of objects). Imagine that `A` in its destructor unregisters itself from some registry. If move assigning from `y` to `x` does not destruct the original object stored in `x` then this unregistration will not happen. Nobody wants to do anyting with `y`, but having the original value of `x` in `y` is horribly risky. Spec of `std::shared_ptr` agrees with me. – Notinlist Jun 05 '23 at 09:02
  • The **language** does not say anything about the state of the moved from object. All resource handling classes (containers. smart pointers, thread handles, etc...) in the **standard library** are specified as being empty after moved from. Which seems to be not coincidental and a general good practice. – Notinlist Jun 05 '23 at 09:06