On Apr 5, 2011, at 11:36 AM, Chandler Carruth wrote:

> On Tue, Apr 5, 2011 at 11:11 AM, Ted Kremenek <[email protected]> wrote:
> The problem is that the analysis currently treats 'x' to be initialized, 
> regardless of whatever is in the initializer.  This is done to halt the 
> propagation of uninitialized values warnings, and only report the earliest 
> instance.  The real fix probably needs to be done in the analysis 
> (UninitializedValues.cpp), not in the error reporting side in 
> AnalysisBasedWarnings.cpp.
> 
> Interesting! I didn't realize this subtlety, thanks for pointing it out. 
> Sorry for not adding a test case that would exercise it and seeing it, I 
> misread one of the existing tests as covering this... my bad on that front.
> 
> I'm looking at this and I wonder what the best way to do this is. Clearly we 
> need to not flag 'x' as initialized in UninitializedValues.cpp.
> 
> Do we want to avoid marking the initializer's reference an uninitialized use 
> inside of UninitializedValues.cpp and remove that logic from 
> AnalysisBasedWarnings?
> 
> Or do we want to just prevent the variable from being marked as initialized 
> in this particular case, and handle any and all suppression or other logic 
> inside AnalysisBasedWarnings?

I agree with you that we should take this approach.  It's about two lines of 
code in UninitializedValues.cpp.  It's a hack, but it's fairly localized.

> 
> I lean toward the latter as it keeps the model in the analyzer more pure. 
> Even this much special casing in UninitializedValues.cpp seems really 
> unfortunate. How important is continuing to warn on later uses of 'x'?

I think it is very important to warn.  Clearly the example I gave was a bug.  
While the code may have 'int x = x' (which is hopefully deliberate), and other 
uses of x afterwards are clearly a mistake.

> I think its probably worthwhile to have the analyzer propagate through this 
> one layer, but it seems a bit close.


I don't think we plan on adding a ton of special cases.  This one is highly 
idiomatic, so I'm fine with letting this one case through.

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

Reply via email to