14

Why is it that in the code below the compiler complains that PureAbstractBase is an ambiguous base class of MultiplyInheritedClass? I realize I have two copies of the PureAbstractBase in MultiplyInheritedClass and that FirstConreteClass and SecondConreteClass should be derived virtually because they're the middle row of the diamond (and that does indeed fix the problem with the code below). But even though I have two copies of the interface why is it that the code in MultiplyInheritedClass does not just override both and unambiguously pick the interface class defined in MultiplyInheritedClass?

#include <iostream>
using namespace std;

class PureAbstractBase {
  public:
    virtual void interface() = 0;
};

// I know that changing the following line to:
// class FirstConcreteClass : public virtual PureAbstractBase {
// fixes the problem with this hierarchy
class FirstConcreteClass : public PureAbstractBase {
  public:
    virtual void interface() { implementation(); }
  private:
    void implementation() { cout << "This is object FirstConcreteClass\n"; }
};

// I know that changing the following line to:
// class SecondConcreteClass : public virtual PureAbstractBase {
// fixes the problem with this hierarchy
class SecondConcreteClass : public PureAbstractBase {
  public:
    virtual void interface() { implementation(); }
  private:
    void implementation() { cout << "This is object SecondConcreteClass\n"; }
};

class MultiplyInheritedClass : public FirstConcreteClass,
                               public SecondConcreteClass {
  public:
    virtual void interface() { implementation(); }
  private:
    void implementation() { cout << "This is object MultiplyInheritedClass\n"; }
};

Further, why do I not have issues with the following hierarchy? Doesn't the ConcreteHandler class have three copies of the AbstractTaggingInterface in this case? So why doesn't it have the same issue as the example above?

#include <iostream>
using namespace std;

class AbstractTaggingInterface {
  public:
    virtual void taggingInterface() = 0;
};

class FirstAbstractHandler : public AbstractTaggingInterface {
  public:
    virtual void taggingInterface() { cout << "FirstAbstractHandler\n"; }
    virtual void handleFirst() = 0;
};

class SecondAbstractHandler : public AbstractTaggingInterface {
  public:
    virtual void taggingInterface() { cout << "SecondAbstractHandler\n"; }
    virtual void handleSecond() = 0;
};

class ThirdAbstractHandler : public AbstractTaggingInterface {
  public:
    virtual void taggingInterface() { cout << "ThridAbstractHandler\n"; }
    virtual void handleThird() = 0;
};

class ConcreteHandler : public FirstAbstractHandler,
                        public SecondAbstractHandler,
                        public ThirdAbstractHandler {
  public:
    virtual void taggingInterface() = { cout << "ConcreteHandler\n"; }
    virtual void handleFirst() {}
    virtual void handleSecond() {}
    virtual void handleThird() {}
};

I am trying to wrap my head around all of this because I had a conversation with a colleague recently where he claimed that if you were inheriting from pure virtual classes (interfaces) without any data members then virtual inheritance was not necessary. I think understanding why the former code example does not work and the latter does would go a long way to getting this straight in my head (and clear up what exactly he meant by his comment). Thanks in advance.

bpw1621
  • 2,962
  • 2
  • 32
  • 35
  • I don't know if I understand your question, so could you please clarify? As far as I can see, the second example is just bog-standard inheritance with no surprises. The first example is ill-defined because you are multiply inheriting ambiguous overrides, which you can fix with virtual inheritance. – Kerrek SB Jun 19 '11 at 17:35
  • @Kerrek SB Thanks for the help and sorry about the hasty downvote (you are now upvoted). I changed the second code snippet to include overrides. I am sure my clarity issue has to do with my confusion over the issue. I guess my question is how is ConcreteHandler able to pick the proper override for the taggingInterface call whereas MultiplyInheritedClass is not able to? – bpw1621 Jun 19 '11 at 17:46
  • @bpw: No worries :-) So, what do you think is the ambiguity in the second example? At what point should it not be 100% clear which override to take? Since all the base functions are pure-virtual, they can't be called, so only one unique implementation remains. In the first example, there are **multiple** equally-valid implementations. Just think about how an `PureAbstractBase*` is supposed to find the actual function that you want without having all the requisite vtables (which you install via virtual inheritance). – Kerrek SB Jun 19 '11 at 17:49
  • @Kerrek SB Oh ... so the reason the second example works is because I can't actually instantiate any of the bases classes in that case? I imagine that means that if I make even one of the Handler functions non pure-virtual this will also break just like the first example? I will try that and if so please update your answer to reflect your comment and I'll mark it as correct. – bpw1621 Jun 19 '11 at 17:55
  • @bpw: Just give it a try. Take my simple example code below, for instance, and remove the "virtual" keywords and see what the compiler says. It'll all boil down to the ability to uniquely determine the correct callee from a base pointer, i.e. `A * p = new C; p->foo();`. That last call _must_ be uniquely determinable, or otherwise you get an error. (Also bear in mind that destructors must be callable.) – Kerrek SB Jun 19 '11 at 17:57
  • @Kerrek SB Just tried removing the abstract method from one of the handler classes making it non pure-virtual and it still compiles and prints what I expect (I have a trivial main where I instaniate a ConcreteHandler dynamically and call the tagging inteface via a pointer). So I think there's still something I am not quite getting. – bpw1621 Jun 19 '11 at 17:57
  • @bpw: Post the code and what you expect should happen and what actually happens. (Keep it minimal and short, I guess.) – Kerrek SB Jun 19 '11 at 17:59
  • @bpw1621: Are you sure the first example does not work with you? I have just tried it on Visual Studio 2008 and it worked very well, not even a warning! what compiler are you using? – Tamer Shlash Jun 19 '11 at 18:02
  • @Kerrek SB I think I messed up the example code so it looks like if I try to instantiate the most derived via a pointer to the tagging interface class it complains about ambiguity. So here is a question, should the second code snippet "work" or is it bad code that will throw compilation errors/warnings depending on how it's used? – bpw1621 Jun 19 '11 at 18:05
  • @bpw: The second snippet is pretty messed up at the moment. You cannot write "= { }", and you misspelled "Thrid"/"Third". But then you're still missing virtual inheritance! Do add code to _invoke_ the override, or you might miss the errors. – Kerrek SB Jun 19 '11 at 18:34
  • @Mr.TAMER yes as you pointed out in your answer the problem only comes when the calls on the classes are invoked in certain ways. – bpw1621 Jun 19 '11 at 20:48
  • @Kerrek SB I fixed the cut and paste errors as you suggested. I think the issue exactly what you and others are pointing out: that the code only fails to compile when it is invoked in certain ways. I was trying to figure out the crux of something more complicated and I think I get it now: the code never is invoked via a pointer to base so the call is never ambiguous and hence everything compiles fine although I now believe that code is incorrect. Thanks again. – bpw1621 Jun 19 '11 at 20:50
  • @Kerrek SB That also begs the question that if there is never a need to invoke the code this way in the "original" design I was trying to paraphrase then is inheritance the right design at all? – bpw1621 Jun 19 '11 at 20:53
  • @bpw: The compiler is certainly free to optimize away features that aren't used, so you can sometimes get away writing ambiguous definitions if those are never needed... so it's best always to write a complete test first! (And post the test on SO to show how far you've got :-) .) As for whether the design makes sense, only you can know. If your classes have a common interface but different specific implementations, then inheritance sounds right, but if you never need a certain interface, maybe something is wrong with the design there... – Kerrek SB Jun 19 '11 at 20:59

4 Answers4

6

You need virtual inheritance to overcome the diamond-ambiguity:

class FirstConcreteClass  : public virtual PureAbstractBase { ... };
class SecondConcreteClass : public virtual PureAbstractBase { ... };

Long-winded explanation: Suppose you have this:

// *** Example with errrors! *** //
struct A { virtual int foo(); };
struct B1 : public A { virtual int foo(); };
struct B2 : public A { virtual int foo(); };
struct C: public B1, public B2 { /* ... */ };  // ambiguous base class A!

int main() {
  A * px = new C;                              // error, ambiguous base!
  px->foo();                                   // error, ambiguous override!
}

The inheritance of the virtual function foo is ambiguous because it comes in three ways: from B1, from B2 and from A. The inheritance diagram forms a "diamond":

   /-> B1 >-\
A->          ->C
   \-> B2 >-/

By making the inheritance virtual, struct B1 : public virtual A; etc., you allow any baseclass of C* to call the correct member:

struct A { virtual int foo(); };
struct B1 : public virtual A { virtual int foo(); };
struct B2 : public virtual A { virtual int foo(); };
struct C: public B1, public B2 { virtual int foo(); };

We must also define C::foo() for this to make sense, as otherwise C would not have a well-defined member foo.

Some more details: Suppose we now have a properly virtually-inheriting class C as above. We can access all the various virtual members as desired:

int main() {
  A * pa = new C;
  pa->foo();      // the most derived one
  pa->A::foo();   // the original A's foo

  B1 * pb1 = new C;
  pb1->foo();     // the most derived one
  pb1->A::foo();  // A's foo
  pb1->B1::foo(); // B1's foo

  C * pc = new C;
  pc->foo();      // the most derived one
  pc->A::foo();   // A's foo
  pc->B1::foo();  // B1's foo
  pc->B2::foo();  // B2's foo
  pc->C::foo();   // C's foo, same as "pc->foo()"
}

 

Update: As David says in the comment, the important point here is that the intermediate classes B1 and B2 inherit virtually so that further classes (in this case C) can inherit from them while simultaneously keeping the inheritance from A unambiguous. Sorry for the initial mistake and thanks for the correction!

Kerrek SB
  • 464,522
  • 92
  • 875
  • 1,084
  • Please read the entire question: I know how to fix the first example as I clearly stated. I want to know why the second example does not suffer from the same issue the first example does. – bpw1621 Jun 19 '11 at 17:18
  • Did you just vote me down without understanding the explanation? (Which I might expand on if you don't tell me off needlessly?) The problem only arises when the virtual function is _overridden_ by the derived classes, i.e. if the derived classes define functions of the same name and signature. Your second example has no overridden functions! – Kerrek SB Jun 19 '11 at 17:26
  • If I add an override to the taggingInterface in any of the iHandler classes the example still compiles fine. I will take away my down vote because you are trying to be helpful and I did not mean to be harsh/rash. It was the second answer that did not seem to address the question. – bpw1621 Jun 19 '11 at 17:32
  • 2
    Downvoted (just now, not four hours ago) because the `MultiplyInheritedClass` is the wrong place to use virtual inheritance. The virtual inheritance has to be employed in both the `FirstConcreteClass` and `SecondConcreteClass`. The `MultiplyInheritedClass` should use simple public inheritance (unless it too might be used in some other meta-diamond pattern). – David Hammen Jun 19 '11 at 22:22
  • A new and different opinion is below now :) – mlvljr Jan 23 '12 at 13:12
  • @mlvljr: OK, I got it: What I meant is that I could "fix" my initial, non-virtual example by explicitly picking a base via `A * px = static_cast(new C);`, and then `px->foo()` would follow the appropriate path (namely `A` to `B1` to `C`). – Kerrek SB Jan 24 '12 at 06:15
  • Yep, but don't even need the cast, since you convertring to (one of) the base class. – mlvljr Jan 24 '12 at 07:45
  • @mlvljr: In the non-virtual example, there are *two* base subobjects of type `A`, so the straight cast is ambiguous - hence the intermediate cast. – Kerrek SB Jan 24 '12 at 13:37
  • Now that was my time to have it wrong ;) (I was looking at your diamond diagram), of cause you're right. Interestingly, this is the case when `static_cast` is used to perform an operation that would not otherwise (were it not for the interm. step) be needed. – mlvljr Jan 24 '12 at 15:52
5

Your first example fails because the compiler cannot disambiguate between the three implementations of implementation(). You are overriding that method in MultiplyInheritedClass, which actually overrides both FirstConcreteClass::implementation and SecondConcreteClass::implementation (once virtual, always virtual). However, both virtual calls still exist in the interface of MultiplyInheritedClass, which makes the call ambiguous at the call site.

The reason that your example works without virtual inheritance is that there is no conflicting implementation of the common base class. Put another way:

class Base
{
public:
    void DoSomething() {
    std::cout << "TADA!";
    }
}

class One : public Base
{
    //...
}

class Two : public Base
{
    //...
}

class Mixed : public One, public Two
{
    //...
}

int main()
{
    Mixed abc;
    abc.DoSomething(); //Fails because the compiler doesn't know whether to call
                       // One::DoSomething or Two::DoSomething, because they both
                       // have implementations.

    //In response to comment:
    abc.One::DoSomething(); //Succeeds! You removed the ambiguity.
}

Because your example has all pure virtual functions, there's no multiple implementations which the compiler needs to disambiguate. Therefore, only one implementation exists, and the call is unambiguous.

Billy ONeal
  • 104,103
  • 58
  • 317
  • 552
  • I changed the second code segment above so that the taggingInterface is now override by every interface (although the difference may be that there is no indirection through a private member function like in the first example?). Does this effect/change your answer at all? – bpw1621 Jun 19 '11 at 17:50
  • @bpw: 1. That code should not compile. (There are errant `=` signs that should not be there.) 2. What does the call site look like? Your first example has a call site, your second example does not. – Billy ONeal Jun 19 '11 at 18:55
  • It doesn't fail, it works well on VC++ 2008 if you duplicate the code of DoSomething instead of every comment. – Tamer Shlash Jun 19 '11 at 19:49
  • @Mr.TAMER: 1. Who cares what VC++ does? Most of us care what the standard says it has to do, because when you're targeting more than one compiler you'll get different results if you rely on something that's nonstandard. (VC++ has been known to be forgiving of things like this) 2. If you duplicate that code in every comment then you completely change the context of my example. Then the function in `Mixed` simply hides `One::DoSomething` and `Two::DoSomething` so there is no ambiguity. The comment means "Really doesn't matter what you put here" so long as you don't implement `DoSomething()`. – Billy ONeal Jun 19 '11 at 20:48
  • @Billy ONeal You comment was on the mark about the call site: I didn't realize that the code would compile fine _provided_ an ambiguous call is never made. I thought that there would be a preemptive warning/error for the design regardless of how it was actually used. – bpw1621 Jun 21 '11 at 21:03
  • 1
    @bpw1621: No -- it's perfectly legal for someone to go (from my second example) `abc.One::DoSomething()` (where `abc` is one of those mixed objects) -- that wouldn't be in error but it is a diamond pattern (You just removed the ambiguity). – Billy ONeal Jun 21 '11 at 21:13
  • @bpw1621: Edited the second example above to demonstrate. – Billy ONeal Jun 21 '11 at 21:16
  • if this is about overriding, should not the methods be virtual? if so, why are we talking about the "three implementations of `implementation`" (which is is not virtual in OP's example)? something seems wrong here.. – mlvljr Jan 23 '12 at 06:03
  • @mlvljr: No. Simple inheritance is sufficient to demonstrate the point. Making the method virtual doesn't change anything. Still the issue is duplicate implementations due to the base class being included twice. – Billy ONeal Jan 23 '12 at 08:07
  • see http://ideone.com/nXbNb, everything seems fine (there, for g++ 4.3, at least); besides Cameau c++ (http://www.comeaucomputing.com/tryitout/) and llvm (http://llvm.org/demo/index.cgi) both compile the same code successfully. – mlvljr Jan 23 '12 at 08:50
  • 1
    @mlvljr: That's not my example. Now in both cases you hid the base class method, which removes the ambiguity. You have to remove the implementations in one, two, and mixed, or make the base one virtual, to see this effect. You'll see that if you compile my actual example it fails: http://ideone.com/RmJQU – Billy ONeal Jan 23 '12 at 08:58
  • @mlvljr: The OP's original example compiles because he never invoked any of the methods. The error occurs at the call site. – Billy ONeal Jan 23 '12 at 08:58
  • ok, but please see http://ideone.com/8pnN2 for the OP's initial example (this also compiles successfully under Cameau and llvm) -- it does not fail, until something like `PureAbstractBase *pab = &mic;` is tried (so, _that_ is ambiguous conversion), i.e. the OP could invoke the methods, but could not convert correctly – mlvljr Jan 23 '12 at 09:09
  • I made my thoughts into an answer, check it below :) – mlvljr Jan 23 '12 at 13:13
1

I tried both of the question codes and they worked fine when instantiating an object of the multi-inherited class. It didn't work only with polymorphism, like this for example:

PureAbstractBase* F;
F = new MultiplyInheritedClass();

And the reason is clear: it doesn't know to which copy of the Abstract base class it should be linked (sorry for bad expressions, I understand the idea but can't express it). And since inherting virtaully makes only one copy exist in the derived class, then it's fine.

Also the code of Billy ONeal is not clear at all, what should we place instead of the comments?

If we place:

public:    
void DoSomething() 
{    std::cout << "TADA!";    }

it works fine, because of no virtuality.

I work on Visual Studio 2008.

Tamer Shlash
  • 9,314
  • 5
  • 44
  • 82
  • Tamer: You could put anything in the place of the comments. (Excepting an implementation of the pure virtual function `DoSomething`) If you have a problem with my answer, comment on my answer. Don't just post something else criticizing me and hope I read it. – Billy ONeal Jun 19 '11 at 19:41
0

Why not do it like this (suggested in Benjamin Supnik's blog entry):

#include <iostream>

class PureAbstractBase {
public:
    virtual void interface() = 0;
};

class FirstConcreteClass : public PureAbstractBase {
public:
    virtual void interface() { implementation(); }
private:
    void implementation() { std::cout << "Fisrt" << std::endl; }
};

class SecondConcreteClass : public PureAbstractBase {
public:
    virtual void interface() { implementation(); }
private:
    void implementation() { std::cout << "Second" << std::endl; }
};

class MultiplyInheritedClass : public FirstConcreteClass,
                               public SecondConcreteClass 
{
public:
    virtual void interface() { implementation(); }
private:
    void implementation() { std::cout << "Multiple" << std::endl; }
};

int main() {
MultiplyInheritedClass mic;
mic.interface();

FirstConcreteClass *fc = &mic; //disambiguate to FirstConcreteClass 
PureAbstractBase *pab1 = fc;
pab1->interface();

SecondConcreteClass *sc = &mic; //disambiguate to SecondConcreteClass 
PureAbstractBase *pab2 = sc;
pab2->interface();
}

which gives:

Multiple
Multiple
Multiple    

This way:

  • no virtual bases are involved (do you really need them?)
  • you can call the overriden function via a an instance of the MultiplyInheritedClass
  • ambiguity is removed by a two-stage conversion
mlvljr
  • 4,066
  • 8
  • 44
  • 61
  • Because sometimes you want to put implementations in the base class. This example works iff there is no implementation at the top of the diamond -- putting an implementation there is the whole reason virtual inheritance exists in the first place. – Billy ONeal Jan 23 '12 at 14:12
  • @Billy ONeal Well, this (OP's initial) exact case is not that 'sometimes' (my example exactly follows his one) ;) Again putting implementation in the base class would be trivial. Besides, we must possibly agree, that putting a _shared_ implementation there is why virtual inheritance exists, right?! – mlvljr Jan 23 '12 at 15:37