[cfe-commits] r128894 - in /cfe/trunk: lib/Sema/AnalysisBasedWarnings.cpp test/Sema/uninit-variables.c test/SemaCXX/uninitialized.cpp

[Ted Kremenek]

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

> On Tue, Apr 5, 2011 at 11:11 AM, Ted Kremenek <kremenek at apple.com> 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.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110405/4797139f/attachment.html>