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

[David Blaikie]

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!


>
> ~Aaron
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140918/dbfbd51d/attachment.html>