Implementing the swap in the copy and swap idiom

Question

Following What is the copy and swap idiom and How to provide a swap function for my class, I tried implementing the swap function like in the latter accepted answer option number 2 (having a free function that calls a member function) instead of the direct friendly free function in the former link.

However the following doesn't compile

#include <iostream>

// Uncommenting the following two lines won't change the state of affairs
// class Bar;
// void swap(Bar &, Bar &);
class Bar {
public:
  Bar(unsigned int bottles=0) : bottles(bottles) { enforce(); } // (1)
  Bar(Bar const & b) : bottles(b.bottles) { enforce(); } // (1)

  Bar & operator=(Bar const & b) {
    // bottles = b.bottles;
    // enforce();
    // Copy and swap idiom (maybe overkill in this example)
    Bar tmp(b); // but apart from resource management it allows (1)
                // to enforce a constraint on the internal state
    swap(*this, tmp); // Can't see the swap non-member function (2)
    return *this;
  }

  void swap(Bar & that) {
    using std::swap;
    swap(bottles, that.bottles);
  }

  friend std::ostream & operator<<(std::ostream & out, Bar const & b) {
    out << b.bottles << " bottles";
    return out;
  }

private:
  unsigned int bottles;
  void enforce() { bottles /=2; bottles *= 2; } // (1) -- Ensure the number of bottles is even
};

void swap(Bar & man, Bar & woman) { // (2)
  man.swap(woman);
}

int main () {
  Bar man (5);
  Bar woman;

  std::cout << "Before -> m: " << man << " / w: " << woman << std::endl;
  swap(man, woman);
  std::cout << "After  -> m: " << man << " / w: " << woman << std::endl;

  return 0;
}

I know that the copy and swap idiom is overkill here but it also allows one to enforce some constraint on the internal state through the copy constructor (1) (A more concrete example would be to maintain a fraction in reduced form). Unfortunately, this doesn't compile because the only candidate for (2) that the compiler sees is the Bar::swap member function. Am I stuck with the friend non-member function approach?

EDIT: Go to my answer below to see what I ended up with thanks to all the answers and comments on this question.

Answer 1

I take it we're post c++11?

In which case, the default implementation of std::swap will be optimal, provided we correctly implement the move-assignment operator and the move-constructor (ideally nothrow)

http://en.cppreference.com/w/cpp/algorithm/swap

#include <iostream>

class Bar {
public:
    Bar(unsigned int bottles=0) : bottles(bottles) { enforce(); } // (1)
    Bar(Bar const & b) : bottles(b.bottles) {
        // b has already been enforced. is enforce necessary here?
        enforce();
    } // (1)
    Bar(Bar&& b) noexcept
    : bottles(std::move(b.bottles))
    {
        // no need to enforce() because b will have already been enforced;
    }

    Bar& operator=(Bar&& b) noexcept
    {
        auto tmp = std::move(b);
        swap(tmp);
        return *this;
    }

    Bar & operator=(Bar const & b)
    {
        Bar tmp(b); // but apart from resource management it allows (1)
        swap(tmp);
        return *this;
    }

    void swap(Bar & that) noexcept {
        using std::swap;
        swap(bottles, that.bottles);
    }

    friend std::ostream & operator<<(std::ostream & out, Bar const & b) {
        out << b.bottles << " bottles";
        return out;
    }

private:
    unsigned int bottles;
    void enforce() {  } // (1)
};

/* not needed anymore
void swap(Bar & man, Bar & woman) { // (2)
    man.swap(woman);
}
*/
int main () {
    Bar man (5);
    Bar woman;

    std::cout << "Before -> m: " << man << " / w: " << woman << std::endl;
    using std::swap;
    swap(man, woman);
    std::cout << "After  -> m: " << man << " / w: " << woman << std::endl;

    return 0;
}

expected result:

Before -> m: 5 bottles / w: 0 bottles
After  -> m: 0 bottles / w: 5 bottles

EDIT:

For the benefit of anyone who is concerned about performance (e.g. @JosephThompson) allow me to allay your concerns. After moving the call to std::swap into a virtual function (to force clang to produce any code at all) and then compiling with apple clang with -O2, this:

void doit(Bar& l, Bar& r) override {
    std::swap(l, r);
}

became this:

__ZN8swapper24doitER3BarS1_:            ## @_ZN8swapper24doitER3BarS1_
    .cfi_startproc
## BB#0:
    pushq   %rbp
Ltmp85:
    .cfi_def_cfa_offset 16
Ltmp86:
    .cfi_offset %rbp, -16
    movq    %rsp, %rbp
Ltmp87:
    .cfi_def_cfa_register %rbp
    movl    (%rsi), %eax
    movl    (%rdx), %ecx
    movl    %ecx, (%rsi)
    movl    %eax, (%rdx)
    popq    %rbp
    retq
    .cfi_endproc 

See? optimal. The c++ standard library rocks!

Answer 2

Note: This is pre C++11 way to use copy and swap. For a C++11 solution see this answer

In order to get this to work you need to fix a couple things. First you need to forward declare the swap free function so that operator= knows about it. In order to do that you also need to forward declare Bar so swap that there is a type named bar

class Bar;

void swap(Bar & man, Bar & woman);

// rest of code

Then we need to tell the compiler where to look for swap. The way we do that is to use the scope resolution operator. This will tell the compile to look in the out scope of the class for a swap function

Bar & operator=(Bar const & b) {
  // bottles = b.bottles;
  // enforce();
  // Copy and swap idiom (maybe overkill in this example)
  Bar tmp(b); // but apart from resource management it allows (1)
            // to enforce a constraint on the internal state
  ::swap(*this, tmp); // Can't see the swap non-member function (2)
//^^ scope operator 
  return *this;
}

We put all of that together and we get this Live Example

Really though the copy operator = should look like

Bar & operator=(Bar b) // makes copy
{
    ::swap(*this, b) // swap the copy
    return *this; // return the new value
}

Answer 3

You know that Bar has a swap member function, so just call it directly.

Bar& operator=(Bar const& b) {
    Bar tmp(b);
    tmp.swap(*this);
    return *this;
}

The non-member swap only exists so that clients of Bar can take advantage of its optimized swap implementation without knowing whether it exists, using the using std::swap idiom to enable argument-dependent lookup:

using std::swap;
swap(a, b);

Answer 4

You need to enable std::swap in that function too.

using std::swap;
swap(*this, tmp); // Can't see the swap non-member function (2)

Quoting the answer you referred to:

If swap is now used as shown in 1), your function will be found.

The way it is used:

{
  using std::swap; // enable 'std::swap' to be found
                   // if no other 'swap' is found through ADL
  // some code ...
  swap(lhs, rhs); // unqualified call, uses ADL and finds a fitting 'swap'
                  // or falls back on 'std::swap'
  // more code ...
}

Live on Coliru

Answer 5

For the above context, where one only needs to enforce some internal constraint, better use the default and just enforce only once the constraint in the direct initialization constructor. Still if you need to implement these functions, look at @RichardHodges answer! See also @HowardHinnant comment (especially the slides part on when the compiler does magic implicitly declares special members ...).

Here's what I ended up with (no explicit copy and swap anymore):

#include <iostream>

class Bar {
public:
  Bar(unsigned int bottles=0) : bottles(bottles) { enforce(); } // The only point of enforcement

  friend std::ostream & operator<<(std::ostream & out, Bar const & b) {
    out << b.bottles << " bottles";
    return out;
  }

private:
  unsigned int bottles;
  void enforce() { bottles /= 2; bottles *=2; }
};

int main () {
  Bar man (5);
  Bar woman;

  std::cout << "Before -> m: " << man << " / w: " << woman << std::endl;
  using std::swap; // Argument dependent lookup
  swap(man, woman);
  std::cout << "After  -> m: " << man << " / w: " << woman << std::endl;

  return 0;
}

Now, what happens if Bar inherits from Foo (which doesn't need enforce). This is the original use case that made me think I needed to unroll my own special functions and to profit from the copy part of the copy and swap idiom to enforce the constraint. It turns out that even in this case I don't need to:

#include <iostream>

class Foo {
public:
  Foo(unsigned int bottles=11) : bottles(bottles) {} // This is odd on purpose

  virtual void display(std::ostream & out) const {
    out << bottles << " bottles";
  }

protected:
  unsigned int bottles;
};

std::ostream & operator<<(std::ostream & out, Foo const & f) {
  f.display(out);
  return out;
}

class Bar : public Foo {
public:
  Bar(unsigned int bottles=0) : Foo(bottles) { enforce(); }
  Bar(Foo const & f) : Foo(f) { enforce(); }

  void display(std::ostream & out) const override {
    out << bottles << " manageable bottles";
  }

private:
  void enforce() { bottles /= 2; bottles *=2; }
};

int main () {
  Bar man (5); // Again odd on purpose
  Bar woman;

  std::cout << "Before -> m: " << man << " / w: " << woman << std::endl;
  using std::swap; // Argument dependent lookup
  swap(man, woman);
  std::cout << "After  -> m: " << man << " / w: " << woman << std::endl;

  Foo fool(7); // Again odd
  Bar like(fool);
  std::cout << fool << " -> (copy) " << like << std::endl;
  Bar crazy;
  crazy = fool;
  std::cout << fool << " ->   (=)  " << crazy << std::endl;

  return 0;
}