After discussion in IRC, David convinced me that this really should be a call site diagnostic instead of using this approach. So instead, here is a patch that adds some comments explaining the rationale. David, do you think I've captured it properly?
~Aaron On Wed, Jul 30, 2014 at 5:30 PM, Aaron Ballman <[email protected]> wrote: > On Wed, Jul 30, 2014 at 4:42 PM, David Blaikie <[email protected]> wrote: >> I made some improvements to -Woverloaded-virtual a while ago and >> semi-deliberately chose the behavior you're observing (in the sense >> that it seemed like this warning was for catching the case where you >> tried to override a virtual function but accidentally ended up >> overloading it). > > That's good to know, I kind of wondered whether this behavior was > deliberate or not. > >> The situation you've described might be better suited to a separate >> flag > > How much do we care about diverging from GCC in this regard? > >> - and should warn even if there's no overriding going on. This >> could fire on non-virtual functions too: >> >> struct base { >> void func(char); >> }; >> >> struct derived: base { >> void func(int); >> }; >> >> We currently don't warn here. > > I could see this making some sense under the same flag as the virtual > case. In either case, it's a hiding issue that prevents overloading. > >> Though I've done this deliberately in one or two places (where I >> didn't care about the base function, but had a derived function with >> the same naem & different args) - the right warning's probably to warn >> at the call site and tell people "this call site would've called a >> different function in the base class, if it were visible". > > That seems like a far more tricky diagnostic though (due to converting > constructors, conversion operators, etc); it would likely have higher > false positive rates, would it not? > > ~Aaron > >> >> On Wed, Jul 30, 2014 at 1:30 PM, Aaron Ballman <[email protected]> >> wrote: >>> I believe the following patch addresses a slight deficiency in our >>> -Woverloaded-virtual warning check. Specifically, it causes the >>> following code to warn (which matches GCC's behavior): >>> >>> struct base { >>> virtual void foo(int I) {} >>> virtual void foo(char C) {} >>> }; >>> >>> struct derived : public base { >>> void foo(int I2) override {} >>> }; >>> >>> It does this by continuing to check other methods instead of early >>> returning out of processing them when an exact signature match is >>> encountered. I believe this is an improvement because it catches >>> problems like: >>> >>> derived d; >>> d.foo('1'); >>> >>> Where derived::foo(int) is called when the user may expect >>> base::foo(char) to be called. >>> >>> ~Aaron >>> >>> _______________________________________________ >>> cfe-commits mailing list >>> [email protected] >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>>
SemaDeclCXX.cpp.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
