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

Reply via email to