Maybe this is a bit paranoid, but I'm confused as to how we can only save one
`LastBindTemporary` value instead of a stack. What's the purpose of that
particular variable again? It seems to be used for more than just "if there are
no temporaries, we don't need a cleanup block".
================
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h:98-99
@@ -97,2 +97,4 @@
ExplodedNode *Pred);
+ void HandleBindTemporary(const CXXBindTemporaryExpr *BTE, const CFGBlock *B,
+ ExplodedNode *Pred);
----------------
Now that I think about it, this name is wrong. It's really
"HandleCleanupTemporaryBranch" or something. The CXXBindTemporaryExpr just
tells you which temporary it is. (And similar for similarly-named functions.)
================
Comment at: lib/Analysis/CFG.cpp:429-438
@@ -422,3 +428,12 @@
bool BindToTemporary);
+ // Adds cleanup logic for temporaries in case of a conditional path.
+ //
+ // 1. Does nothing if 'E' does not construct temporary objects.
+ // 2. Equivalent to VisitForTemporaryDtors if 'E' contains temporary
+ // destructors, but they are already handled in a branch due to a child
+ // statement of 'E'.
+ // 3. Otherwise, creates a new decision node that conditionally executes the
+ // temporary destructor and routes back to the current block on entry.
+ CFGBlock *VisitForTemporaryDtorsBranch(Stmt *E, bool BindToTemporary);
// NYS == Not Yet Supported
----------------
You might as well make this a proper doc comment.
================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:766-767
@@ +765,4 @@
+ if (getAnalysisManager().options.includeTemporaryDtorsInCFG()) {
+ VisitCXXBindTemporaryExpr(cast<CXXBindTemporaryExpr>(S), Pred,
PreVisit,
+ Next);
+ } else {
----------------
Calling this VisitCXXBindTemporaryExpr but then only having it actually be
called when the flag is set seems like a bad idea if anyone ever needs to add
behavior that should happen all the time. Either the check should be moved
inside the method or it should be called something else.
================
Comment at: test/Analysis/temporaries.cpp:316
@@ +315,3 @@
+ ++y;
+ if (true) (void)0; else (void)check(NoReturnDtor());
+ }
----------------
This and the `true ? ...` below will get optimized out at CFG construction
time. If that's correct, you should comment it; if you want to test path
sensitivity, you should take a boolean input variable and then fully constrain
it at the top of the function.
http://reviews.llvm.org/D3627
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
