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
}