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 <[email protected]> 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
> [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

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to