[llvm] r218050 - Fixing a bunch of -Woverloaded-virtual warnings due to hiding getSubtargetImpl from the base class. NFC.

[Aaron Ballman]

On Thu, Sep 18, 2014 at 12:12 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
> On Thu, Sep 18, 2014 at 9:04 AM, Aaron Ballman <aaron at aaronballman.com>
> wrote:
>>
>> On Thu, Sep 18, 2014 at 11:55 AM, David Blaikie <dblaikie at gmail.com>
>> wrote:
>> >
>> >
>> > On Thu, Sep 18, 2014 at 6:27 AM, Aaron Ballman <aaron at aaronballman.com>
>> > wrote:
>> >>
>> >> Author: aaronballman
>> >> Date: Thu Sep 18 08:27:14 2014
>> >> New Revision: 218050
>> >>
>> >> URL: http://llvm.org/viewvc/llvm-project?rev=218050&view=rev
>> >> Log:
>> >> Fixing a bunch of -Woverloaded-virtual warnings due to hiding
>> >> getSubtargetImpl from the base class. NFC.
>> >
>> >
>> > Which compiler is producing that warning here?
>>
>> I believe this is GCC warning on it. It's our attribute documentation
>> building bot that is kicking off these warnings, so I don't have a
>> link to a bot to show you. If you would like, I can forward the bot
>> report email.
>
>
> That's OK - it looks most likely like the GCC flavor of this warning.
>
>>
>>
>> > I /think/ this is one of those cases where we improved the Clang warning
>> > and
>> > should just disable the GCC one.
>> >
>> > (there's some debate about what the /point/ of this warning is, which is
>> > fair - but I tuned Clang's to detect the cases where someone tried to
>> > override and accidentally overloaded - which isn't the case here. The
>> > base
>> > class has a couple of virtual methods, the derived class correctly
>> > overrode
>> > one of them. I don't think this is a major source of bugs/worth fixing,
>> > possibly)
>> >
>> > (the gap of course is that you can still end up with this problem:
>> > struct base { virtual void func(int); virtual void func(bool); };
>> > struct derived : base { void func(bool) override; void stuff() {
>> > func(42);
>> > /* oops, this calls func(bool) because shadowing */ } };
>> > hence the debate about what the purpose of the warning is)
>>
>> I don't have any strong opinions on whether the warning provides value
>> or not, but I do have strong opinions on ensuring a warning-free build
>> for this bot, which is the only reason I cleaned these up.
>
>
> *nod* if it's not too much hassle - could we clean it up by disabling the
> warning instead? I assume we already have a bunch of GCC warnings we disable
> & it's just a matter of adding this one to the list...

I don't know how much hassle is involved, I've not disabled any GCC
warnings (only MSVC ones). If someone who knows more about where that
magic lives in the build system would like to tackle it, I am okay
with the idea.

~Aaron