[PATCH] check for Incorrect logic in operator

[Richard Trieu]

I'm fine with it for now.  We can use this to evaluate the difference
between CFG and Sema based warnings.  Eventually, I would like to see the
warnings in one place.


On Fri, Nov 8, 2013 at 11:13 AM, Anna Zaks <ganna at apple.com> wrote:

> Richard,
>
> Are you OK with this warning going on the CFG?
>
> Thanks,
> Anna.
>
> On Nov 7, 2013, at 2:42 AM, Anders Rönnholm <Anders.Ronnholm at evidente.se>
> wrote:
>
> I don't have any strong opinions of where to place the check. I will place
> it where you feel is the best place for it.
>
> I think I would be good if CFG could use it at least as it already has
> some checks to see if an expression always evaluates to true/false.
>
> //Anders
>
> *From:* Anna Zaks [mailto:ganna at apple.com <ganna at apple.com>]
> *Sent:* den 30 oktober 2013 19:25
> *To:* Jordan Rose; Ted Kremenek
> *Cc:* Richard Trieu; Anders Rönnholm; cfe-commits at cs.uiuc.edu
> *Subject:* Re: [PATCH] check for Incorrect logic in operator
>
>
> On Oct 30, 2013, at 9:28 AM, Jordan Rose <jordan_rose at apple.com> wrote:
>
>
>
> On Oct 29, 2013, at 19:14 , Richard Trieu <rtrieu at google.com> wrote:
>
>
> With these points in mind, are there particular concerns about cases where
> the “CFG won’t check all the code a Sema based warning would”.  If you
> addressed the last point, I think you’d pretty much get the coverage that
> you want.  What do you think?
>
>
> I think that globals and global initializers are not represented in the
> CFG.  That's my main concern about using only the CFG at the moment.
>
> I don't think it would be difficult to build a CFG from a single global
> initializer (or member initializer). No one's done it yet, but I don't
> think that means it can't be done.
>
> On the more general issue I can see both sides: avoiding extra walks over
> the AST or CFG is good, knowing what's trivially dead code is good (except
> when it isn't), and not conflating concerns is good.
>
> For these particular checks, though (and I haven't looked at the patch,
> just the checks), I think they are fundamentally syntactic checks, not
> flow-sensitive ones, and that we will actually get little benefit out of
> using this information to improve the CFG.
>
> We are still constructing a CFG after reporting these warnings, so it
> would be beneficial to feed the output of these checks into CFG
> construction. This way the CFG and CFG based warnings(and static analyzer
> checks) will be consistent with these warnings. Also, we could possibly
> extend the warning to show a note on code which becomes unreachable along
> with the diagnostic and extend it to cover more intricate flow sensitive
> cases in the future.
>
> If we can add the analysis of globals, there is no downside of performing
> the checks when we build the CFG as far as I can tell. Not sure if there
> are performance implications - do we always build the CFG?
>
>
> Each case is warning about a likely-incorrect boolean expression, which
> implies that if the user fixed the expression we could easily get a
> different CFG. That doesn't help answer questions of general policy, but it
> might at least untangle this patch from the discussion.
>
> Jordan
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131108/c581db15/attachment.html>