<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br><div><div>On Feb 5, 2013, at 14:00 , Ted Kremenek <<a href="mailto:kremenek@apple.com">kremenek@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">Author: kremenek<br>Date: Tue Feb 5 16:00:19 2013<br>New Revision: 174447<br><br>URL: <a href="http://llvm.org/viewvc/llvm-project?rev=174447&view=rev">http://llvm.org/viewvc/llvm-project?rev=174447&view=rev</a><br>Log:<br>Change subexpressions to be visited in the CFG from left-to-right.<br><br>This is a more natural order of evaluation, and it is very important<br>for visualization in the static analyzer. Within Xcode, the arrows<br>will not jump from right to left, which looks very visually jarring.<br>It also provides a more natural location for dataflow-based diagnostics.<br><br>Along the way, we found a case in the analyzer diagnostics where we<br>needed to indicate that a variable was "captured" by a block.<br><br>-fsyntax-only timings on sqlite3.c show no visible performance change,<br>although this is just one test case.<br><br>Fixes <<a href="rdar://problem/13016513">rdar://problem/13016513</a>><br><br>Modified:<br> cfe/trunk/include/clang/AST/Expr.h<br> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h<br> cfe/trunk/lib/Analysis/CFG.cpp<br> cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp<br> cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp<br> cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp<br> cfe/trunk/test/Analysis/unix-fns.c<br> cfe/trunk/test/Sema/warn-unreachable.c<br><br>Modified: cfe/trunk/include/clang/AST/Expr.h<br>URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=174447&r1=174446&r2=174447&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=174447&r1=174446&r2=174447&view=diff</a><br>==============================================================================<br>--- cfe/trunk/include/clang/AST/Expr.h (original)<br>+++ cfe/trunk/include/clang/AST/Expr.h Tue Feb 5 16:00:19 2013<br>@@ -2207,6 +2207,15 @@ public:<br> return SubExprs+PREARGS_START+getNumPreArgs()+getNumArgs();<br> }<br><br>+ /// This method provides fast access to all the subexpressions of<br>+ /// a CallExpr without going through the slower virtual child_iterator<br>+ /// interface. This provides efficient reverse iteration of the<br>+ /// subexpressions. This is currently used for CFG construction.<br>+ ArrayRef<Stmt*> getRawSubExprs() {<br>+ return ArrayRef<Stmt*>(SubExprs,<br>+ getNumPreArgs() + PREARGS_START + getNumArgs());<br>+ }<br>+<br> /// getNumCommas - Return the number of commas that must have been present in<br> /// this function call.<br> unsigned getNumCommas() const { return NumArgs ? NumArgs - 1 : 0; }<br><br>Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h<br>URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h?rev=174447&r1=174446&r2=174447&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h?rev=174447&r1=174446&r2=174447&view=diff</a><br>==============================================================================<br>--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h (original)<br>+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h Tue Feb 5 16:00:19 2013<br>@@ -662,6 +662,10 @@ public:<br> return *this;<br> }<br> };<br>+<br>+ /// Return the original region for a captured region, if<br>+ /// one exists.<br>+ const VarRegion *getOriginalRegion(const VarRegion *VR) const;<br><br> referenced_vars_iterator referenced_vars_begin() const;<br> referenced_vars_iterator referenced_vars_end() const; <br><br>Modified: cfe/trunk/lib/Analysis/CFG.cpp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=174447&r1=174446&r2=174447&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=174447&r1=174446&r2=174447&view=diff</a><br>==============================================================================<br>--- cfe/trunk/lib/Analysis/CFG.cpp (original)<br>+++ cfe/trunk/lib/Analysis/CFG.cpp Tue Feb 5 16:00:19 2013<br>@@ -233,6 +233,43 @@ public:<br> }<br> };<br><br>+class reverse_children {<br>+ llvm::SmallVector<Stmt *, 12> childrenBuf;<br>+ ArrayRef<Stmt*> children;<br>+public:<br>+ reverse_children(Stmt *S);<br>+<br>+ typedef ArrayRef<Stmt*>::reverse_iterator iterator;<br>+ iterator begin() const { return children.rbegin(); }<br>+ iterator end() const { return children.rend(); }<br>+};<br>+<br>+<br>+reverse_children::reverse_children(Stmt *S) {<br>+ if (CallExpr *CE = dyn_cast<CallExpr>(S)) {<br>+ children = CE->getRawSubExprs();<br>+ return;<br>+ }<br>+ switch (S->getStmtClass()) {<br>+ case Stmt::InitListExprClass: {<br>+ InitListExpr *IE = cast<InitListExpr>(S);<br>+ children = llvm::makeArrayRef(reinterpret_cast<Stmt**>(IE->getInits()),<br>+ IE->getNumInits());<br>+ return;<br>+ }<br>+ default:<br>+ break;<br>+ }<br>+<br>+ // Default case for all other statements.<br>+ for (Stmt::child_range I = S->children(); I; ++I) {<br>+ childrenBuf.push_back(*I);<br>+ }<br>+<br>+ // This needs to be done *after* childrenBuf has been populated.<br>+ children = childrenBuf;<br>+}<br>+<br> /// CFGBuilder - This class implements CFG construction from an AST.<br> /// The builder is stateful: an instance of the builder should be used to only<br> /// construct a single CFG.<br>@@ -1166,14 +1203,19 @@ CFGBlock *CFGBuilder::VisitStmt(Stmt *S,<br> }<br><br> /// VisitChildren - Visit the children of a Stmt.<br>-CFGBlock *CFGBuilder::VisitChildren(Stmt *Terminator) {<br>- CFGBlock *lastBlock = Block;<br>- for (Stmt::child_range I = Terminator->children(); I; ++I)<br>- if (Stmt *child = *I)<br>- if (CFGBlock *b = Visit(child))<br>- lastBlock = b;<br>+CFGBlock *CFGBuilder::VisitChildren(Stmt *S) {<br>+ CFGBlock *B = Block;<br><br>- return lastBlock;<br>+ // Visit the children in their reverse order so that they appear in<br>+ // left-to-right (natural) order in the CFG.<br>+ reverse_children RChildren(S);<br>+ for (reverse_children::iterator I = RChildren.begin(), E = RChildren.end();<br>+ I != E; ++I) {<br>+ if (Stmt *Child = *I)<br>+ if (CFGBlock *R = Visit(Child))<br>+ B = R;<br>+ }<br>+ return B;<br> }<br><br> CFGBlock *CFGBuilder::VisitAddrLabelExpr(AddrLabelExpr *A,<br>@@ -3093,19 +3135,14 @@ tryAgain:<br><br> CFGBlock *CFGBuilder::VisitChildrenForTemporaryDtors(Stmt *E) {<br> // When visiting children for destructors we want to visit them in reverse<br>- // order. Because there's no reverse iterator for children must to reverse<br>- // them in helper vector.<br>- typedef SmallVector<Stmt *, 4> ChildrenVect;<br>- ChildrenVect ChildrenRev;<br>- for (Stmt::child_range I = E->children(); I; ++I) {<br>- if (*I) ChildrenRev.push_back(*I);<br>- }<br>-<br>+ // order that they will appear in the CFG. Because the CFG is built<br>+ // bottom-up, this means we visit them in their natural order, which<br>+ // reverses them in the CFG.<br> CFGBlock *B = Block;<br>- for (ChildrenVect::reverse_iterator I = ChildrenRev.rbegin(),<br>- L = ChildrenRev.rend(); I != L; ++I) {<br>- if (CFGBlock *R = VisitForTemporaryDtors(*I))<br>- B = R;<br>+ for (Stmt::child_range I = E->children(); I; ++I) {<br>+ if (Stmt *Child = *I)<br>+ if (CFGBlock *R = VisitForTemporaryDtors(Child))<br>+ B = R;<br> }<br> return B;<br> }<br></blockquote><div><br></div><div>This is wrong. Destructors <i>must</i> be visited in reverse order of initializers, as required by the standard.</div><div><br></div><div>Jordan</div></div><br></body></html>