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