Warnings in C::B

[oBFusCATed]

Hello,

There are lots of warnings in C::B, some of them are harmless, but these two are not:

Code

../../../include/compiletargetbase.h:133: warning: ‘virtual void CompileTargetBase::SetTargetType(TargetType)’ was hidden
../../../include/projectbuildtarget.h:105: warning:   by ‘virtual void ProjectBuildTarget::SetTargetType(const TargetType&)’

Can someone familiar with the code of the compiler framework fix them?

[Jenna]

What do I have to do to get this warning ?
I don't get it on linux (trunk), neither from inside C::B, nor with automake.

[oBFusCATed]

I've this in my global compiler options:

Code

       -Woverloaded-virtual (C++ and Objective-C++ only)
           Warn when a function declaration hides virtual functions from a base class.  For example, in:

                   struct A {
                     virtual void f();
                   };

                   struct B: public A {
                     void f(int);
                   };

           the "A" class version of "f" is hidden in "B", and code like:

                   B* b;
                   b->f();

           will fail to compile.

I don't know why I have it added to the globals though

[killerbot]

this is however interesting. I will experiment with this ..

[Jenna]

The question is, if it is really necessary to declare these functions virtual, and the other question is if it is a problem to ignore some of these warnings and/or rename some of the functions g++ is warning about (like BreakpointsDlg::Refresh or InfoPane::Show ).

[oBFusCATed]

Today, I was reading the C++ FAQ and found something interesting:

http://www.parashift.com/c++-faq-lite/strange-inheritance.html#faq-23.9

[thomas]

I think the issue is really that these are not overloaded virtual functions at all, since one passes the parameter by value and the other passes the parameter by const reference.

And though this is a serious error rather than some pedantic sophistry, it so happens that the compiler seems to incidentially pick the "correct" function (const&), as it happens to be the one that optimizes better.

Now, the right solution would be, obviously, to change the base class (and all other derived classes that might have a wrong signature too) to const reference. However, that would mean that we break the SDK's interface (which we agreed on some 1-1.5 years ago shouldn't be done). Yiannis?

[oBFusCATed]

I think the issue is really that these are not overloaded virtual functions at all, since one passes the parameter by value and the other passes the parameter by const reference.

There are also functions with different number of parameters.
Also there are virtual operator= which are totally wrong (search google for info).

[MortenMacFly]

However, that would mean that we break the SDK's interface (which we agreed on some 1-1.5 years ago shouldn't be done). Yiannis?

IIRC we agreed not to break in in the release branches (08/02 and 10/05), but in trunk I'm afraid we have already several times, not to mention the developer branches.

[thomas]

There are also functions with different number of parameters.

Well yes, obviously, and that's nothing to worry about.
But in this case, the apparent/obvious intent is to overload a base class' function. It's just not what is done, and the fact that it works is pure coincidence. Thus I say that it's actually a severe error.

Also there are virtual operator= which are totally wrong (search google for info)

While I'm not sure how it makes sense to overload the assignment operator, I don't see a real hindrance to doing that. After all, operator= is just a function, like every other function. In fact, SetTargetType could be thought of a kind of "partial" operator=.
Which, again, makes me wonder (only judging from the function's name, without actually looking at the implementation) if it's necessary to overload such a function. It actually shouldn't ever need to be any different than the one in the base class. But well, there may be some call to UpdateFoobar() which is necessary in one and not the other, I sadly don't have the time to dig through source now to look.