dcoughlin added inline comments.
================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:2234
+ // should go away, but the assertion should remain, without the
+ // "DidCacheOutOnCleanup" part, of course.
+ bool DidCacheOutOnCleanup = false;
----------------
Can you add a comment explaining the criteria under which the
DidCacheOutOnCleanup part can be removed?
================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:2257
+ } else {
+ Pred = *CleanUpTemporaries.begin();
+ }
----------------
I realize this isn't new code in this patch, but can you use the result of
Bldr.generateNode() to determine whether we cached out and also to get the new
Pred? That seems cleaner than querying the ExplodedNodeSet.
================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:2262
+ assert(DidCacheOutOnCleanup ||
+ areInitializedTemporariesClear(Pred->getState(),
Pred->getLocationContext(),
----------------
It sounds like this means if we did cache out then there are places where the
initialized temporaries are not cleared (that is, we have extra gunk in the
state that we don't want).
Is that true? If so, then this relaxation of the assertion doesn't seem right
to me.
Do we need to introduce a new program point when calling `Bldr.generateNode` on
the cleaned up state (for example, with a new tag or a new program point kind)?
This would make it so that when we cache out when generating a node for the
state with the cleaned up temporaries we know that it is safe to early return
from processEndOfFunction(). It would be safe because we would know that the
other node (the one we cached out because of) has already had its temporaries
cleared and notified the checkers about the end of the function, etc.
Repository:
rC Clang
https://reviews.llvm.org/D43666
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
