[PATCH] -Woverloaded-virtual

[David Blaikie]

Looks good - I'd probably simplify the example to something more like
what we talked about on IRC:

struct base { void func(char); };
struct derived: base { void func(int); };

derived d;
d.func('x'); // FIXME...
d.func(42); // don't warn

& maybe mention that that should be under another flag (since these
functions aren't virtual) - (though this could also mean that it'd be
best in another test file, since this file is named
"overloaded-virtual.cpp", but that's a minor detail)

Up to you there,
- David

On Wed, Jul 30, 2014 at 3:54 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
> On Wed, Jul 30, 2014 at 6:31 PM, David Blaikie <dblaikie at gmail.com> wrote:
>> I think something pithier might be helpful (while your comment is
>> correct, I found it a little hard to follow the motivations, etc) -
>> spitballing:
>>
>> Here Clang deviates from GCC by only diagnosing overloads of inherited
>> virtual function that do not override any other virtual functions in
>> the base.
>> GCC's -Woverloaded-virtual is more about any case of derived functions
>> hiding base functions and doesn't seem specific to virtual functions.
>> The cases GCC catches that Clang does not may be better served by a
>> general warning on call sites that, had the base function been
>> visible, would've called that function. This would avoid false
>> positives such as warning on base::func() + derived::func(int) where
>> callers are unlikely to call the latter when they intended the former
>> (and Clang's typo correction can correct derived::func() to
>> base::func() already).
>>
>> Maybe that's too much of a rant. I'm not sure how much detail to give,
>> really... The last sentence there, while useful, is perhaps too much
>> detail - maybe it'd be better served by an example in a test case
>> somewhere with a FIXME of "warn here"? (and/or, as you say, a bug
>> filed)
>
> I took the last sentence out as it seemed a bit too specific, and
> changed the wording around slightly. I like the idea of a FIXME, so I
> made an example and added it to a test case. Think this looks
> reasonable?
>
> Thanks!
>
> ~Aaron