22

I'm one of those people that has to get their code to compile with 0 warnings. Normally I respect the compiler and if it issues me a warning I take it as a sign that I should touch up my code a little. If I have to tell a compiler to ignore a given warning, I twitch a little.

But this one I can't seem to get around, and from what I can tell I haven't done anything "bad". Does anyone think that this is a poor design? I can't see anything particularly nasty about it (except for the "evil diamond") but it's perfectly valid and useful code. But it generates (in MSVC) a level 2 warning!

class IFoo
{
public:
    virtual void foo() = 0;
};

class Bar : public virtual IFoo
{
public:
    virtual void foo() { std::cout << "Hello, world!"; }
};

class Baz : public virtual IFoo
{

};

class Quux : public Bar, public Baz
{

};

Now if I create a Quux object it should be expected to call the Bar::foo implementation. MSVC is very helpful: it warns me for not being ambiguous enough?

warning C4250: 'Quux' : inherits 'Bar::Bar::foo' via dominance

Now I recognize I can turn this warning off with a pragma, but that's not the question I'm trying to ask here. Is there a reason I should be listening to the compiler here, or is this just an extremely overzealous warning?

3
  • 1
    Honestly, using virtual inheritance at all is bad design in my opinion. It's just too complicated for very little gain. I would seriously think twice (or three or four times) before using it. Aug 15, 2011 at 22:50
  • 1
    The code might not be bad, but it is fragile. Relying on obscure disambiguation rules to select the function to call doesn't make the code very easy to read or maintain.
    – Bo Persson
    Aug 16, 2011 at 8:15
  • 4
    @PeterAlexander "It's just too complicated" complicated how?
    – curiousguy
    Nov 2, 2011 at 22:30

3 Answers 3

12

When performing virtual inheritance, it is a bad idea to not explicitly override every member in the most derived class. Else, you are asking for your code to die a horrible death when someone changes one of your base classes that inherits from the virtual base. There's nothing actively wrong with this, your program won't crash or anysuch, but it's a maintainability bad idea. If you want to call the Bar::foo version, then you should just delegate to it in Quux::foo.

6
  • I think this is a good point. It doesn't quite fit in the actual problem for me to move everything to the most-derived class (the above is, of course, a simplified example) so I may have to double-check my overall design. (I do expect users of the Bar class.)
    – Shirik
    Aug 15, 2011 at 23:00
  • 3
    Nonsense. Just because you don't practice something doesn't make it unsound/bad/whatever. Just say: "I am not used to that."
    – curiousguy
    Nov 2, 2011 at 22:33
  • 1
    @curiousguy: I never said that I don't practice it, and I gave a perfectly good reason why it's bad. If you want to suggest that I'm wrong, then give an objective reason why it's not bad.
    – Puppy
    Nov 2, 2011 at 23:37
  • 3
    "die a horrible death" horrible death = compiler message whenever you introduce an ambiguity. Horrible, indeed. (lol)
    – curiousguy
    Aug 7, 2012 at 4:48
  • 1
    Actually, what happens with virtual function table in this case? Is there 1 entry for foo() or 2? If there is only one entry and foo() is overridden both in Bar and in Baz, what would that do to a virtual funtction table of a final class?
    – Serge
    Oct 6, 2015 at 0:08
2

As far as the runability of your code is concerned, it is just there to remind you that Bar is the dominant implementation of foo. It is just there to inform you, it's not really a warning, so that if you're debugging and think it's Baz you don't pull your hair out :).

2
  • 1
    Personally, at least, I view level 4 warnings as "just to inform you." Level 2 (which this is) seems to be more along the lines of "what are you doing here...". (For comparison: C4390 is level 3, and that's usually bad.)
    – Shirik
    Aug 15, 2011 at 22:58
  • 3
    I would be interested in a compiler that informs me of every non-obvious C++ rule used to compile a program. For every overloaded function call, there would be like 5 messages; for every builtin operator called by overloading resolution; there would be 15 messages; for every template overloaded function call, there would be 30 messages. The compile log would be a lot bigger than the program source code.
    – curiousguy
    Nov 23, 2011 at 5:19
1

Is there a reason you aren't writing:

class Quux : public Bar, public Baz
{
    using Bar::foo;
};

?

This gives you the same level of reuse, without the fragility.

1
  • 6
    It doesn't help. It throws the warning anyway. VC++ 2013
    – Maks
    Jun 2, 2015 at 17:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.