[PATCH] Static analysis checker for catch handler inversion

Aaron Ballman aaron at aaronballman.com
Mon Oct 27 18:00:08 PDT 2014


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_v2.patch
Type: application/octet-stream
Size: 7275 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141027/6f0d8a31/attachment.obj>


More information about the cfe-commits mailing list