Does it make sense to use the move-and-swap idiom on a movable and non-copyable class
Asked Answered
N

3

8

If I have a class such as

class Foo{
public:
    Foo(){...}
    Foo(Foo && rhs){...}
    operator=(Foo rhs){ swap(*this, rhs);}
    void swap(Foo &rhs);
private:
    Foo(const Foo&);
// snip: swap code
};
void swap(Foo& lhs, Foo& rhs);

Does it make sense to implement operator= by value and swap if I don't have a copy constructor? It should prevent copying my objects of class Foo but allow moves.

This class is not copyable so I shouldn't be able to copy construct or copy assign it.

Edit

I've tested my code with this and it seems to have the behaviour I want.

#include <utility>
#include <cstdlib>
using std::swap;
using std::move;
class Foo{
public: Foo():a(rand()),b(rand()) {}
        Foo(Foo && rhs):a(rhs.a), b(rhs.b){rhs.a=rhs.b=-1;}
        Foo& operator=(Foo rhs){swap(*this,rhs);return *this;}
        friend void swap(Foo& lhs, Foo& rhs){swap(lhs.a,rhs.a);swap(lhs.b,rhs.b);}
private:
    //My compiler doesn't yet implement deleted constructor
    Foo(const Foo&);
private:
    int a, b;
};

Foo make_foo()
{
    //This is potentially much more complicated
    return Foo();
}

int main(int, char*[])
{
    Foo f1;
    Foo f2 = make_foo(); //move-construct
    f1 = make_foo(); //move-assign
    f2 = move(f1);
    Foo f3(move(f2));
    f2 = f3; // fails, can't copy-assign, this is wanted
    Foo f4(f3); // fails can't copy-construct

    return 0;
}
Notional answered 26/7, 2011 at 0:35 Comment(6)
It doesn't hurt, I suppose, if you already have the swap code. You can't accidentally copy by assigning, because assignment takes the argument by value, which will only work for moves in your case. Also, say Foo(const Foo&) = delete;!Scholarship
"Move-and-swap", surely.Leal
@Kerrek SB Unfortunately my current compiler at work does not support = default nor =delete. See connect.microsoft.com/VisualStudio/feedback/details/679447/…Notional
This won't even compile! fix operator= and swapInsult
It's a terrible thing to do. No copy ctor -> no copy assignment.Insult
do the right thing and provide a move assignment Foo& operator=(Foo&&) insteadInsult
B
6

Move-and-swap is indeed reasonable. If you disable the copy constructor, then the only way that you can invoke this function is if you were to construct the argument with the move constructor. This means that if you write

lhs = rhs; // Assume rhs is an rvalue

Then the constructor of the argument to operator = will be initialized with the move constructor, emptying rhs and setting the argument to the old value of rhs. The call to swap then exchanges lhs's old value and rhs's old value, leaving lhs holding rhs's old value. Finally, the destructor for the argument fires, cleaning up lhs's old memory. As a note, this really isn't copy-and-swap as much as move-and-swap.

That said, what you have now isn't correct. The default implementation of std::swap will internally try to use the move constructor to move the elements around, which results in an infinite recursion loop. You'd have to overload std::swap to get this to work correctly.

You can see this online here at ideone.

For more information, see this question and its discussion of the "rule of four-and-a-half."

Hope this helps!

Burgoo answered 26/7, 2011 at 0:52 Comment(15)
I think you should salvage my program, apply your fix, and post it here as an example. That would make for a really good answer.Leal
@Tomalak Geret'kal- Great idea. Fixed.Burgoo
+1. :) I still don't fully understand where the copy-requirement vanishes to. You instantiate a std::move, but doesn't that still get copied into the function argument? Barring optimisations? [edit: hmm, ok, the move constructor is being used for that...]Leal
@Tomalak Geret'kal- Yeah, I'm still getting used to this too. :-) I try to remember that pass-by-value now means one of two things - pass-by-copy or pass-by-move.Burgoo
It's all so disgusting :( I particularly hate that std::move doesn't move anything; it should be called std::get_rvalue or something. They've tried to use a keyword that describes the typical use case (rather than the actual meaning), which isn't in the spirit of C++ at all.Leal
@template: You shouldn't specialize std::swap, but rather just provide a friend function to be found through ADL. C++0x requires std::swap find that, and C++03 user should do this: using std::swap; swap(a, b); to enable ADL (or just use boost::swap). There's nothing wrong with specializing the std::swap function per se, but it tends to be messier and isn't even possible for template classes.Unhandled
Was not the whole problem with std::auto_ptr that when you applied the assignment operator that rhs was modified and this is not what you normally expect of your obejcts. Assignment should not (in normal situations) modify the rhs because it confuses the user of the object. With the new std::unique_ptr you have to explicitly move the object, I would hope that users would adapt this as the standard mechanism when implementing their own move operations.Zebec
@Martin- Note that you can't invoke this assignment operator in normal use because there's no copy constructor. The only way you can use it is if the rhs is an rvalue that's either explicitly moved (as is the case in unique_ptr) or is already an rvalue (for example, the return value of a function).Burgoo
But now rhs is gone ("emptied", in your words). I would consider an assignment operation that destroys the assigned-from thing to be a serious violation of the principle of least surprise... hence my answer.Telepathist
Move-and-swap is indeed reasonable -- there is absolutely nothing reasonable about it, it goes against clear meaning of copy assignment.Insult
@Gene Bushuyev- This doesn't go against copy assignment because it isn't a copy assignment operator. C++0x differentiates between move and copy assignment, and the operator defined here is a move assignment operator. Although it takes its parameter by value, it can't be invoked on an lvalue. The only way to invoke it is if the rhs is an rvalue, which only happens if you capture a return value or explicitly move the rhs int the lhs. It's perfectly legal and normal C++0x code to do this; see unique_ptr or the streams classes for details.Burgoo
@Burgoo -- unique_ptr and streams implement honest move assignments: unique_ptr& operator=(unique_ptr&& u), basic_istream& operator=(basic_istream&& rhs), etc. I don't know a single class in standard library, except auto_ptr that would use these tricks.Insult
@templatetypedef: and the operator defined here is a move assignment operator -- no, it is not, it is copy assignment operator (12.8/17): A user-declared copy assignment operator X::operator= is a non-static non-template member function of class X with exactly one parameter of type X, X&, const X&, volatile X& or const volatile X&.Insult
I stand corrected! Thanks for pointing this out. However, I have a follow-up. I have read about a proposed rule-of-four-and-a-half that uses an assignment operator like this one to implement both copy and move semantics based on the type of the argument. Is this not considered good C++0x practice?Burgoo
@templatetypedef: there was a quite comprehensive discussion here: cpp-next.com/archive/2009/09/your-next-assignmentInsult
H
2

I think this is fine, but I don't really understand why you wouldn't just do:

operator=(Foo&& rhs) // pass by rvalue reference not value

And save yourself a move.

Hightension answered 26/7, 2011 at 2:21 Comment(3)
except one thing, move semantics is not swap semantics, see for example, cpp-next.com/archive/2009/09/your-next-assignmentInsult
so basically, you need to implement swap in terms of move (e.g. std::swap), not the move in terms of swap.Insult
I was mainly focusing on changing pass by value to pass by rvalue reference. I just copied and pasted the other code from the question, not intending to address that.Hightension
T
1

What follows is opinion, and I am not really up on the 0x standard, but I think I have fairly solid reasoning backing me up.

No. In fact, it would be proper not to support assignment at all.

Consider the semantics:

"assign" means "cause B, which exists already, to be identical to A". "copy" means "create B, and cause it to be identical to A". "swap" means "cause B to be identical to what A was, and simultaneously cause A to be identical to what B was". "move" means "cause B to be identical to what A was, and destroy A."

If we cannot copy, then we cannot copy-and-swap. Copy-and-swap is meant to be a safe way of implementing assignment: we create C which is identical to A, swap it with B (so that C is now what B was, and B is identical to A), and destroy C (cleaning up the old B data). This simply doesn't work with move-and-swap: we must not destroy A at any point, but the move will destroy it. Further, moving doesn't create a new value, so what happens is we move A into B, and then there is nothing to swap with.

Besides which - the reason for making the class noncopyable is surely not because "create B" will be problematic, but because "cause it to be identical to A" will be problematic. IOW, if we can't copy, why should we expect to be able to assign?

Telepathist answered 26/7, 2011 at 2:40 Comment(7)
You want to be able to assign an rvalue. Such as Foo f = returns_Foo_byval(); this is move assignment.Notional
Foo f; f = returns_Foo_byval(); would be an assignment, but the line you have would be a construction, unless they changed a lot more than they needed to in 0x.Telepathist
I respectfully disagree. There are many cases (assigning to the return value of a function, for one) where you would want to support assigning from rvalues. You also may have cases like uncopyable, movable objects in a container where revalued assignment is crucial to proper operation. Finally, objects like unique_ptr work specifically by doing this.Burgoo
Agreed, this violates the meaning of (copy) assignment, using copy assignment syntax to fake move assignment. And because of that it will fail every time compiler tries to invoke it as copy assignment. The standard just got rid of auto_ptr, there is no need to sneak back the same practices.Insult
@Burgoo what do you mean by "revalued assignment"?Telepathist
@Karl Knechtel- Sorry, autocorrect mangled "rvalue assignment" into "revalued assignment."Burgoo
"move" means "cause B to be identical to what A was, and destroy A." This is simply not true. You simply change A to ensure its eventual destruction will not affect B. Which invalidates the rest of your case against "move-and-swap." You create a C, and if A happens to die later it won't matter [it won't die, of course, until the expression the assignment is a part of completes]. Further, moving doesn't create a new value... No, but the paramter to the assignment operator does.Harwell

© 2022 - 2024 — McMap. All rights reserved.