On Mon, Oct 28, 2013 at 11:47 AM, Ted Kremenek <[email protected]> wrote:
> On Oct 28, 2013, at 11:41 AM, Ted Kremenek <[email protected]> wrote: > > On Oct 25, 2013, at 4:42 PM, Richard Trieu <[email protected]> wrote: > > If you check out -Wlogical-not-parentheses and > -Wtautological-compare-out-of-range, they are in lib/Sema/SemaChecking.cpp. > Why did you use a CFG approach instead? > > > Hi Richard, > > I told Anders that modifying the CFG builder would be an interesting > approach for two reasons: > > (1) The CFG is used for a variety of dataflow analyses, thus folding this > “smarts” into the CFG builder seems to have dividends for other uses. > > (2) The CFG builder already does some branch pruning by doing some simply > analysis of expressions at branches. This seemed like a natural > enhancement. > > I did consider this to take a similar approach to > -Wlogical-not-parentheses, but thought this would be a more promising > direction to explore. > > Flipping the question around, do you think it would be interesting to > consider the opposite approach and move the logic to > -Wlogical-not-parentheses, etc., into the CFG builder, for the same reasons > I have given above? > > Ted > > > Further, there is a very natural relationship between *all* of these > warnings and -Wunreachable-code (a warning we’d one day like to fully > productize), which is built directly on top of the CFG. Bringing the > underlying pieces closer together seems like a promising direction to > explore. > My worry is that using the CFG won't check all the code a Sema based warning would (ignoring intentionally skipped dead/unreachable code). This would then require code in both the CFG and Sema for a warning to cover all cases. Clang already has this for the self-reference on initialization warning. If you have "int x = x + x;" in a function scope, the CFG-based uninitialized checker will produce the warning. If that same code is in a global scope, the AST-based check is performed in Sema. I think it's better to have the warning in one place, but that may differ by warning. Some warnings might benefit having its results be fed back into CFG, such as using -Wtautological-compare for branch analysis, although the enum part might need to be changed. Other warnings, such as warnings based on size would be good to silence on unreachable code. On the other hand, -Wlogical-not-parentheses doesn't provide any information to the CFG, and is almost always a true positive so warning on unreachable code is desirable.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
