8

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
  • 80,654
  • 26
  • 169
  • 231
  • 3
    http://stackoverflow.com/q/9847860/560648? – Lightness Races in Orbit Aug 26 '15 at 19:15
  • 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
  • *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

1 Answers1

7

"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
  • 24,484
  • 9
  • 56
  • 82
  • 1
    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
  • @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
  • @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
  • 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
  • @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