7

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;
}
wilx
  • 17,697
  • 6
  • 59
  • 114
Flame
  • 2,166
  • 2
  • 20
  • 40

3 Answers3

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!

Community
  • 1
  • 1
templatetypedef
  • 362,284
  • 104
  • 897
  • 1,065
  • 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. – Lightness Races in Orbit Jul 26 '11 at 01:00
  • @Tomalak Geret'kal- Great idea. Fixed. – templatetypedef Jul 26 '11 at 01:05
  • +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...] – Lightness Races in Orbit Jul 26 '11 at 01:09
  • @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. – templatetypedef Jul 26 '11 at 01:13
  • 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. – Lightness Races in Orbit Jul 26 '11 at 01:13
  • @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. – GManNickG Jul 26 '11 at 02:04
  • 1
    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. – Martin York Jul 26 '11 at 02:05
  • @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). – templatetypedef Jul 26 '11 at 02:23
  • 1
    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. – Karl Knechtel Jul 26 '11 at 02:42
  • *Move-and-swap is indeed reasonable* -- there is absolutely nothing reasonable about it, it goes against clear meaning of copy assignment. – Gene Bushuyev Jul 26 '11 at 17:02
  • @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. – templatetypedef Jul 26 '11 at 17:26
  • @templatetypedef -- `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. – Gene Bushuyev Jul 26 '11 at 18:01
  • @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&.** – Gene Bushuyev Jul 26 '11 at 18:07
  • 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? – templatetypedef Jul 26 '11 at 18:24
  • @templatetypedef: there was a quite comprehensive discussion here: http://cpp-next.com/archive/2009/09/your-next-assignment/ – Gene Bushuyev Jul 26 '11 at 18:37
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.

Clinton
  • 22,361
  • 15
  • 67
  • 163
  • except one thing, *move* semantics is not *swap* semantics, see for example, http://cpp-next.com/archive/2009/09/your-next-assignment/ – Gene Bushuyev Jul 26 '11 at 17:08
  • so basically, you need to implement `swap` in terms of move (e.g. `std::swap`), not the move in terms of swap. – Gene Bushuyev Jul 26 '11 at 17:14
  • 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. – Clinton Jul 28 '11 at 04:19
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?

Karl Knechtel
  • 62,466
  • 11
  • 102
  • 153
  • 1
    You want to be able to assign an rvalue. Such as Foo f = returns_Foo_byval(); this is move assignment. – Flame Jul 26 '11 at 02:46
  • `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. – Karl Knechtel Jul 26 '11 at 02:58
  • 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. – templatetypedef Jul 26 '11 at 04:17
  • 1
    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. – Gene Bushuyev Jul 26 '11 at 16:58
  • @templatetypedef what do you mean by "revalued assignment"? – Karl Knechtel Jul 26 '11 at 17:02
  • @Karl Knechtel- Sorry, autocorrect mangled "rvalue assignment" into "revalued assignment." – templatetypedef Jul 26 '11 at 17:24
  • `"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. – Dennis Zickefoose Jul 26 '11 at 20:22