[PATCH][StaticAnalyzer] new check comparing equal expression

Hans Wennborg hans at chromium.org
Mon Sep 23 11:14:19 PDT 2013


On Mon, Sep 23, 2013 at 8:45 AM, Daniel Marjamäki
<Daniel.Marjamaki at evidente.se> wrote:
>
> Hello!
>
> Thank you very much for commenting.
>
>> Nice! This is a subset of http://llvm.org/PR9952 (except it's done in the analyzer).
>
> I am working on Clang together with Per and Anders.
>
> I am sorry.. I didn't see the work-in-progress for this. I believe the work-in-progress patch looks interesting.
>
> I have discussed this diagnose and was recommended that this diagnose is put in the static analyzer to begin with so it could be tried out.
> For 2 reasons:
>  * It is possibly a bit cpu expensive since it's parsing subexpressions recursively.

This is my main concern with the patch that I uploaded. I haven't
really measured, but it potentially does *a lot* of hashing of
expressions, so it requires more work.

>  * The signal noise is not fully known, having it in the static analyzer means people get the feeling about this.

I tried my patch on a Clang self-host and a Chromium build. The
signal-to-noise ratio was very good, and it found some real issues.

> It is true that this patch seems to be a subset. It is intended to be a start. We intentionally didn't include && and || to start with. If this checker is added it can be extended so it handles more operators later.
>
>> +Hans, since he has looked at this in the past.
>
> I wonder if Hans wants to continue his excellent work or if we can continue.

Please continue :) I think this is a good thing to check for, and it
seems reasonable to start it in the analyzer.

Feel free to look at my patch for inspiration. For example, I learned
that "x == x" is a common pattern for checking if a float is NaN, so
that shouldn't trigger a warning.

Thanks,
Hans




More information about the cfe-commits mailing list