reusing the copy-and-swap idiom
Asked Answered
M

6

11

I'm trying to put the copy-and-swap idiom into a reusable mixin:

template<typename Derived>
struct copy_and_swap
{
    Derived& operator=(Derived copy)
    {
        Derived* derived = static_cast<Derived*>(this);
        derived->swap(copy);
        return *derived;
    }
};

I intend it to be mixed in via CRTP:

struct Foo : copy_and_swap<Foo>
{
    Foo()
    {
        std::cout << "default\n";
    }

    Foo(const Foo& other)
    {
        std::cout << "copy\n";
    }

    void swap(Foo& other)
    {
        std::cout << "swap\n";
    }
};

However, a simple test shows that it is not working:

Foo x;
Foo y;
x = y;

This only prints "default" twice, neither "copy" nor "swap" is printed. What am I missing here?

Madcap answered 16/8, 2011 at 14:48 Comment(1)
Maybe the compiler is providing his own version of operator= since you are missing one in your Foo class? Maybe you'll have to do Foo::operator=(){return copy_and_swap();} (pseudocode)?Issue
E
0

I am afraid this is one area where a macro is necessary, because of the complex rules about automatically generated copy and assignment operators.

No matter what you do, you are in either of two cases:

  • You have provided (explicitly) a declaration of the assignment operator, in which case you are expected to provide a definition too
  • You have not provided (explicitly) a declaration of the assignment operator, in which case the compiler will generate one if the base classes and non-static members have one available.

The next question, therefore, is: Is it worth it to automate such writing ?

Copy-And-Swap is only used for very specific classes. I do not think it's worth it.

Educatory answered 16/8, 2011 at 15:50 Comment(0)
E
7

This:

 Derived& operator=(Derived copy)

doesn't declare a copy assignment operator for the base class (it has the wrong signature). So the default generated assignment operator in Foo will not use this operator.

Remember 12.8:

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&.) [Note: an overloaded assignment operator must be declared to have only one parameter; see 13.5.3. ] [Note: more than one form of copy assignment operator may be declared for a class. ] [Note: if a class X only has a copy assignment operator with a parameter of type X&, an expression of type const X cannot be assigned to an object of type X.

EDIT don't do this (can you see why ?):

You can do:

template<typename Derived>
struct copy_and_swap
{
    void operator=(const copy_and_swap& copy)
    {
        Derived copy(static_cast<const Derived&>(copy));
        copy.swap(static_cast<Derived&>(*this));
    }
};

but you lose the potential copy elision optimization.

Indeed, this would assign twice the members of derived classes: once via copy_and_swap<Derived> assignment operator, and once via the derived class' generated assignment operator. To correct the situation, you'd have to do (and not forget to do):

struct Foo : copy_and_swap<Foo>
{

    Foo& operator=(const Foo& x)
    {
        static_cast<copy_and_swap<Foo>&>(*this) = x;
        return *this;
    }

private:
    // Some stateful members here
}

The moral of the story: don't write a CRTP class for the copy and swap idiom.

Explicable answered 16/8, 2011 at 14:53 Comment(4)
What if you = delete the compiler-generated assignment operator for Foo (assuming C++0x)?Baccate
= delete will forbid calling operator= on Foo, which is counterproductive to this.Padding
@R. Martinho: no, it is that you force the base class to assign the members of the derived class, and the default assignment operator of the derived class will also assign the members. So in the end, you assign twice.Explicable
@Mike - if you have c++0x, wouldn't you just use the 'move constructor'?Ricercare
P
1

You cannot inherit assignment operators as a special case, if memory correctly serves. I believe that they can be explicitly using'd in if you need.

Also, be careful about over use of copy-and-swap. It produces non-ideal results where the original has resources that could be re-used to make the copy, such as containers. Safety is guaranteed but optimum performance is not.

Psycholinguistics answered 16/8, 2011 at 14:50 Comment(1)
Adding the using copy_and_swap<Foo>::operator=; in struct Foo, doesn't correct the situation. See Alexandre C.'s answer.Hui
F
0

The compiler automatically generates a copy assignment operator for Foo, since there is none. If you add a

    using copy_and_swap<Foo>::operator=;

to Foo you will see an error telling you about the ambiguity on g++.

Fa answered 16/8, 2011 at 14:54 Comment(2)
I tried this and it doesn't give any ambiguity. However, the operator is still not invoked.Hui
@André Caron: I checked it again: g++ does msvc not.Fa
B
0

Maybe you could rewrite it so it looks like so:

template<class Derived>
struct CopySwap
{
  Dervied &operator=(Derived const &other)
  {
    return AssignImpl(other);
  }

  Derived &operator=(Dervied &&other)
  {
    return AssignImpl(std::move(other));
  }

private:
  Derived &AssignImpl(Derived other)
  {
    auto self(static_cast<Derived*>(this));
    self->swap(other);
    return *self;
  }
};

It'll probably all get inlined and likely won't be any slower than the original code.

Baccate answered 16/8, 2011 at 15:12 Comment(0)
E
0

I am afraid this is one area where a macro is necessary, because of the complex rules about automatically generated copy and assignment operators.

No matter what you do, you are in either of two cases:

  • You have provided (explicitly) a declaration of the assignment operator, in which case you are expected to provide a definition too
  • You have not provided (explicitly) a declaration of the assignment operator, in which case the compiler will generate one if the base classes and non-static members have one available.

The next question, therefore, is: Is it worth it to automate such writing ?

Copy-And-Swap is only used for very specific classes. I do not think it's worth it.

Educatory answered 16/8, 2011 at 15:50 Comment(0)
C
0

This does not really answer the question (@Alexandre C. already did), but if you reverse the inheritance, you could make it work:

template<typename Base>
struct copy_and_swap : Base
{
    copy_and_swap& operator=(copy_and_swap copy)
    {
        swap(copy);
        return *this;
    }
};

struct Foo_
{
    Foo_()
    {
        std::cout << "default\n";
    }

    Foo_(const Foo_& other)
    {
        std::cout << "copy\n";
    }

    void swap(Foo_& other)
    {
        std::cout << "swap\n";
    }
};

typedef copy_and_swap<Foo_> Foo;

int main()
{
    Foo x;
    Foo y;
    x = y;
}
Childbed answered 16/8, 2011 at 15:56 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.