[cfe-commits] Patch: Add warning for variable initialization that references the value of itself

[Ted Kremenek]

Hi Chandler, Richard,

I just noticed Chandler's commit.  I think this is a needed warning, but I'm a little concerned that the logic here is redundant.  Clang has a generic -Wuninitialized dataflow analysis, and it seems like cases like this should be caught there.  Doing an extra check seems like redundant work.  Was there a particular motivation to do the check this way?

I'm also mildly concerned that variable initialization is essentially a flow-sensitive property.  While contrived, the following case would falsely trigger warning from the logic added in r128376:

  int x = (x = 1, x);

While this is obviously contrived, my point is that reasoning about such cases doesn't require any explicit logic in the dataflow analysis; it just falls out.  While the dataflow analysis doesn't handle C++ initializers (yet) in constructors, when it does it will be able to handle control-flow dependencies between initializers as well.  My feeling is that since we already have a dataflow analysis for uninitialized variables, I'd rather focus on making that better than adding ad hoc checks for corner cases.

What do you guys think?

Ted

On Mar 27, 2011, at 2:53 AM, Chandler Carruth wrote:

> Thanks Richard. I've committed this after making a few tweaks in r128376.
> 
> For reference, the tweaks I made were:
> - Moved under -Wuninitialized and grouped the warning appropriately for that
> - Slightly tweaked the wording to flow better and match existing uninitialized warnings' wording
> - Only underline the identifier rather than the entire declaration. I think this looks a bit better and should avoid issues in cases like:
> 
> int x, y = y + 2;
> 
> - Some formatting tweaks here and there.
> 
> On Tue, Mar 22, 2011 at 6:43 PM, Richard Trieu <rtrieu at google.com> wrote:
> Add a warning on variable initialization which references its own value.  Non-evaluated expressions, such as address of or size of operations, are not warned on. 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> 
> 
> _______________________________________________
> 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/20110327/4b8e0c88/attachment.html>