Acceptable to use virtual inheritance to prevent accidentally creating a diamond?

Question

This is a simplification of some real code, and a real mistake I made when I didn't realize someone else had already implemented Foo and derived from it.

#include <iostream>

struct Base {
   virtual ~Base() { }
   virtual void print() = 0;
};

struct OtherBase {
   virtual ~OtherBase() { }
};

struct Foo : public Base { // better to use virtual inheritance?
   virtual void print() { std::cout << "Foo" << std::endl; };
};

struct Bar : public Base { // better to use virtual inheritance?
   virtual void print() { std::cout << "Bar" << std::endl; };
};

// The design is only supposed to implement Base once, but I
// accidentally created a diamond when I inherited from Bar also.
class Derived
   : public OtherBase
   , public Foo
   , public Bar // oops.
{
};

int main() {
   Derived d;
   OtherBase *pO = &d;

   // cross-casting
   if (Base *pBase = dynamic_cast<Base *>(pO))
      pBase->print();
   else
      std::cout << "fail" << std::endl;
}

EDIT: to save you from having to run this code...

• If run as-is, it prints "fail" (undesireable, hard to debug).
• If you delete the line marked "oops" it prints "Foo" (desired behavior).
• If you leave the "oops" and make the two inheritances virtual, it won't compile (but at least you know what to fix).
• If you delete the "oops" and make them virtual, it will compile and will print "Foo" (desired behavior).

With virtual inheritance, the outcomes are either good or compiler-error. Without virtual inheritance, the outcomes are either good or unexplained, hard-to-debug runtime failure.

---

When I implemented Bar, which basically duplicated what Foo was already doing, it caused the dynamic cast to fail, which meant bad things in the real code.

At first I was surprised there was no compiler error. Then I realized there was no virtual inheritance, which would have triggered the 'no unique final overrider' error in GCC. I purposefully chose not to use virtual inheritance since there aren't supposed to be any diamonds in this design.

But had I used virtual inheritance when deriving from Base, the code would have worked just as well (without my oops), and I would have been warned about the diamond at compile time vs. having to track down the bug at run time.

So the question is -- do you think it's acceptable to use virtual inheritance to prevent making a similar mistake in the future? There's no good technical reason (that I can see) for using virtual inheritance here, since there should never be a diamond in the design. It would only be there to enforce that design constraint.

Answer 1

Not a good idea.

Virtual inheritance is only to be used when it is planned in advance. As you just discovered, all descendant classes must know about it in many cases. If the base class has a non-default constructor you have to worry about the fact that it is always constructed by the leaf class.

Oh, and unless things have changed since last I looked, you cannot downcast a virtual base class to any derived class without help from the base class.

Answer 2

There is no diamond here!
What you created was a multiple inheritance. Each Base class has its own copy of Base.

pO has a type of OtherBase*.
There is no way to convert an object of OtherBase* to type Base*.
Thus dynamic cast will return a NULL pointer.

The problem is that dynamic cast at runtime has a pointer to an object of Derived. But to get from here to Base is an ambigious operation and thus fails with a NULL. No compiler error because dynamic_cast is a runtime operation. (You can try to cast from anything to anything the result is a NULL on failure (or throw if using references)).

Two options:

• You can get dynamic_cast to throw exceptions if you cast references though.
• Or you can use a cast that is checked at compile time static_cast<>

Check it out with this:

struct Base
{
    Base(int x): val(x) {}
    int val;
...

struct Foo : public Base
{
    Foo(): Base(1)  {}
.... 

struct Bar : public Base
{
    Bar(): Base(2)  {}
....


// In main:
    std::cout << "Foo" << dynamic_cast<Foo&>(d).val << "\n"
              << "Bar" << dynamic_cast<Bar&>(d).val << "\n";


> ./a.exe  
fail
Foo1
Bar2

Compile Time Check:

std::cout << static_cast<Base*>(pO) << "\n"; // Should fail to compile.
std::cout << static_cast<Base*>(&d) << "\n"; // Will only fail if ambigious.
                                             // So Fails if Foo and Bar derived from Base
                                             // But works if only one is derived.

Answer 3

First thing you should take into account is that inheritance is not for code reuse, so think it twice when inheriting from two bases with a common ancestor and methods implemented in both sides.

If you think you really want to inherit form both bases anyway, you will want to use virtual inheritance and not duplicate the ancestor. This is common when implementing exception hierarchies. Note that virtual bases are initialized directly by the constructor of the most-derived-type, and needs to care about that.