[PATCH] Static analysis checker for catch handler inversion

[Aaron Ballman]

I noticed that this warning was a bit anachronistic, so I've done some
updates. This patch now gives the warning a new group (GCC does not
have a group for this, so I've created -Wexceptions), and now prints
the QualType directly instead of forcing it through a string.

~Aaron

On Mon, Oct 27, 2014 at 9:00 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
> Here is a reworked version of the same patch -- one thing to note is
> that this already had a FIXME in the code regarding it being quadratic
> performance. This patch retains the note about that issue, but I am
> not too worried about the performance as this is per-try block. If
> performance is truly questionable, we can always use the checker-based
> approach instead.
>
> I've removed the TypeWithHandler helper class as it's essentially just
> a std::pair with special logic for operator<. Since we have to perform
> the quadratic check for derivative classes, it doesn't make sense to
> then loop a second time over the same (sorted) data just for a simple
> comparison of QualTypes. This makes my patch a performance regression
> for testing duplicate handlers -- I do not have numbers regarding
> whether this is actually a measurable regression for realistic test
> cases or not, but my gut reaction based on our assumption of 8
> handlers being a reasonable number suggests this likely isn't
> perceptible.
>
> ~Aaron
>
> On Mon, Oct 27, 2014 at 7:40 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
>> On Mon, Oct 27, 2014 at 7:38 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>>> On Mon, Oct 27, 2014 at 4:35 PM, Aaron Ballman <aaron at aaronballman.com>
>>> wrote:
>>>>
>>>> On Mon, Oct 27, 2014 at 7:30 PM, Jordan Rose <jordan_rose at apple.com>
>>>> wrote:
>>>> > Nifty! But do you think this is cheap enough for a general compiler
>>>> > warning? It certainly doesn't depend on the analyzer's path-sensitive
>>>> > analysis, so it's mostly just how much we care about the cost of
>>>> > isDerivedFrom.
>>>>
>>>> This should be relatively inexpensive, so it may make sense as a
>>>> general compiler warning if others feel that's a better approach.
>>>
>>>
>>> Yes, I think this is a good candidate for an (on-by-default) compiler
>>> warning.
>>
>> Then I'll rework this, thanks!
>>
>> ~Aaron
-------------- next part --------------
A non-text attachment was scrubbed...
Name: CatchHandlerChecker_v3.patch
Type: application/octet-stream
Size: 9754 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141028/c5e2a1e1/attachment.obj>