What is the proper approach to swap and copy idiom in virtual inheritance?

Question

Consider classic virtual inheritance diamond hierarchy. I wonder to know what is the right implementation of copy and swap idiom in such hierarchy.

The example is a little artificial - and it is not very smart - as it would play good with default copy semantic for A,B,D classes. But just to illustrate the problem - please forget about the example weaknesses and provide the solution.

So I have class D derived from 2 base classes (B<1>,B<2>) - each of B classes inherits virtually from A class. Each class has non trivial copy semantics with using of copy and swap idiom. The most derived D class has problem with using this idiom. When it calls B<1> and B<2> swap methods - it swaps virtual base class members twice - so A subobject remains unchanged!!!

A:

class A {
public:
  A(const char* s) : s(s) {}
  A(const A& o) : s(o.s) {}
  A& operator = (A o)
  {
     swap(o);
     return *this;
  }
  virtual ~A() {}
  void swap(A& o)
  {
     s.swap(o.s);
  }
  friend std::ostream& operator << (std::ostream& os, const A& a) { return os << a.s; }

private:
  S s;
};

B

template <int N>
class B : public virtual A {
public:
  B(const char* sA, const char* s) : A(sA), s(s) {}
  B(const B& o) : A(o), s(o.s) {}
  B& operator = (B o)
  {
     swap(o);
     return *this;
  }
  virtual ~B() {}
  void swap(B& o)
  {
     A::swap(o);
     s.swap(o.s);
  }
  friend std::ostream& operator << (std::ostream& os, const B& b) 
  { return os << (const A&)b << ',' << b.s; }

private:
  S s;
};

D:

class D : public B<1>, public B<2> {
public:
  D(const char* sA, const char* sB1, const char* sB2, const char* s) 
   : A(sA), B<1>(sA, sB1), B<2>(sA, sB2), s(s) 
  {}
  D(const D& o) : A(o), B<1>(o), B<2>(o), s(o.s) {}
  D& operator = (D o)
  {
     swap(o);
     return *this;
  }
  virtual ~D() {}
  void swap(D& o)
  {
     B<1>::swap(o); // calls A::swap(o); A::s changed to o.s
     B<2>::swap(o); // calls A::swap(o); A::s returned to original value...
     s.swap(o.s);
  }
  friend std::ostream& operator << (std::ostream& os, const D& d) 
  { 
     // prints A::s twice...
     return os 
    << (const B<1>&)d << ',' 
    << (const B<2>&)d << ',' 
        << d.s;
  }
private:
  S s;
};

S is just a class storing string.

When doing copy you will see A::s remains unchanged:

int main() {
   D x("ax", "b1x", "b2x", "x");
   D y("ay", "b1y", "b2y", "y");
   std::cout << x << "\n" << y << "\n";
   x = y;
   std::cout << x << "\n" << y << "\n";
}

And the result is:

ax,b1x,ax,b2x,x
ay,b1y,ay,b2y,y
ax,b1y,ax,b2y,y
ay,b1y,ay,b2y,y

Probably adding B<N>::swapOnlyMewould resolve the problem:

void B<N>::swapOnlyMe(B<N>& b) { std::swap(s, b.s); }
void D::swap(D& d) { A::swap(d); B<1>::swapOnlyMe((B<1>&)d); B<2>::swapOnlyMe((B<2>&)d); ... }

But what when B inherits privately from A?

Answer 1

Here's a philosophical rant:

1. I don't think virtual inheritance can or should be private. The entire point of a virtual base is that the most derived class owns the virtual base, and not the intermediate classes. Thus no intermediate class should be permitted to "hog" the virtual base.

2. Let me repeat the point: The most derived class owns the virtual base. This is evident in constructor initializers:

   D::D() : A(), B(), C() { }
   //       ^^^^
   //       D calls the virtual base constructor!
   

   In the same sense, all other operations in D should be immediately responsible for A. Thus we are naturally led to writing the derived swap function like this:

   void D::swap(D & rhs)
   {
       A::swap(rhs);   // D calls this directly!
       B::swap(rhs);
       C::swap(rhs);
   
       // swap members
   }
   

3. Putting all this together, we're left with only one possible conclusion: You have to write the swap functions of the intermediate classes without swapping the base:

   void B::swap(B & rhs)
   {
       // swap members only!
   }
   
   void C::swap(C & rhs)
   {
       // swap members only!
   }
   

Now you ask, "what if someone else wants to derive from D? Now we see the reason for Scott Meyer's advice to always make non-leaf classes abstract: Following that advice, you only implement the final swap function which calls the virtual base swap in the concrete, leaf classes.

---

Update: Here's something only tangentially related: Virtual swapping. We continue to assume that all non-leaf classes are abstract. First off, we put the following "virtual swap function" into every base class (virtual or not):

struct A
{
    virtual void vswap(A &) = 0;
    // ...
};

Usage of this function is of course reserved only to identical types. This is safeguarded by an implicit exception:

struct D : /* inherit */
{
    virtual void vswap(A & rhs) { swap(dynamic_cast<D &>(rhs)); }

    // rest as before
};

The overall utility of this is limited, but it does allow us swap to objects polymorphically if we happen to know that they're the same:

std::unique_ptr<A> p1 = make_unique<D>(), p2 = make_unique<D>();
p1->vswap(*p2);

Answer 2

Virtual base generally means that most derived class of object is in control of it.

First solution: Reorganize your classes to be more fit for polymorphism. Make copy construction protected. Remove assignment and swap(). Add virtual clone(). Idea is that the classes should be treated as polymorphic. So they should be used with pointer or smart pointer. Swapped or assigned should be the pointer values, not the object values. In such context swap and assignment only confuse.

Second solution: Make B and C abstract and their pointers not to manage object lifetime. Destructors of B and C should be protected and non-virtual. B and C will not therefore be most derived classes of object. Make B::swap() and C::swap() protected and not swap the A subobject, may rename or add comment that it is business of inherited classes now. That removes lot of object slicing possibilities. Make D::swap() to swap A subobject. You get one swap of A.

Third solution: Make D::swap() to swap A subobject. That way the A subobject will be swapped 3 times and lands in correct place. Inefficient? The whole construct is probably bad idea anyway. I am for example not sure how well the virtual destructors and swaps cooperate here and lot of ways to slice objects are public here. It all seems similar to attempt of making virtual assignment operators that is bad idea in C++.

If something inherits from D on its order then it should make sure by swapping or not swapping A subobject that the count of swapping A is odd. It becomes to control so should take over and fix.

The private virtual idiom is one of ways to make a class final in C++. Nothing should be able to inherit from it. Interesting that you asked. If you ever use it, be sure to comment, it confuses most of the readers of code.