[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:44 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
> On Thu, Sep 18, 2014 at 9:15 AM, Aaron Ballman <aaron at aaronballman.com>
> wrote:
>>
>> 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.
>
>
> Little confusion when I started to look. Addressed in r218059.
>
> (if you like, you can revert this commit now that it won't flag on the bots
> - I don't really mind all that much either way)

Thanks! I've reverted these in r218062.

~Aaron