<div dir="ltr"><div class="gmail_extra">I think it's not too hard to avoid the quadratic performance here. Algorithm outline:</div><div class="gmail_extra"><br></div><div class="gmail_extra">Start with empty map of type -> handlers</div><div class="gmail_extra">For each catch handler for a type T:</div><div class="gmail_extra">  For each base class U of T (including T itself), most-derived-first:<br></div><div class="gmail_extra">    If we have a handler for U in our map, warn, break</div><div class="gmail_extra">  Add handler for T to our map</div><div class="gmail_extra"><br></div><div class="gmail_extra">(This is still quadratic if you have struct A<n> : A<n-1>, and catch A<0>, A<1>, ..., A<n> in that order, but in that case I think your approach is cubic (because IsDerivedFrom is itself linear in the number of base classes.)</div><div class="gmail_extra"><br></div><div class="gmail_extra">Also, I'm not sure you're correctly handling this case:</div><div class="gmail_extra"><br></div><div class="gmail_extra">  try { ... }</div><div class="gmail_extra">  catch (Base *) {}</div><div class="gmail_extra">  catch (Derived &) {}</div><div class="gmail_extra"><br></div><div class="gmail_extra">We shouldn't warn here, but I think you might, because you strip off a top-level * or & quite early. I'd also like to see some test coverage for catch parameters with cv-qualifiers.</div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Oct 28, 2014 at 7:23 AM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I noticed that this warning was a bit anachronistic, so I've done some<br>
updates. This patch now gives the warning a new group (GCC does not<br>
have a group for this, so I've created -Wexceptions), and now prints<br>
the QualType directly instead of forcing it through a string.<br>
<span class="HOEnZb"><font color="#888888"><br>
~Aaron<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
On Mon, Oct 27, 2014 at 9:00 PM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>> wrote:<br>
> Here is a reworked version of the same patch -- one thing to note is<br>
> that this already had a FIXME in the code regarding it being quadratic<br>
> performance. This patch retains the note about that issue, but I am<br>
> not too worried about the performance as this is per-try block. If<br>
> performance is truly questionable, we can always use the checker-based<br>
> approach instead.<br>
><br>
> I've removed the TypeWithHandler helper class as it's essentially just<br>
> a std::pair with special logic for operator<. Since we have to perform<br>
> the quadratic check for derivative classes, it doesn't make sense to<br>
> then loop a second time over the same (sorted) data just for a simple<br>
> comparison of QualTypes. This makes my patch a performance regression<br>
> for testing duplicate handlers -- I do not have numbers regarding<br>
> whether this is actually a measurable regression for realistic test<br>
> cases or not, but my gut reaction based on our assumption of 8<br>
> handlers being a reasonable number suggests this likely isn't<br>
> perceptible.<br>
><br>
> ~Aaron<br>
><br>
> On Mon, Oct 27, 2014 at 7:40 PM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>> wrote:<br>
>> On Mon, Oct 27, 2014 at 7:38 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>> wrote:<br>
>>> On Mon, Oct 27, 2014 at 4:35 PM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>><br>
>>> wrote:<br>
>>>><br>
>>>> On Mon, Oct 27, 2014 at 7:30 PM, Jordan Rose <<a href="mailto:jordan_rose@apple.com">jordan_rose@apple.com</a>><br>
>>>> wrote:<br>
>>>> > Nifty! But do you think this is cheap enough for a general compiler<br>
>>>> > warning? It certainly doesn't depend on the analyzer's path-sensitive<br>
>>>> > analysis, so it's mostly just how much we care about the cost of<br>
>>>> > isDerivedFrom.<br>
>>>><br>
>>>> This should be relatively inexpensive, so it may make sense as a<br>
>>>> general compiler warning if others feel that's a better approach.<br>
>>><br>
>>><br>
>>> Yes, I think this is a good candidate for an (on-by-default) compiler<br>
>>> warning.<br>
>><br>
>> Then I'll rework this, thanks!<br>
>><br>
>> ~Aaron<br>
</div></div></blockquote></div><br></div></div>