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 <[email protected]> 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 <[email protected]> > 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:[email protected] <[email protected]>] > *Sent:* den 30 oktober 2013 19:25 > *To:* Jordan Rose; Ted Kremenek > *Cc:* Richard Trieu; Anders Rönnholm; [email protected] > *Subject:* Re: [PATCH] check for Incorrect logic in operator > > > On Oct 30, 2013, at 9:28 AM, Jordan Rose <[email protected]> wrote: > > > > On Oct 29, 2013, at 19:14 , Richard Trieu <[email protected]> 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 > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
