Diamond-inheritance scenario compiles fine in G++, but produces warnings/errors in VC++/Eclipse

Question

I have a base class 'Base', which is a pure virtual class:

class Base {

public:
    virtual void A() = 0;
    virtual void B() = 0;

    virtual ~Base() { } // Eclipse complains that a class with virtual members must have virtual destructor
};

I also have 2 other classes, one of them implements A() and the other one implements B():

class DerivedA : public virtual Base
{
public:
    virtual void A() {
        printf("Hello from A");
    }
};

class DerivedB : public virtual Base
{
public:
    virtual void B() {
        printf("Hello from B");
    }
};

The virtual keyword in the declaration should solve the diamond problem.

Now I would like to combine the two classes into another class, so that both A() and B() are implemented, like this:

class DerivedC: public DerivedA, public DerivedB {
     // Now DerivedA and DerivedB are combined
};

// Somewhere else in the code
DerivedC c;
c.A();
c.B();

The problem: Even though G++ compiles the code just fine, Eclipse gives an error: The type 'DerivedC' must implement the inherited pure virtual method 'Base::B'. When compiling with visual studio, I get 2 warnings:

warning C4250: 'DerivedC' : inherits 'DerivedB::DerivedB::B' via dominance
warning C4250: 'DerivedC' : inherits 'DerivedA::DerivedA::A' via dominance

So the question is: what is the correct way of doing this? Does the code above produce undefined behavior?

Note: The title may be a little misleading, I have no idea what a good title for this question would be.

I have updated the title of your question. While not perfect, it is less generic. Your question could be further improved if you provided a complete code-example that we can copy/paste/compile to reproduce the error (see http://sscce.org). — Björn Pollex, Aug 15 '12 at 07:59
@BjörnPollex Thanks, this title is a little better. The original code in which I want to use the concept is way too complex to put in this question, but you could copy and paste the current code if you want to compile. Just add the stdio.h header, and place the 'Somewhere else in the code' part in the main function, and that's it. — Tibi, Aug 15 '12 at 08:08
I know that I could easily construct a sample from the code you have provided myself. The idea here is that you are likely to get more help if you make it *as easy as possible* for others to reproduce your error. Another remark: Could you specify the compiler-versions you are using (and also which compiler you Eclipse is using?). — Björn Pollex, Aug 15 '12 at 08:12
@BjörnPollex Eclipse version is Juno (the error was generated by the eclipse error checker or however it is called). Note that eclipse compiles using G++. G++ version is 4.7.0 (mingw package), and Visual Studio version is 2010. — Tibi, Aug 15 '12 at 08:23

Alok Save · Accepted Answer · 2012-08-15T08:23:30.440

8

What is the correct way of doing this? Does the code above produce undefined behavior?

The code is perfectly valid. There is no Undefined Behavior here.
An unqualified call of A() through a DerivedC class object, will always call DerivedA::A(), while an unqualified call of B() through a DerivedC class object, will always call the DerivedB::B() instance.

Visual C++ gives you a warning because your code uses a less commonly known feature of virtual Inheritance which may not be obvious to most common users and might surprise them. In this case, the warning should be taken as an Informative Nitpick rather than a warning.

Note that the C++ Standard does not restrict compilers from emitting informative warnings for perfectly valid code. The documentation for warning C4250 gives an example which tells you why visual C++ chooses to give this warning.

edited Aug 15 '12 at 08:23

answered Aug 15 '12 at 08:09

Alok Save

202,538
53
430
533

I understand, however the warnings are there for a reason, maybe there could be a scenario where this would cause a conflict. For example, if DerivedA or DerivedB implemented both functions, this could cause some trouble. – Tibi Aug 15 '12 at 08:11
3

The formal specification of dominance may not be widely known, but the results are what one would intuitively expect. It's the basis for the widely used mixin technique. The warning is totally stupid. (But Visual Studios allows turning warnings off on a one by one basis, so you can just turn this one off.) – James Kanze Aug 15 '12 at 08:23
@JamesKanze: I wouldn't go as far as saying the warning being *"totally stupid"*. For some fairly experienced users the warning may seem that way but for novice programmers it could be fairly useful, especially since the documentation explains *"why?"* in much detail.And since the warning can be easily suppressed by using the [warning pragma.](http://msdn.microsoft.com/en-us/library/2c8f766e(v=vs.80).aspx) – Alok Save Aug 15 '12 at 08:27
1

@Als The information is OK, but "warning" seems to imply more. The formal semantics of C++ result in what one would intuitively expect, and it doesn't seem very sensible to "warn" that you're getting what you would intuitively expect. – James Kanze Aug 15 '12 at 08:56

score 6 · Answer 2 · answered Aug 15 '12 at 07:40

6

You might want to try this :

class DerivedC: public DerivedA, public DerivedB {
public:
    using DerivedA::A;
    using DerivedB::B;
};

I can't test with Eclipse or VC++ myself...

answered Aug 15 '12 at 07:40

BatchyX

4,986
2
18
17

In Visual Studio it gives the same warnings, but in Eclipse/G++ the error seems to be gone. – Tibi Aug 15 '12 at 07:45
You probably want one of those, but not both of them. – user541686 Aug 15 '12 at 08:00
@Mehrdad: They are different functions, so it should be fine to declare `using` for both. – Björn Pollex Aug 15 '12 at 08:04

score 4 · Answer 3 · answered Aug 15 '12 at 08:20

I don't know why a compiler would complain about any of this; this is just the standard mixin technique. Classes Base, DerivedA and DerivedB are abstract, and can't be instantiated, but that's usually the case for mixins. The whole point of a mixin is that it doesn't implement all of the interface. And DerivedC implements both A() and B() through its inherited members.

If a compiler refuses to compile this code, it is broken.

As for warnings... a compiler is free to warn about anything it pleases:

There is no requirement that a class with virtual members have a virtual destructor. In practice, it's usually a good idea, however (unless the destructor is protected), and a compiler warning is appropriate.
The warnings from Visual Studio are "informative", I guess, but this is the way the language is designed to work. And it's certainly not something to avoid. For that matter, I don't think that dominance actually plays a role here, since the functions in Base are pure virtual. What Visual Studios seems to be trying to say is that in DerivedC, the actual overload of A() is DerivedA::A(), and not Base::A(). Which seems to be what one would intuitively expect to me; the rules concerning dominance are really just a formal statement of what one would intuitively expect.

Anyway, I'd definitely turn the warning about dominance off. There's certainly nothing to worry about in that respect. And I'd complain loudly about a compiler which didn't compile the code.

Both the compilers compiled the code, VS just gave warnings, and the eclipse error was generated by the eclipse automated error checker or something, but the actual compilation in eclipse is done using g++. — Tibi, Aug 15 '12 at 08:26

score 3 · Answer 4 · answered Aug 15 '12 at 08:31

Visual Studio is known to have a compiler bug issuing warning C4250 in situations where the dominated function is pure virtual. The bug has been closed as "Won't fix"; the accepted solution is to suppress the warning using:

#pragma warning( disable: 4250 ) /* 'class1' : inherits 'class2::member' via dominance */

See also discussion at http://msdn.microsoft.com/en-us/library/6b3sy7ae(VS.80).aspx#CommunityContentHeader.

score 0 · Answer 5 · edited Aug 15 '12 at 08:06

0

Your Base class is abstract : it can't be instantiated. The B and A class are also abstract because they implement only one method.

The two solutions are, in file DerivedC.cpp

void DerivedC::A(){ 
   DerivedA::A();
}
void DerivedC::B(){
   Derived:B();
}

Or you can use using keyword in your header-file:

class DerivedC: public DerivedA, public DerivedB {
public:
    using DerivedA::A;
    using DerivedB::B;
};

edited Aug 15 '12 at 08:06

Björn Pollex

75,346
28
201
283

answered Aug 15 '12 at 07:44

artragis

3,677
1
18
30

In the first case, if I call `A()` will it also call the inherited version, or only the `DerviedC::A()` which is empty? Shouldn't you call `DerivedA::A()` inside `DerviedC::A()`? – Tibi Aug 15 '12 at 07:58
Okay I understand, you should also do a little spellcheck, it's 'Derived' instead of 'Dervied'. – Tibi Aug 15 '12 at 08:03
This shouldn't be necessary (unless `DerivedC` defined other `A()`). – James Kanze Aug 15 '12 at 08:21
In fact it can be necessary because DerivedC is a DerivedA so if one function or method just need a DerivedA you can give it a DerivedC. As DerivedA::A and DerviedA::B are virtual (and pure virtual for ::B) if the function calls the B method it will call DerivedC::B. So you got to define the behaviour of DerivedC::B as a call to DerivedB::B by writing it or by using `using` keyword. – artragis Aug 15 '12 at 08:28

Diamond-inheritance scenario compiles fine in G++, but produces warnings/errors in VC++/Eclipse

5 Answers5