44

I'm trying to implement a rather large object that implements many interfaces. Some of these interfaces are pure virtual. I may have a problem in diamond inheritance. Visual Studio is reporting a warning of C4250 ('class1' : inherits 'class2::member' via dominance). First of all these classes are inherited virtually as it should be. The following is the partial class design that causes this problem.

A        B        C
 \      / \      /
  \    /   \    /
    AB       BC 
    |         |
    |        BC2
    |         |
     \        D: Implementation of B, C, BC, BC2
      \      /
        Big

In this entire tree only D implements virtual methods, there is no other definition of the method in question. And all virtual methods of B is listed in warnings. If important, D is a complete class.

I read this happens with Boost serialization and it is safe to disregard the warning.

Is this method I am trying to achieve valid? Is it safe to disregard this warning?

Note 1: This is not a duplicate of Visual Studio Compiler warning C4250 ('class1' : inherits 'class2::member' via dominance), I have tried the solution proposed there.

Note 2: I can also send class diagram but its a little more complicated than this.

EDIT: Full warning is as follows:

warning C4250: 'gge::resource::ImageResource' : inherits 
'gge::graphics::ImageTexture::gge::graphics::ImageTexture::drawin' 
via dominance

gge::resource::ImageResource is Big in the drawing, gge::graphics::ImageTexture is D, drawin is one of the six methods I get warning for.

Community
  • 1
  • 1
Cem Kalyoncu
  • 14,120
  • 4
  • 40
  • 62
  • Implementing many interfaces is a sign that your class may have too much responsibility. Consider refactoring it into several smaller classes. – hammar Jul 28 '11 at 19:33
  • @hammar: I tried but the problem is not about the class, Big is just an image but there are many interfaces to support many different type of graphics. Functions that require the services of a specific interface should use the smallest interface that satisfies their needs. As a sample, using these properties, I can substitute a particle system as a mouse pointer. In short I couldn't break into smaller classes. – Cem Kalyoncu Jul 28 '11 at 19:39
  • 3
    Consider the Decoration pattern (http://en.wikipedia.org/wiki/Decorator_pattern) as an alternative to subclassing. – Emile Cormier Jul 28 '11 at 19:51
  • @emile: I read Decorator pattern, its more complicated compared to MI. This method compiles and runs fine but not sure if it will keep working. I have an alternative to create a variable of D and map public methods to it. But this design seems more natural. – Cem Kalyoncu Jul 28 '11 at 20:00
  • 2
    Which classes specifically from your diagram are "class1" and "class2" in the message? – Mark B Jul 28 '11 at 20:13
  • @mark: nmspc::Big inherits nmspc::D::nmspc::D::member, D is repeated twice if it means anything. – Cem Kalyoncu Jul 28 '11 at 20:16
  • @Cem Kalyoncu: Can you post the exact (and complete) warning? Also identify in where in the drawing each one of the classes in the warning lies (i.e. `nmspc::Big` is `Big` in the diagram, `nmspc::D` is *what*?) – David Rodríguez - dribeas Jul 28 '11 at 21:02
  • @David: D is right above big in the right side. – Cem Kalyoncu Jul 28 '11 at 21:50

1 Answers1

27

Everything is absolutely valid. A compiler is allowed to warn about valid code, no problem here. You can try silencing the warning with a using declaration. If this doesn't work (probably due to an MSVC bug), silence it with a pragma.

n. m. could be an AI
  • 112,515
  • 14
  • 128
  • 243
  • If using was working I was not going to ask the question, pragma should do the trick. Thanks for the answer. – Cem Kalyoncu Jul 28 '11 at 20:01
  • I hear that for some people `using` works or fails depending on non-significant whitespace in the source file. If it's true, there's definitely a compiler bug there. – n. m. could be an AI Jul 28 '11 at 20:27
  • 4
    @DeniseSkidmore: it should be `using base::method;` but it doesn't work with VC++ anyway, and other compilers don't need it. – n. m. could be an AI Dec 10 '13 at 08:15
  • A using-declaration will not override inheritance by dominance, whatever compiler you use: https://godbolt.org/z/LtUAJU still returns 2. – Spencer Jan 08 '20 at 14:43
  • @Spencer the problem is not in overriding anything or changing how the feature works but in.silencing a fairly useless warning. – n. m. could be an AI Jan 08 '20 at 15:10
  • It certainly boils down to suppressing the warning, but the "uselessness" of the warning is a matter of opinion. The using-declaration in the sample code I provided is ineffective and also misleading. Without the warning, the program would silently misbehave with no obvious reason. – Spencer Jan 08 '20 at 19:02
  • @Spencer There is one single implementation of the method in the class and all its base classes, which the programmer obviously wants to be used. I don't see what other intent could possibly be there. – n. m. could be an AI Jan 09 '20 at 14:28
  • That's true In OP's case, but not in my sample code. My point is that `using` will never work. – Spencer Jan 09 '20 at 17:23
  • It seems that clang (at least version 9.0.0) disagrees here and the `using B::f` in the linked godbolt example changes the output. With the `using` it returns 1, without it returns 2, as gcc and MSVC. – Joachim Mairböck Jan 27 '20 at 14:31
  • @JoachimMairböck This most definitely looks like a clang bug to me. Calling the function via A, B or C references to a D object results in the expected 1 being returned. Only calling through a D reference returns 2. But there should be a single final overrider. – n. m. could be an AI Jan 27 '20 at 16:44
  • @n.'pronouns'm. If there is a bug, it is not limited to clang, as we have at least 3 different interpretations of this situation: https://godbolt.org/z/nX7Kbv – MFH Jan 28 '20 at 10:31
  • @MFH I'm not surprised one little bit. This is a seldom-explored area of the language. This code should print 2 in every case (IMHO) as C::f is the final overrider. – n. m. could be an AI Jan 28 '20 at 10:55
  • 1
    @MFH Late update: the latest versions of clang, icc and gcc all seem to handle this correctly. – n. m. could be an AI Jan 31 '23 at 18:51