0

Consider the following C++ sample main.cpp file:

class FooIf
{
public:
    virtual int handle(char *req, char *res) = 0;
};

class BarIf
{
public:

    virtual void handle(char *msg) = 0;
};

class Bar : private BarIf
{
private:
    void handle(char * msg){}
};

class Zoo : public FooIf, public Bar
{
public:
    using FooIf::handle;
public:
    int handle(char *req, char *res){ return (0); }
};

int main(){

    Zoo zoo;
    return (0);
}

I'm getting this warning :

$ clang++ -ggdb -c main.cpp -Wall
main.cpp:23:6: warning: 'Zoo::handle' hides overloaded virtual function [-Woverloaded-virtual]
        int handle(char *req, char *res){ return (0); }
            ^
main.cpp:17:7: note: hidden overloaded virtual function 'Bar::handle' declared here: different number of parameters (1 vs 2)
        void handle(char * msg){}
             ^

Now .. I'm indeed hiding Bar::handle and I'm doing it on purpose.

Is there a way to avoid suppressing the warning while getting this around?

It is unnecessary to say that g++ does not complain at all about this.

Riccardo Manfrin
  • 653
  • 1
  • 7
  • 15
  • 1
    You should not be doing it on purpose. You are defeating the intention of polymorphism. If you don't want polymorphism, use encapsulation. – Richard Hodges Sep 29 '16 at 13:49
  • I don't see your point, can you make an example? The only answer I found to this was given here (http://stackoverflow.com/questions/18515183/c-overloaded-virtual-function-warning-by-clang) and definitely looks more like an excuse to me rather than a real motivation. I'd gladly avoid removing the warning, but don't want to screw the code just because clang wants to prevent me from typos.. – Riccardo Manfrin Sep 29 '16 at 13:56
  • 1
    the definition of handle is in the wrong place. A polymorphic base class obliges all derived classes to implement all methods in the base class. So remove handle() from BarIf, because you don't need it there -- or derive Zoo from something else – Richard Hodges Sep 29 '16 at 13:59
  • 1
    TL;DR: The warning should probably be an error. The design of the classes is broken. – Kuba hasn't forgotten Monica Sep 29 '16 at 14:37
  • > It is unnecessary to say that g++ does not complain at all about this. huh? – xaxxon Oct 01 '16 at 08:30

2 Answers2

5

You should consider a different design. This seems like a code smell / design issue if you really need this.

If (for whatever reason) you really want to do that you can turn the warning off at the specific place. See here for info http://clang.llvm.org/docs/UsersManual.html#controlling-diagnostics-via-pragmas

In you case this would look like this

class Zoo : public FooIf, public Bar
{
public:
    using FooIf::handle;
public:
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Woverloaded-virtual"
    int handle(char *req, char *res){ return (0); }
#pragma clang diagnostic pop
};
Hayt
  • 5,210
  • 30
  • 37
  • Really appreciated, but I miss the point about wrong design.. if I gave one of the `handle` methods a different name it would be perfectly fine. The design here is a class that inherits from an virtual pure interface AND from another class, which accidentally have a method with a clashing name. What is wrong with this design? – Riccardo Manfrin Sep 29 '16 at 14:02
  • 2
    @RiccardoManfrin Why is it there the need to hide it in the first place? why not rename the functions to something appropriate? "handle" is a very generic name. I cannot really tell you where the issue in this design is without details. As I said it's a code-smell. Something smells wrong which usually indicates an issue in your design. This can lead to unexpected behavior if someone sees it's derived from BarIf and calls handle and expects "BarIf::handle" to be called. And the compiler wont catch this so you run into potential runtime bugs. – Hayt Sep 29 '16 at 14:09
  • You are right about this (renaming) AND I'm about to solve this way. Yet, if renaming is the solution, design is not necessarly wrong. Details on design: Zoo is equivalent to Bar and has to expose exactly the same API, plus some extra methods to specialize it: it's a specialization of Bar that extends its properties but has to expose the same API (which I would not want to rewrite). At the same time Zoo has to respond to another (completely different) API defined by ZooIf which is addressed by a completely different component. – Riccardo Manfrin Sep 29 '16 at 14:35
  • 1
    @RiccardoManfrin I was with "bad design" more referring to the "hiding on purpose" if for some reason you have to do that (and renaming would not be an options) this would seem like there is something wrong somewhere. There can be very specific reasons for needing this though (e.g. 3rd party libraries). If the hiding was "accidental" because both interfaces had the same name in the function than simply renaming is fine. – Hayt Sep 29 '16 at 14:40
  • It is all the way accidental (feel better). – Riccardo Manfrin Sep 29 '16 at 15:22
1

Warnings identify possible problems.

Possible is a key word here. Your code can run completely fine if you ignore the warnings. But in the experience of millions of developers over trillions of lines of code has led the compiler to say "this is a bad idea".

Fixing it consists of suppressing the warning, or simply renaming the method.

This is a potential warning because the meaning of handle in that class varies substatially depending on exactly how you invoke it. In addition, people sometimes accidentally fail to overload a function by giving it a slightly different signature.

Between the two, it is considered worthwhile to flag it as a warning.

The best route to fix it will depend on details not shown in your question.

Yakk - Adam Nevraumont
  • 262,606
  • 27
  • 330
  • 524
  • Yes, what I wanted to understand is the motivation for the warning to be... and it appears to be rather easy in my case: don't call two functions the same way even though they have different fingerprints if they don't relate to each other since this can confuse people.. – Riccardo Manfrin Sep 29 '16 at 15:27