[cfe-dev] CFG initializers and destructors patch
Marcin Świderski
marcin.sfider at gmail.com
Tue Sep 14 23:41:25 PDT 2010
Thanks for looking through the patch. Commented on some of your comments
inline.
W dniu 15 września 2010 06:52 użytkownik Ted Kremenek
<kremenek at apple.com>napisał:
> On Sep 2, 2010, at 3:29 PM, Marcin Świderski wrote:
>
> > Hi
> >
> > I'm sending a patch with implementation of C++
> > - initializers from constructor initialization list,
> > - implicit destructors for objects with automatic storage duration.
> >
> > For destructors I've taken care of:
> > - block local scopes,
> > - if/switch/for/while/do local scopes (in case there's no block),
> > - if/switch/for/while condition variables,
> > - catch exception variable,
> > - temporaries bound to const reference.
> >
> > Is there something that I've missed?
> >
> > As it have been suggested I've created hierarchy of CFGElements.
> Currently there're two unused types for implicit destructor calls in
> destructor. I've did not revert ability of CFGElement to cast/dyn_cast to
> Stmt, because it would lead to situation when cast<Stmt>(SomeCFGElement)
> would return null. I did however add method for downcasting CFGElement
> object to object of its implemntation class (returned by value, not pointer)
> with returning invalid object on invalid cast.
> >
> > I did not add CFGElement for destructors of temporary objects. After
> giving it some thought I came to a conclusion that it shouldn't be covered
> by CFG, because it clearly needs path-sensitiveness.
> >
> > Cheers
> > Marcin
> >
>
> Comments inline below on the patch. Is the plan to still ad the
> CFGElements for the destructors of temporary objects? Overall, this is
> truly fantastic work. Sorry I took so long to look it over.
>
> One thing that would help is a general big comment (above LocalScopes
> perhaps) that explains the algorithm for adding destructors. I was able to
> tease it out of the code, but a high-level description would be really
> useful.
>
> One thing that occurred to me that while I argued against putting the scope
> information in the CFG itself, it looks like we do all the work to
> reconstruct scope information (via the LocalScope objects). Perhaps that
> information should be exposed as a public API? Just thinking (and not
> something to immediately consider).
>
> This could be done. However LocalScope graph construction process works
only on variables that will need destructor to be called. Adding all
variables for creating full scope info could break the graph. I'll include
comment in code describing the process, so it will be clear why.
> Index: include/clang/Analysis/FlowSensitive/DataflowSolver.h
> ===================================================================
> --- include/clang/Analysis/FlowSensitive/DataflowSolver.h (revision
> 112851)
> +++ include/clang/Analysis/FlowSensitive/DataflowSolver.h (working
> copy)
> @@ -273,8 +273,15 @@
> void ProcessBlock(const CFGBlock* B, bool recordStmtValues,
> dataflow::forward_analysis_tag) {
>
> - for (StmtItr I=ItrTraits::StmtBegin(B), E=ItrTraits::StmtEnd(B);
> I!=E;++I)
> - ProcessStmt(*I, recordStmtValues, AnalysisDirTag());
> + for (StmtItr I=ItrTraits::StmtBegin(B), E=ItrTraits::StmtEnd(B);
> I!=E;++I) {
> + if (const CFGStmt* SE = dyn_cast<const CFGStmt>(&*I))
> + ProcessStmt(*SE, recordStmtValues, AnalysisDirTag());
> + // FIXME: (CFGElement) Should handle other cases.
> + else if (const CFGInitializer* IE = dyn_cast<const
> CFGInitializer>(&*I))
> + (void)IE;
> + else if (const CFGImplicitDtor* DE = dyn_cast<const
> CFGImplicitDtor>(&*I))
> + (void)DE;
> + }
>
> TF.VisitTerminator(const_cast<CFGBlock*>(B));
> }
> @@ -284,8 +291,15 @@
>
> TF.VisitTerminator(const_cast<CFGBlock*>(B));
>
> - for (StmtItr I=ItrTraits::StmtBegin(B), E=ItrTraits::StmtEnd(B);
> I!=E;++I)
> - ProcessStmt(*I, recordStmtValues, AnalysisDirTag());
> + for (StmtItr I=ItrTraits::StmtBegin(B), E=ItrTraits::StmtEnd(B);
> I!=E;++I) {
> + if (const CFGStmt* SE = dyn_cast<const CFGStmt>(&*I))
> + ProcessStmt(*SE, recordStmtValues, AnalysisDirTag());
> + // FIXME: (CFGElement) Should handle other cases.
> + else if (const CFGInitializer* IE = dyn_cast<const
> CFGInitializer>(&*I))
> + (void)IE;
> + else if (const CFGImplicitDtor* DE = dyn_cast<const
> CFGImplicitDtor>(&*I))
> + (void)DE;
> + }
> }
>
>
> It would probably be more succinct to introduce a ProcessCFGElement(),
> which then in turn called ProcessStmt(). That way the two loops look like:
>
> - for (StmtItr I=ItrTraits::StmtBegin(B), E=ItrTraits::StmtEnd(B);
> I!=E;++I)
> - ProcessCFGElement(*I, recordStmtValues, AnalysisDirTag());
>
> and then have ProcessCFGElement do the dispatch logic. Right now this is
> basically copy-paste for the forward/backward analyses.
>
>
>
> void ProcessStmt(const Stmt* S, bool record,
> dataflow::forward_analysis_tag) {
> Index: include/clang/Analysis/Visitors/CFGRecStmtVisitor.h
> ===================================================================
> --- include/clang/Analysis/Visitors/CFGRecStmtVisitor.h (revision 112851)
> +++ include/clang/Analysis/Visitors/CFGRecStmtVisitor.h (working copy)
> @@ -47,6 +47,9 @@
>
> // Defining operator() allows the visitor to be used as a C++ style
> functor.
> void operator()(Stmt* S) {
> static_cast<ImplClass*>(this)->BlockStmt_Visit(S);}
> + void operator()(CFGElement E) {
> + static_cast<ImplClass*>(this)->VisitCFGElement(E);
> + }
> };
>
> Looks good.
>
>
>
> } // end namespace clang
> Index: include/clang/Analysis/Visitors/CFGStmtVisitor.h
> ===================================================================
> --- include/clang/Analysis/Visitors/CFGStmtVisitor.h (revision 112851)
> +++ include/clang/Analysis/Visitors/CFGStmtVisitor.h (working copy)
> @@ -61,6 +61,16 @@
> RetTy VisitConditionVariableInit(Stmt *S) {
> return RetTy();
> }
> +
> + /// VisitCFGElement - Handle CFGElements: Statements, Initializers and
> + /// ImplicitDtors. Those CFGElements act like statements from CFG point
> of
> + /// view.
> + RetTy VisitCFGElement(CFGElement E) {
> + if (CFGStmt S = E.getAs<CFGStmt>())
> + return BlockStmt_Visit(S);
> + // FIXME: (CFGElement) Handle Initializers and ImplicitDtors
> + return RetTy();
> + }
>
> Looks good.
>
> /// BlockVisit_XXX - Visitor methods for visiting the "root" statements
> in
> /// CFGBlocks. Root statements are the statements that appear explicitly
> in
> Index: include/clang/Analysis/CFG.h
> ===================================================================
> --- include/clang/Analysis/CFG.h (revision 112851)
> +++ include/clang/Analysis/CFG.h (working copy)
> @@ -28,30 +28,234 @@
> }
> namespace clang {
> class Decl;
> + class VarDecl;
> class Stmt;
> class Expr;
> + class CXXBaseOrMemberInitializer;
> + class CXXBindTemporaryExpr;
> + class CXXDestructorDecl;
> + class CXXExprWithTemporaries;
> + class FieldDecl;
> + class TypeSourceInfo;
> class CFG;
> + class CFGBlock;
> class PrinterHelper;
> class LangOptions;
> class ASTContext;
>
> -/// CFGElement - Represents a top-level expression in a basic block.
> +/// CFGElement - Represents a top-level expression or other language
> construct
> +/// in a basic block.
> class CFGElement {
> - llvm::PointerIntPair<Stmt *, 2> Data;
> +protected:
> + // ExternalData - External data sturcture used by implemntations to
> store
> + // more data then one pointer.
> + struct ExternalData {
> + llvm::PointerIntPair<void*, 2> PrimData;
> + llvm::PointerIntPair<void*, 2> SecData;
> + };
> +
> + ExternalData& getExternalData() {
> + return *static_cast<ExternalData*>(Data.getPointer());
> + }
> + const ExternalData& getExternalData() const {
> + return *static_cast<ExternalData*>(Data.getPointer());
> + }
> +
> + llvm::PointerIntPair<void*, 2> Data;
> +
> public:
> - enum Type { StartScope, EndScope };
> - explicit CFGElement() {}
> - CFGElement(Stmt *S, bool lvalue) : Data(S, lvalue ? 1 : 0) {}
> - CFGElement(Stmt *S, Type t) : Data(S, t == StartScope ? 2 : 3) {}
> - Stmt *getStmt() const { return Data.getPointer(); }
> - bool asLValue() const { return Data.getInt() == 1; }
> - bool asStartScope() const { return Data.getInt() == 2; }
> - bool asEndScope() const { return Data.getInt() == 3; }
> - bool asDtor() const { return Data.getInt() == 4; }
> + enum Kind {
> + Statement,
> + LvalStatement,
> + BEGIN_STATEMENTS = Statement,
> + END_STATEMENTS = LvalStatement,
> +
> + Initializer,
> +
> + AutomaticObjectDtor,
> + BaseDtor,
> + MemberDtor,
> + BEGIN_DTORS = AutomaticObjectDtor,
> + END_DTORS = MemberDtor,
> +
> + BEGIN_EXTERNALS = BEGIN_DTORS
> + };
> +
> + // Construct invalid CFGElement. Each implementation will provide
> default
> + // constructor for constructing invalid instance. Invalid instance won't
> + // preserve it's kind.
> + CFGElement() {}
> +
> + Kind getKind() const {
> + if (Data.getInt() != BEGIN_EXTERNALS)
> + return Kind(Data.getInt());
> + return Kind(BEGIN_EXTERNALS + getExternalData().PrimData.getInt());
> + }
> +
> + bool isValid() const { return Data.getPointer(); }
> + operator bool() const { return isValid(); };
> +
> + bool isStmt() const {
> + Kind K = Kind(Data.getInt());
> + return K >= BEGIN_STATEMENTS && K <= END_STATEMENTS;
> + }
> +
> + bool isInitializer() const {
> + return Data.getInt() == Initializer;
> + }
> +
> + bool isImplicitDtor() const {
> + if (Data.getInt() != BEGIN_EXTERNALS)
> + return false;
> + Kind K = Kind(BEGIN_EXTERNALS + getExternalData().PrimData.getInt());
> + return K >= BEGIN_DTORS && K <= END_DTORS;
> + }
> +
> + template<class ElemTy> ElemTy getAs() const {
> + if (llvm::isa<ElemTy>(this))
> + return *static_cast<const ElemTy*>(this);
>
> Could we use llvm::dyn_cast<> here? You already implement the classof()
> functions.
>
> You mean?:
if (ElemTy *E = llvm::dyn_cast<ElemTy>(this))
return *E;
We could, no real difference for me.
> + return ElemTy();
> + }
>
>
> +
> + static bool classof(const CFGElement* E) { return true; }
> +
> +protected:
> + CFGElement(Kind K, void* P)
> + : Data(P, K) {
> + assert (K < BEGIN_EXTERNALS && "CFGElement needs external data.");
> + }
> +
> + CFGElement(Kind K, void* P, void* S, llvm::BumpPtrAllocator& A)
> + : Data(new (A.Allocate<ExternalData>()) ExternalData(),
> BEGIN_EXTERNALS) {
> + assert (K >= BEGIN_EXTERNALS && "CFGElement doesn't need external
> data.");
> + getExternalData().PrimData.setInt(K - BEGIN_EXTERNALS);
> + getExternalData().PrimData.setPointer(P);
> + getExternalData().SecData.setPointer(S);
> + }
> +};
>
> Looks great.
>
>
> +
> +/// CFGStmt - Represents a top-level expression in a basic block.
> +class CFGStmt : public CFGElement {
> +public:
> + CFGStmt() {}
> + CFGStmt(Stmt* S, bool asLValue)
> + : CFGElement(asLValue ? LvalStatement : Statement, S) {}
> +
> + Stmt* getStmt() const { return static_cast<Stmt*>(Data.getPointer()); }
> operator Stmt*() const { return getStmt(); }
> - operator bool() const { return getStmt() != 0; }
> +
> + bool asLValue() const { return getKind() == LvalStatement; }
> +
> + static bool classof(const CFGElement* E) { return E->isStmt(); }
> };
>
> Looks great.
>
>
> +/// CFGInitializer - Represents a C++ initializer in constructor
> initialization
> +/// list.
> +class CFGInitializer : public CFGElement {
> +public:
> + CFGInitializer() {}
> + CFGInitializer(CXXBaseOrMemberInitializer* I)
> + : CFGElement(Initializer, I) {}
> +
> + CXXBaseOrMemberInitializer* getInitializer() const {
> + return static_cast<CXXBaseOrMemberInitializer*>(Data.getPointer());
> + }
> + operator CXXBaseOrMemberInitializer*() const { return getInitializer();
> }
> +
> + static bool classof(const CFGElement* E) { return E->isInitializer(); }
> +};
>
> Looks great.
>
> +
> +/// CFGImplicitDtor - Represents implicit call to C++ object's destructor.
> This
> +/// is a base class for more specific implementations.
> +class CFGImplicitDtor : public CFGElement {
> +public:
> + CFGImplicitDtor() {}
> +
> + CXXDestructorDecl* getDestructor() const;
> + SourceLocation getCallLoc() const;
> + SourceLocation getObjectLoc() const;
>
> Please add doxygen comments for these two methods that return
> SourceLocations.
>
> +
> + static bool classof(const CFGElement* E) { return E->isImplicitDtor(); }
> +
> +protected:
> + CFGImplicitDtor(Kind K, void* P, void* S, llvm::BumpPtrAllocator& A)
> + : CFGElement(K, P, S, A) {}
> +};
>
> Looks good.
>
>
> +
> +/// CFGAutomaticObjDtor - Represents implicit call to destructor of C++
> object
> +/// with automatic storage duration.
> +class CFGAutomaticObjDtor : public CFGImplicitDtor {
> +public:
> + CFGAutomaticObjDtor() {}
> + CFGAutomaticObjDtor(VarDecl* VD, Stmt* S, llvm::BumpPtrAllocator& A)
> + : CFGImplicitDtor(AutomaticObjectDtor, VD, S, A) {}
> +
> + CXXDestructorDecl* getDestructor() const;
> + SourceLocation getCallLoc() const;
> + SourceLocation getObjectLoc() const;
>
> Please comment that we use our own dynamic-dispatch here for these methods
> since they are non-virtual and we don't want a vptr. When I first looked at
> this I thought this was a bug. I think it is very important to document the
> design decision here.
>
> +
> + VarDecl* getVarDecl() const {
> + return static_cast<VarDecl*>(getExternalData().PrimData.getPointer());
> + }
>
> Looks great.
>
> +
> + // Get statement end of which triggered the destructor call.
> + Stmt* getTriggerStmt() const {
> + return static_cast<Stmt*>(getExternalData().SecData.getPointer());
> + }
>
> Looks great. Make the comment a doxygen comment.
>
> +
> + static bool classof(const CFGElement* E) {
> + return E->getKind() == AutomaticObjectDtor;
> + }
> +};
> +
> +/// CFGBaseDtor - Represents implicit call to destructor of base class in
> +/// C++ class destructor.
> +class CFGBaseDtor : public CFGImplicitDtor {
> +public:
> + CFGBaseDtor() {}
> +
> + CXXDestructorDecl* getDestructor() const;
> + SourceLocation getCallLoc() const;
> + SourceLocation getObjectLoc() const;
>
> Please comment that we use our own dynamic-dispatch here for these methods
> since they are non-virtual and we don't want a vptr. When I first looked at
> this I thought this was a bug.
>
> +};
> +
> +/// CFGMemberDtor - Represents implicit call to destructor of member in
> +/// C++ class destructor.
> +class CFGMemberDtor : public CFGImplicitDtor {
> +public:
> + CFGMemberDtor() {}
> +
> + CXXDestructorDecl* getDestructor() const;
> + SourceLocation getCallLoc() const;
> + SourceLocation getObjectLoc() const;
>
> Please comment that we use our own dynamic-dispatch here for these methods
> since they are non-virtual and we don't want a vptr. When I first looked at
> this I thought this was a bug.
>
>
> +};
> +
> +// Dispatch methods from CFGImplicitDtor's interface.
> +#define CFG_IMPLICIT_DTOR_DISPATCH(Meth) \
> + assert (Data.getInt() == BEGIN_EXTERNALS && "Bad CFGElement kind"); \
> + Kind K = Kind(getExternalData().PrimData.getInt() + BEGIN_EXTERNALS); \
> + switch (K) { \
> + case AutomaticObjectDtor: \
> + return static_cast<const CFGAutomaticObjDtor*>(this)->Meth(); \
> + case BaseDtor: \
> + return static_cast<const CFGBaseDtor*>(this)->Meth(); \
> + default: \
> + assert (K == MemberDtor && "CFGElement not of a destructor kind");\
> + return static_cast<const CFGMemberDtor*>(this)->Meth(); \
> + }
> +
> +inline CXXDestructorDecl* CFGImplicitDtor::getDestructor() const {
> + CFG_IMPLICIT_DTOR_DISPATCH(getDestructor);
> +}
> +inline SourceLocation CFGImplicitDtor::getCallLoc() const {
> + CFG_IMPLICIT_DTOR_DISPATCH(getCallLoc);
> +}
> +inline SourceLocation CFGImplicitDtor::getObjectLoc() const {
> + CFG_IMPLICIT_DTOR_DISPATCH(getObjectLoc);
> +}
> +
> +#undef CFG_IMPLICIT_DTOR_DISPATCH
> +
>
> Cute.
>
> /// CFGBlock - Represents a single basic block in a source-level CFG.
> /// It consists of:
> ///
> @@ -77,11 +281,11 @@
> /// &&, || expression that uses result of && or ||, RHS
> ///
> class CFGBlock {
> - class StatementList {
> + class ElementList {
> typedef BumpVector<CFGElement> ImplTy;
> ImplTy Impl;
> public:
> - StatementList(BumpVectorContext &C) : Impl(C, 4) {}
> + ElementList(BumpVectorContext &C) : Impl(C, 4) {}
>
> Looks good.
>
> typedef std::reverse_iterator<ImplTy::iterator> iterator;
> typedef std::reverse_iterator<ImplTy::const_iterator> const_iterator;
> @@ -89,6 +293,11 @@
> typedef ImplTy::const_iterator
> const_reverse_iterator;
>
> void push_back(CFGElement e, BumpVectorContext &C) { Impl.push_back(e,
> C); }
> + reverse_iterator insert(reverse_iterator I, size_t Cnt, CFGElement E,
> + BumpVectorContext& C) {
> + return Impl.insert(I, Cnt, E, C);
> + }
> +
> CFGElement front() const { return Impl.back(); }
> CFGElement back() const { return Impl.front(); }
>
> @@ -110,8 +319,8 @@
> bool empty() const { return Impl.empty(); }
> };
>
> - /// Stmts - The set of statements in the basic block.
> - StatementList Stmts;
> + /// Elements - The set of elements in the basic block.
> + ElementList Elements;
>
> /// Label - An (optional) label that prefixes the executable
> /// statements in the block. When this variable is non-NULL, it is
> @@ -140,33 +349,33 @@
>
> public:
> explicit CFGBlock(unsigned blockid, BumpVectorContext &C)
> - : Stmts(C), Label(NULL), Terminator(NULL), LoopTarget(NULL),
> + : Elements(C), Label(NULL), Terminator(NULL), LoopTarget(NULL),
> BlockID(blockid), Preds(C, 1), Succs(C, 1) {}
> ~CFGBlock() {}
>
> - // Statement iterators
> - typedef StatementList::iterator iterator;
> - typedef StatementList::const_iterator const_iterator;
> - typedef StatementList::reverse_iterator reverse_iterator;
> - typedef StatementList::const_reverse_iterator
> const_reverse_iterator;
> + // Element iterators
> + typedef ElementList::iterator iterator;
> + typedef ElementList::const_iterator const_iterator;
> + typedef ElementList::reverse_iterator reverse_iterator;
> + typedef ElementList::const_reverse_iterator
> const_reverse_iterator;
>
> - CFGElement front() const { return Stmts.front();
> }
> - CFGElement back() const { return Stmts.back();
> }
> + CFGElement front() const { return
> Elements.front(); }
> + CFGElement back() const { return
> Elements.back(); }
>
> - iterator begin() { return Stmts.begin();
> }
> - iterator end() { return Stmts.end();
> }
> - const_iterator begin() const { return Stmts.begin();
> }
> - const_iterator end() const { return Stmts.end();
> }
> + iterator begin() { return
> Elements.begin(); }
> + iterator end() { return
> Elements.end(); }
> + const_iterator begin() const { return
> Elements.begin(); }
> + const_iterator end() const { return
> Elements.end(); }
>
> - reverse_iterator rbegin() { return
> Stmts.rbegin(); }
> - reverse_iterator rend() { return Stmts.rend();
> }
> - const_reverse_iterator rbegin() const { return
> Stmts.rbegin(); }
> - const_reverse_iterator rend() const { return Stmts.rend();
> }
> + reverse_iterator rbegin() { return
> Elements.rbegin(); }
> + reverse_iterator rend() { return
> Elements.rend(); }
> + const_reverse_iterator rbegin() const { return
> Elements.rbegin(); }
> + const_reverse_iterator rend() const { return
> Elements.rend(); }
>
> - unsigned size() const { return Stmts.size();
> }
> - bool empty() const { return Stmts.empty();
> }
> + unsigned size() const { return
> Elements.size(); }
> + bool empty() const { return
> Elements.empty(); }
>
> - CFGElement operator[](size_t i) const { return Stmts[i]; }
> + CFGElement operator[](size_t i) const { return Elements[i]; }
>
>
> All looks great.
>
>
> // CFG iterators
> typedef AdjacentBlocks::iterator
> pred_iterator;
> @@ -240,14 +449,24 @@
> }
>
> void appendStmt(Stmt* Statement, BumpVectorContext &C, bool asLValue) {
> - Stmts.push_back(CFGElement(Statement, asLValue), C);
> - }
> - void StartScope(Stmt* S, BumpVectorContext &C) {
> - Stmts.push_back(CFGElement(S, CFGElement::StartScope), C);
> + Elements.push_back(CFGStmt(Statement, asLValue), C);
> }
> - void EndScope(Stmt* S, BumpVectorContext &C) {
> - Stmts.push_back(CFGElement(S, CFGElement::EndScope), C);
> + void appendInitializer(CXXBaseOrMemberInitializer* I, BumpVectorContext&
> C) {
> + Elements.push_back(CFGInitializer(I), C);
> }
> +
> + // Destructors must be inserted in reversed order. So insertion is in
> two
> + // steps. First we prepare space for some number of elements, then we
> insert
> + // the elements begining at the last position in prepared space.
> + iterator beginAutomaticObjDtorsInsert(iterator I, size_t Cnt,
> + BumpVectorContext& C) {
> + return iterator(Elements.insert(I.base(), Cnt, CFGElement(), C));
> + }
> + iterator insertAutomaticObjDtor(iterator I, VarDecl* VD, Stmt* S,
> + BumpVectorContext& C) {
> + *I = CFGAutomaticObjDtor(VD, S, C.getAllocator());
> + return ++I;
> + }
>
> Looks great.
>
> };
>
>
> /// createBlock - Create a new block in the CFG. The CFG owns the block;
> /// the caller should not directly free it.
> @@ -321,11 +568,12 @@
>
> //===--------------------------------------------------------------------===//
>
> template <typename CALLBACK>
> - void VisitBlockStmts(CALLBACK& O) const {
> + void VisitCFGElements(CALLBACK& O) const {
> for (const_iterator I=begin(), E=end(); I != E; ++I)
> for (CFGBlock::const_iterator BI=(*I)->begin(), BE=(*I)->end();
> - BI != BE; ++BI)
> + BI != BE; ++BI) {
> O(*BI);
> + }
> }
>
> Cool.
>
>
>
> //===--------------------------------------------------------------------===//
> @@ -398,18 +646,6 @@
>
> namespace llvm {
>
> -/// Implement simplify_type for CFGElement, so that we can dyn_cast from
> -/// CFGElement to a specific Stmt class.
> -template <> struct simplify_type<const ::clang::CFGElement> {
> - typedef ::clang::Stmt* SimpleType;
> - static SimpleType getSimplifiedValue(const ::clang::CFGElement &Val) {
> - return Val.getStmt();
> - }
> -};
> -
> -template <> struct simplify_type< ::clang::CFGElement>
> - : public simplify_type<const ::clang::CFGElement> {};
> -
>
> Makes sense to remove, since we have getAs<> now and a much richer
> CFGElement hierarchy.
>
>
> // Traits for: CFGBlock
>
> template <> struct GraphTraits< ::clang::CFGBlock* > {
> Index: include/clang/Analysis/ProgramPoint.h
> ===================================================================
> --- include/clang/Analysis/ProgramPoint.h (revision 112851)
> +++ include/clang/Analysis/ProgramPoint.h (working copy)
> @@ -117,10 +117,6 @@
> const CFGBlock* B = getBlock();
> return B->empty() ? CFGElement() : B->front();
> }
> -
> - const Stmt *getFirstStmt() const {
> - return getFirstElement().getStmt();
> - }
>
> static bool classof(const ProgramPoint* Location) {
> return Location->getKind() == BlockEntranceKind;
> @@ -136,11 +132,6 @@
> return reinterpret_cast<const CFGBlock*>(getData1());
> }
>
> - const Stmt* getLastStmt() const {
> - const CFGBlock* B = getBlock();
> - return B->empty() ? CFGElement() : B->back();
> - }
> -
> const Stmt* getTerminator() const {
> return getBlock()->getTerminator();
> }
>
> Index: include/clang/Checker/PathSensitive/GRCoreEngine.h
> ===================================================================
> --- include/clang/Checker/PathSensitive/GRCoreEngine.h (revision 112851)
> +++ include/clang/Checker/PathSensitive/GRCoreEngine.h (working copy)
> @@ -259,7 +259,9 @@
>
> /// getStmt - Return the current block-level expression associated with
> /// this builder.
> - const Stmt* getStmt() const { return B[Idx]; }
> + const Stmt* getStmt() const {
> + return B[Idx].isStmt() ? B[Idx].getAs<CFGStmt>().getStmt() : NULL;
> + }
>
> Looks great.
>
>
> /// getBlock - Return the CFGBlock associated with the block-level
> expression
> /// of this builder.
> Index: lib/Analysis/CFG.cpp
> ===================================================================
> --- lib/Analysis/CFG.cpp (revision 112851)
> +++ lib/Analysis/CFG.cpp (working copy)
> @@ -52,6 +52,113 @@
> Kind k;
> };
>
> +/// LocalScope - List of automatic variables declared in current
> +/// local scope and position in previous local scope.
> +class LocalScope {
> +public:
> + typedef std::vector<VarDecl*> AutomaticVarsTy;
>
> A SmallVector might reduce overall malloc() traffic.
>
> + typedef AutomaticVarsTy::const_reverse_iterator AutomaticVarsIter;
> +
> + class const_iterator {
> + const LocalScope* Scope;
> + AutomaticVarsIter VarIter;
> +
> + // Guard for "valid" invalid operator.
> + static LocalScope GuardScope;
> +
> + public:
> + /// Create invalid iterator.
> + const_iterator()
> + : Scope(&GuardScope), VarIter(Scope->Vars.rbegin()) {}
> +
> + /// Create valid iterator.
> + const_iterator(const LocalScope* Scope, AutomaticVarsIter VarIter)
> + : Scope(Scope), VarIter(VarIter) {}
> +
> + VarDecl* operator*() const { return *VarIter; }
> + VarDecl* const* operator->() const { return &*VarIter; }
> +
> + const_iterator& operator++() {
> + ++VarIter;
> + if (VarIter == Scope->Vars.rend())
> + *this = Scope->Prev;
> + return *this;
> + }
> + const_iterator operator++(int) {
> + const_iterator prev = *this;
> + ++*this;
> + return prev;
> + }
> +
> + bool operator==(const const_iterator& rhs) const {
> + return Scope == rhs.Scope && VarIter == rhs.VarIter;
> + }
> + bool operator!=(const const_iterator& rhs) const {
> + return !(*this == rhs);
> + }
> +
> + int distance(const_iterator L);
> + friend int distance(const_iterator F, const_iterator L);
> + };
> +
> + friend class const_iterator;
> +
> +private:
> + // Automatic variables in order of declaration.
> + AutomaticVarsTy Vars;
> + // Iterator to variable in previous scope that was declared just before
> + // begin of this scope.
> + const_iterator Prev;
> +
> + // Creates invalid scope that can serve as guard scope for iterator.
> + LocalScope()
> + : Vars(1, NULL)
> + , Prev(this, Vars.rbegin()) {}
> +
> +public:
> + LocalScope(const_iterator P)
> + : Vars()
> + , Prev(P) {}
> +
> + // Begin of scope in direction of CFG building (backwards).
> + const_iterator begin() const { return const_iterator(this,
> Vars.rbegin()); }
> +
> + void addVar(VarDecl* VD) {
> + Vars.push_back(VD);
> + }
> +};
> +
> +LocalScope LocalScope::const_iterator::GuardScope;
> +
> +/// distance - Calculates distance from F to L. L must be reachable from F
> (with
> +/// use of ++ operator). Cost of calculating the distance is linear w.r.t.
> +/// number between F and L.
> +int distance(LocalScope::const_iterator F, LocalScope::const_iterator L) {
> + return F.distance(L);
> +}
> +
> +int LocalScope::const_iterator::distance(LocalScope::const_iterator L) {
> + int D = 0;
> + const_iterator F = *this;
> + while (F.Scope != L.Scope) {
> + assert (F.Scope != &GuardScope
> + && "L iterator is not reachable from F iterator.");
> + D += F.Scope->Vars.rend() - F.VarIter;
> + F = F.Scope->Prev;
> + }
> + D += L.VarIter - F.VarIter;
> + return D;
> +}
> +
> +struct BlockScopePosPair {
> + BlockScopePosPair() {}
> + BlockScopePosPair(CFGBlock* B, LocalScope::const_iterator S)
> + : Block(B), ScopePos(S) {}
> +
> + CFGBlock* Block;
> + LocalScope::const_iterator ScopePos;
> +};
> +
> /// 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.
> @@ -67,41 +174,59 @@
> /// implicit fall-throughs without extra basic blocks.
> ///
> class CFGBuilder {
> + typedef BlockScopePosPair JumpTarget;
> + typedef BlockScopePosPair JumpSource;
> +
> ASTContext *Context;
> llvm::OwningPtr<CFG> cfg;
>
> CFGBlock* Block;
> CFGBlock* Succ;
> - CFGBlock* ContinueTargetBlock;
> - CFGBlock* BreakTargetBlock;
> + JumpTarget ContinueJumpTarget;
> + JumpTarget BreakJumpTarget;
> CFGBlock* SwitchTerminatedBlock;
> CFGBlock* DefaultCaseBlock;
> CFGBlock* TryTerminatedBlock;
>
> - // LabelMap records the mapping from Label expressions to their blocks.
> - typedef llvm::DenseMap<LabelStmt*,CFGBlock*> LabelMapTy;
> + // Current position in local scope.
> + LocalScope::const_iterator ScopePos;
> +
> + // LabelMap records the mapping from Label expressions to their jump
> targets.
> + typedef llvm::DenseMap<LabelStmt*, JumpTarget> LabelMapTy;
> LabelMapTy LabelMap;
>
> // A list of blocks that end with a "goto" that must be backpatched to
> their
> // resolved targets upon completion of CFG construction.
> - typedef std::vector<CFGBlock*> BackpatchBlocksTy;
> + typedef std::vector<JumpSource> BackpatchBlocksTy;
> BackpatchBlocksTy BackpatchBlocks;
>
> // A list of labels whose address has been taken (for indirect gotos).
> typedef llvm::SmallPtrSet<LabelStmt*,5> LabelSetTy;
> LabelSetTy AddressTakenLabels;
>
> + // A list of local scopes, which must be destroyed at the end of build
> + // process.
> + typedef std::vector<LocalScope*> LocalScopesTy;
> + LocalScopesTy LocalScopes;
>
> Consider making this a SmallVector to reduce malloc() traffic.
>
> +
> public:
> explicit CFGBuilder() : cfg(new CFG()), // crew a new CFG
> Block(NULL), Succ(NULL),
> - ContinueTargetBlock(NULL),
> BreakTargetBlock(NULL),
> + ContinueJumpTarget(), BreakJumpTarget(),
> SwitchTerminatedBlock(NULL),
> DefaultCaseBlock(NULL),
> - TryTerminatedBlock(NULL) {}
> + TryTerminatedBlock(NULL), ScopePos() {}
>
> + ~CFGBuilder() {
> + // Clean up local scopes.
> + for (LocalScopesTy::iterator I = LocalScopes.begin(), E =
> LocalScopes.end()
> + ; I != E; ++I) {
> + delete *I;
>
> Another possibility is to make the LocalScopes be BumpVectors, and
> BumpPtrAllocate them. Then you don't need to free them explicitly, and it
> reduces malloc() traffic.
>
> BumpVector with BumpPtrAllocator handled by CFG or CFGBuilder? I would
assume that the first case would be slightly faster, but would leave some
unused data after build process.
> + }
> + }
> +
> // buildCFG - Used by external clients to construct the CFG.
> CFG* buildCFG(const Decl *D, Stmt *Statement, ASTContext *C,
> - bool pruneTriviallyFalseEdges, bool AddEHEdges,
> - bool AddScopes);
> + CFG::BuildOptions BO);
>
> private:
> // Visitors to walk an AST and construct the CFG.
> @@ -150,37 +275,42 @@
> return Block;
> }
>
> void autoCreateBlock() { if (!Block) Block = createBlock(); }
> CFGBlock *createBlock(bool add_successor = true);
> bool FinishBlock(CFGBlock* B);
> +
> CFGBlock *addStmt(Stmt *S) {
> return Visit(S, AddStmtChoice::AlwaysAdd);
> }
> + CFGBlock *addInitializer(CXXBaseOrMemberInitializer* I);
> + CFGBlock *addAutomaticObjDtors(LocalScope::const_iterator B,
> + LocalScope::const_iterator E, Stmt* S);
>
> + // Local scopes creation.
> + LocalScope* createOrReuseLocalScope(LocalScope* Scope);
> +
> + LocalScope* addLocalScopeForStmt(Stmt* S, LocalScope* Scope = NULL);
> + LocalScope* addLocalScopeForDeclStmt(DeclStmt* DS, LocalScope* Scope =
> NULL);
> + LocalScope* addLocalScopeForVarDecl(VarDecl* VD, LocalScope* Scope =
> NULL);
> +
> + LocalScope* addLocalScopeAndDtors(Stmt* S, LocalScope* Scope = NULL);
> +
> + // Interface to CFGBlock - adding CFGElements.
> void AppendStmt(CFGBlock *B, Stmt *S,
> AddStmtChoice asc = AddStmtChoice::AlwaysAdd) {
> B->appendStmt(S, cfg->getBumpVectorContext(), asc.asLValue());
> }
> + void appendInitializer(CFGBlock* B, CXXBaseOrMemberInitializer* I) {
> + B->appendInitializer(I, cfg->getBumpVectorContext());
> + }
>
> + void insertAutomaticObjDtors(CFGBlock* Blk, CFGBlock::iterator I,
> + LocalScope::const_iterator B, LocalScope::const_iterator E, Stmt* S);
> + void appendAutomaticObjDtors(CFGBlock* Blk, LocalScope::const_iterator
> B,
> + LocalScope::const_iterator E, Stmt* S);
> + void prependAutomaticObjDtorsWithTerminator(CFGBlock* Blk,
> + LocalScope::const_iterator B, LocalScope::const_iterator E);
> +
> void AddSuccessor(CFGBlock *B, CFGBlock *S) {
> B->addSuccessor(S, cfg->getBumpVectorContext());
> }
> @@ -207,7 +337,7 @@
> /// TryEvaluateBool - Try and evaluate the Stmt and return 0 or 1
> /// if we can evaluate to a known value, otherwise return -1.
> TryResult TryEvaluateBool(Expr *S) {
> - if (!PruneTriviallyFalseEdges)
> + if (!BuildOpts.PruneTriviallyFalseEdges)
> return TryResult();
>
> Expr::EvalResult Result;
> @@ -219,15 +349,7 @@
> }
>
> bool badCFG;
> };
>
> // FIXME: Add support for dependent-sized array types in C++?
> @@ -250,19 +372,17 @@
> /// transferred to the caller. If CFG construction fails, this method
> returns
> /// NULL.
> CFG* CFGBuilder::buildCFG(const Decl *D, Stmt* Statement, ASTContext* C,
> - bool pruneTriviallyFalseEdges,
> - bool addehedges, bool AddScopes) {
> + CFG::BuildOptions BO) {
>
> - AddEHEdges = addehedges;
> - PruneTriviallyFalseEdges = pruneTriviallyFalseEdges;
> -
> Context = C;
> assert(cfg.get());
> if (!Statement)
> return NULL;
>
> - this->AddScopes = AddScopes;
> badCFG = false;
> + BuildOpts = BO;
> + if (!C->getLangOptions().CPlusPlus)
> + BuildOpts.AddImplicitDtors = false;
>
> // Create an empty block that will serve as the exit block for the CFG.
> Since
> // this is the first block added to the CFG, it will be implicitly
> registered
> @@ -274,9 +394,14 @@
> // Visit the statements and create the CFG.
> CFGBlock* B = addStmt(Statement);
>
> + // For C++ constructor add initializers to CFG.
> if (const CXXConstructorDecl *CD =
> dyn_cast_or_null<CXXConstructorDecl>(D)) {
> - // FIXME: Add code for base initializers and member initializers.
> - (void)CD;
> + for (CXXConstructorDecl::init_const_reverse_iterator I =
> CD->init_rbegin()
> + , E = CD->init_rend(); I != E; ++I) {
> + B = addInitializer(*I);
> + if (badCFG)
> + return NULL;
> + }
> }
> if (!B)
> B = Succ;
> @@ -291,15 +416,17 @@
> for (BackpatchBlocksTy::iterator I = BackpatchBlocks.begin(),
> E = BackpatchBlocks.end(); I != E; ++I ) {
>
> - CFGBlock* B = *I;
> + CFGBlock* B = I->Block;
> GotoStmt* G = cast<GotoStmt>(B->getTerminator());
> LabelMapTy::iterator LI = LabelMap.find(G->getLabel());
>
> // If there is no target for the goto, then we are looking at an
> // incomplete AST. Handle this by not registering a successor.
> if (LI == LabelMap.end()) continue;
> + JumpTarget JT = LI->second;
>
> - AddSuccessor(B, LI->second);
> + prependAutomaticObjDtorsWithTerminator(B, I->ScopePos, JT.ScopePos);
> + AddSuccessor(B, JT.Block);
> }
>
> // Add successors to the Indirect Goto Dispatch block (if we have one).
> @@ -314,7 +441,9 @@
> // at an incomplete AST. Handle this by not registering a
> successor.
> if (LI == LabelMap.end()) continue;
>
> - AddSuccessor(B, LI->second);
> + // FIXME: Are indirect gotos supported in C++? If yes we have to
> add
> + // additional block for each possible branch with destructors
> called.
> + AddSuccessor(B, LI->second.Block);
>
>
> How horrible to consider. I don't know the answer to this question.
>
>
> }
>
> Succ = B;
> @@ -344,6 +473,148 @@
> return true;
> }
>
> +/// addInitializer - Add C++ base or member initializer element to CFG.
> +CFGBlock* CFGBuilder::addInitializer(CXXBaseOrMemberInitializer* I) {
> + if (!BuildOpts.AddInitializers)
> + return Block;
> +
> + autoCreateBlock();
> + appendInitializer(Block, I);
> +
> + if (!I->getInit())
> + // FIXME: Remove this check if is unnecessery.
> + return Block;
> +
> + return addStmt(I->getInit());
> +}
>
> Looks good.
>
> +
> +CFGBlock* CFGBuilder::addAutomaticObjDtors(LocalScope::const_iterator B,
> + LocalScope::const_iterator E, Stmt* S) {
> + if (!BuildOpts.AddImplicitDtors)
> + return Block;
> + if (B == E)
> + return Block;
> +
> + autoCreateBlock();
> + appendAutomaticObjDtors(Block, B, E, S);
> + return Block;
> +}
>
> Looks good.
>
> +
> +LocalScope* CFGBuilder::createOrReuseLocalScope(LocalScope* Scope) {
> + if (!Scope) {
> + Scope = new LocalScope(ScopePos);
> + LocalScopes.push_back(Scope);
> + }
> + return Scope;
> +}
>
> Consider BumpPtrAllocating LocalScope, since we may create a lot of them.
>
> +
> +LocalScope* CFGBuilder::addLocalScopeForStmt(Stmt* S, LocalScope* Scope) {
> + if (!BuildOpts.AddImplicitDtors)
> + return Scope;
> +
> + // For compound statement we will be creating explicit scope.
> + if (CompoundStmt* CS = dyn_cast<CompoundStmt>(S)) {
> + for (CompoundStmt::body_iterator BI = CS->body_begin(), BE =
> CS->body_end()
> + ; BI != BE; ++BI) {
> + if (DeclStmt* DS = dyn_cast<DeclStmt>(*BI))
> + Scope = addLocalScopeForDeclStmt(DS, Scope);
> + }
> + // For any other statement scope will be implicit and as such will be
> + // interesting only for DeclStmt.
> + } else if (DeclStmt* DS = dyn_cast<DeclStmt>(S)) {
> + Scope = addLocalScopeForDeclStmt(DS, Scope);
> + }
> + return Scope;
> +}
>
> Looks great.
>
> +
> +LocalScope* CFGBuilder::addLocalScopeForDeclStmt(DeclStmt* DS,
> + LocalScope* Scope) {
> + if (!BuildOpts.AddImplicitDtors)
> + return Scope;
> +
> + for (DeclStmt::decl_iterator DI = DS->decl_begin(), DE = DS->decl_end()
> + ; DI != DE; ++DI) {
> + if (VarDecl* VD = dyn_cast<VarDecl>(*DI))
> + Scope = addLocalScopeForVarDecl(VD, Scope);
> + }
> + return Scope;
> +}
>
> Looks great. Does each VarDecl get it's own scope?
>
Scope variable passed and returned from addLocalScopeForVarDecl is for lazy
creating new scope when needed. So when created it will be used for more
variables, but we won't create scope if no variables will be added to it.
>
> +
> +LocalScope* CFGBuilder::addLocalScopeForVarDecl(VarDecl* VD,
> + LocalScope* Scope) {
> + if (!BuildOpts.AddImplicitDtors)
> + return Scope;
> +
> + // Check if variable is local.
> + switch (VD->getStorageClass()) {
> + case SC_None:
> + case SC_Auto:
> + case SC_Register:
> + break;
> + default: return Scope;
> + }
>
> Does the 'default' case handle static variables?
>
> +
> + // Check for const references bound to temporary. Set type to pointee.
> + QualType QT = VD->getType();
> + if (const ReferenceType* RT = QT.getTypePtr()->getAs<ReferenceType>()) {
> + QT = RT->getPointeeType();
> + if (!QT.isConstQualified() ||
> !VD->getInit()->Classify(*Context).isRValue())
> + return Scope;
> + }
> +
> + // Check if type is a C++ class with non-trivial destructor.
> + const RecordType* RT = QT.getTypePtr()->getAs<RecordType>();
> + if (!RT || cast<CXXRecordDecl>(RT->getDecl())->hasTrivialDestructor())
> + return Scope;
> +
> + // Add the variable to scope
> + Scope = createOrReuseLocalScope(Scope);
> + Scope->addVar(VD);
> + ScopePos = Scope->begin();
> + return Scope;
> +}
>
> Wow.
>
> +
> +/// addLocalScopeAndDtors - For given statement add local scope for it and
> +/// add destructors that will cleanup the scope.
> +LocalScope* CFGBuilder::addLocalScopeAndDtors(Stmt* S, LocalScope* Scope)
> {
> + if (!BuildOpts.AddImplicitDtors)
> + return Scope;
> +
> + LocalScope::const_iterator scopeBeginPos = ScopePos;
> + Scope = addLocalScopeForStmt(S, Scope);
> + addAutomaticObjDtors(ScopePos, scopeBeginPos, S);
> + return Scope;
> +}
>
> Awesome.
>
> +
> +/// insertAutomaticObjDtors - Insert destructor CFGElements for variables
> with
> +/// automatic storage duration to CFGBlock's elements vector. Insertion
> will be
> +/// performed in place specified with iterator.
> +void CFGBuilder::insertAutomaticObjDtors(CFGBlock* Blk, CFGBlock::iterator
> I,
> + LocalScope::const_iterator B, LocalScope::const_iterator E, Stmt* S) {
> + BumpVectorContext& C = cfg->getBumpVectorContext();
> + I = Blk->beginAutomaticObjDtorsInsert(I, distance(B, E), C);
> + while (B != E)
> + I = Blk->insertAutomaticObjDtor(I, *B++, S, C);
> +}
> +
> +/// appendAutomaticObjDtors - Append destructor CFGElements for variables
> with
> +/// automatic storage duration to CFGBlock's elements vector. Elements
> will be
> +/// appended to physical end of the vector which happens to be logical
> begining.
> +void CFGBuilder::appendAutomaticObjDtors(CFGBlock* Blk,
> + LocalScope::const_iterator B, LocalScope::const_iterator E, Stmt* S) {
> + insertAutomaticObjDtors(Blk, Blk->begin(), B, E, S);
> +}
> +
> +/// prependAutomaticObjDtorsWithTerminator - Prepend destructor
> CFGElements for
> +/// variables with automatic storage duration to CFGBlock's elements
> vector.
> +/// Elememnts will be prepended to physical begining of the vector which
> happens
>
> Typo: 'Elements'
>
> +/// to be logical end. Use blocks terminator as statement that specifies
> +/// destructors call site.
> +void CFGBuilder::prependAutomaticObjDtorsWithTerminator(CFGBlock* Blk,
> + LocalScope::const_iterator B, LocalScope::const_iterator E) {
> + insertAutomaticObjDtors(Blk, Blk->end(), B, E, Blk->getTerminator());
> +}
> +
> /// Visit - Walk the subtree of a statement and add extra
> /// blocks for ternary operators, &&, and ||. We also process "," and
> /// DeclStmts (which may contain nested control-flow).
> @@ -589,9 +860,10 @@
>
> // If there is no target for the break, then we are looking at an
> incomplete
> // AST. This means that the CFG cannot be constructed.
> - if (BreakTargetBlock)
> - AddSuccessor(Block, BreakTargetBlock);
> - else
> + if (BreakJumpTarget.Block) {
> + addAutomaticObjDtors(ScopePos, BreakJumpTarget.ScopePos, B);
> + AddSuccessor(Block, BreakJumpTarget.Block);
> + } else
> badCFG = true;
>
>
> @@ -625,7 +897,7 @@
>
> // Languages without exceptions are assumed to not throw.
> if (Context->getLangOptions().Exceptions) {
> - if (AddEHEdges)
> + if (BuildOpts.AddEHEdges)
> AddEHEdge = true;
> }
>
> @@ -703,8 +975,7 @@
>
>
> CFGBlock* CFGBuilder::VisitCompoundStmt(CompoundStmt* C) {
> - EndScope(C);
> -
> + addLocalScopeAndDtors(C);
> CFGBlock* LastBlock = Block;
>
> for (CompoundStmt::reverse_body_iterator I=C->body_rbegin(),
> E=C->body_rend();
> @@ -718,8 +989,6 @@
> return NULL;
> }
>
> - LastBlock = StartScope(C, LastBlock);
> -
> return LastBlock;
> }
>
> @@ -846,6 +1115,10 @@
> VA = FindVA(VA->getElementType().getTypePtr()))
> Block = addStmt(VA->getSizeExpr());
>
> + // Remove variable from local scope.
> + if (VD == *ScopePos)
> + ++ScopePos;
> +
> return Block;
> }
>
> @@ -857,6 +1130,18 @@
> // middle of a block, we stop processing that block. That block is then
> the
> // implicit successor for the "then" and "else" clauses.
>
> + // Save local scope position because in case of condition variable
> ScopePos
> + // won't be restored when traversing AST.
> + SaveAndRestore<LocalScope::const_iterator> save_scope_pos(ScopePos);
> +
> + // Create local scope for possible condition variable.
> + // Store scope position. Add implicit destructor.
> + if (VarDecl* VD = I->getConditionVariable()) {
> + LocalScope::const_iterator BeginScopePos = ScopePos;
> + addLocalScopeForVarDecl(VD);
> + addAutomaticObjDtors(ScopePos, BeginScopePos, I);
> + }
> +
> // The block we were proccessing is now finished. Make it the successor
> // block.
> if (Block) {
> @@ -874,6 +1159,12 @@
> // NULL out Block so that the recursive call to Visit will
> // create a new basic block.
> Block = NULL;
> +
> + // If branch is not a compound statement create implicit scope
> + // and add destructors.
> + if (!isa<CompoundStmt>(Else))
> + addLocalScopeAndDtors(Else);
> +
> ElseBlock = addStmt(Else);
>
> if (!ElseBlock) // Can occur when the Else body has all NullStmts.
> @@ -891,6 +1182,12 @@
> assert(Then);
> SaveAndRestore<CFGBlock*> sv(Succ);
> Block = NULL;
> +
> + // If branch is not a compound statement create implicit scope
> + // and add destructors.
> + if (!isa<CompoundStmt>(Then))
> + addLocalScopeAndDtors(Then);
> +
> ThenBlock = addStmt(Then);
>
> if (!ThenBlock) {
> @@ -949,6 +1246,7 @@
>
> // Create the new block.
> Block = createBlock(false);
> + addAutomaticObjDtors(ScopePos, LocalScope::const_iterator(), R);
>
> // The Exit block is the only successor.
> AddSuccessor(Block, &cfg->getExit());
> @@ -967,7 +1265,7 @@
> LabelBlock = createBlock(); // scopes that only contains NullStmts.
>
> assert(LabelMap.find(L) == LabelMap.end() && "label already in map");
> - LabelMap[ L ] = LabelBlock;
> + LabelMap[ L ] = JumpTarget(LabelBlock, ScopePos);
>
> // Labels partition blocks, so this is the end of the basic block we were
> // processing (L is the block's label). Because this is label (and we
> have
> @@ -1000,9 +1298,12 @@
>
> if (I == LabelMap.end())
> // We will need to backpatch this block later.
> - BackpatchBlocks.push_back(Block);
> - else
> - AddSuccessor(Block, I->second);
> + BackpatchBlocks.push_back(JumpSource(Block, ScopePos));
> + else {
> + JumpTarget JT = I->second;
> + addAutomaticObjDtors(ScopePos, JT.ScopePos, G);
> + AddSuccessor(Block, JT.Block);
> + }
>
> return Block;
> }
> @@ -1010,6 +1311,22 @@
> CFGBlock* CFGBuilder::VisitForStmt(ForStmt* F) {
> CFGBlock* LoopSuccessor = NULL;
>
> + // Save local scope position because in case of condition variable
> ScopePos
> + // won't be restored when traversing AST.
> + SaveAndRestore<LocalScope::const_iterator> save_scope_pos(ScopePos);
> +
> + // Create local scope for init statement and possible condition
> variable.
> + // Add destructor for init statement.
> + // Store scope position for continue statement.
> + if (Stmt* Init = F->getInit()) {
> + addLocalScopeForStmt(Init);
> + addAutomaticObjDtors(ScopePos, save_scope_pos.get(), F);
> + }
> +
> + LocalScope::const_iterator LoopBeginScopePos = ScopePos;
> + if (VarDecl* VD = F->getConditionVariable())
> + addLocalScopeForVarDecl(VD);
> +
> // "for" is a control-flow statement. Thus we stop processing the
> current
> // block.
> if (Block) {
> @@ -1021,8 +1338,8 @@
>
> // Save the current value for the break targets.
> // All breaks should go to the code following the loop.
> - SaveAndRestore<CFGBlock*> save_break(BreakTargetBlock);
> - BreakTargetBlock = LoopSuccessor;
> + SaveAndRestore<JumpTarget> save_break(BreakJumpTarget);
> + BreakJumpTarget = JumpTarget(LoopSuccessor, LoopBeginScopePos);
>
> // Because of short-circuit evaluation, the condition of the loop can
> span
> // multiple basic blocks. Thus we need the "Entry" and "Exit" blocks
> that
> @@ -1072,11 +1389,14 @@
> assert(F->getBody());
>
> // Save the current values for Block, Succ, and continue targets.
> - SaveAndRestore<CFGBlock*> save_Block(Block), save_Succ(Succ),
> - save_continue(ContinueTargetBlock);
> + SaveAndRestore<CFGBlock*> save_Block(Block), save_Succ(Succ);
> + SaveAndRestore<JumpTarget> save_continue(ContinueJumpTarget);
>
> // Create a new block to contain the (bottom) of the loop body.
> Block = NULL;
> +
> + // Loop body should end with destructor of Condition variable (if
> any).
> + addAutomaticObjDtors(ScopePos, LoopBeginScopePos, F);
>
> if (Stmt* I = F->getInc()) {
> // Generate increment code in its own basic block. This is the
> target of
> @@ -1086,7 +1406,7 @@
> // No increment code. Create a special, empty, block that is used as
> the
> // target block for "looping back" to the start of the loop.
> assert(Succ == EntryConditionBlock);
> - Succ = createBlock();
> + Succ = Block ? Block : createBlock();
> }
>
> // Finish up the increment (or empty) block if it hasn't been already.
> @@ -1097,18 +1417,23 @@
> Block = 0;
> }
>
> - ContinueTargetBlock = Succ;
> + ContinueJumpTarget = JumpTarget(Succ, LoopBeginScopePos);
>
> // The starting block for the loop increment is the block that should
> // represent the 'loop target' for looping back to the start of the
> loop.
> - ContinueTargetBlock->setLoopTarget(F);
> + ContinueJumpTarget.Block->setLoopTarget(F);
>
> + // If body is not a compound statement create implicit scope
> + // and add destructors.
> + if (!isa<CompoundStmt>(F->getBody()))
> + addLocalScopeAndDtors(F->getBody());
> +
> // Now populate the body block, and in the process create new blocks as
> we
> // walk the body of the loop.
> CFGBlock* BodyBlock = addStmt(F->getBody());
>
> if (!BodyBlock)
> - BodyBlock = ContinueTargetBlock; // can happen for "for
> (...;...;...) ;"
> + BodyBlock = ContinueJumpTarget.Block;//can happen for "for
> (...;...;...);"
> else if (Block && !FinishBlock(BodyBlock))
> return 0;
>
> @@ -1217,11 +1542,12 @@
> // Now create the true branch.
> {
> // Save the current values for Succ, continue and break targets.
> - SaveAndRestore<CFGBlock*> save_Succ(Succ),
> - save_continue(ContinueTargetBlock), save_break(BreakTargetBlock);
> + SaveAndRestore<CFGBlock*> save_Succ(Succ);
> + SaveAndRestore<JumpTarget> save_continue(ContinueJumpTarget),
> + save_break(BreakJumpTarget);
>
> - BreakTargetBlock = LoopSuccessor;
> - ContinueTargetBlock = EntryConditionBlock;
> + BreakJumpTarget = JumpTarget(LoopSuccessor, ScopePos);
> + ContinueJumpTarget = JumpTarget(EntryConditionBlock, ScopePos);
>
> CFGBlock* BodyBlock = addStmt(S->getBody());
>
> @@ -1273,6 +1599,16 @@
> CFGBlock* CFGBuilder::VisitWhileStmt(WhileStmt* W) {
> CFGBlock* LoopSuccessor = NULL;
>
> + // Save local scope position because in case of condition variable
> ScopePos
> + // won't be restored when traversing AST.
> + SaveAndRestore<LocalScope::const_iterator> save_scope_pos(ScopePos);
> +
> + // Create local scope for possible condition variable.
> + // Store scope position for continue statement.
> + LocalScope::const_iterator LoopBeginScopePos = ScopePos;
> + if (VarDecl* VD = W->getConditionVariable())
> + addLocalScopeForVarDecl(VD);
> +
> // "while" is a control-flow statement. Thus we stop processing the
> current
> // block.
> if (Block) {
> @@ -1328,9 +1664,9 @@
> assert(W->getBody());
>
> // Save the current values for Block, Succ, and continue and break
> targets
> - SaveAndRestore<CFGBlock*> save_Block(Block), save_Succ(Succ),
> - save_continue(ContinueTargetBlock),
> - save_break(BreakTargetBlock);
> + SaveAndRestore<CFGBlock*> save_Block(Block), save_Succ(Succ);
> + SaveAndRestore<JumpTarget> save_continue(ContinueJumpTarget),
> + save_break(BreakJumpTarget);
>
> // Create an empty block to represent the transition block for looping
> back
> // to the head of the loop.
> @@ -1338,19 +1674,27 @@
> assert(Succ == EntryConditionBlock);
> Succ = createBlock();
> Succ->setLoopTarget(W);
> - ContinueTargetBlock = Succ;
> + ContinueJumpTarget = JumpTarget(Succ, LoopBeginScopePos);
>
> // All breaks should go to the code following the loop.
> - BreakTargetBlock = LoopSuccessor;
> + BreakJumpTarget = JumpTarget(LoopSuccessor, LoopBeginScopePos);
>
> // NULL out Block to force lazy instantiation of blocks for the body.
> Block = NULL;
>
> + // Loop body should end with destructor of Condition variable (if
> any).
> + addAutomaticObjDtors(ScopePos, LoopBeginScopePos, W);
> +
> + // If body is not a compound statement create implicit scope
> + // and add destructors.
> + if (!isa<CompoundStmt>(W->getBody()))
> + addLocalScopeAndDtors(W->getBody());
> +
> // Create the body. The returned block is the entry to the loop body.
> CFGBlock* BodyBlock = addStmt(W->getBody());
>
> if (!BodyBlock)
> - BodyBlock = ContinueTargetBlock; // can happen for "while(...) ;"
> + BodyBlock = ContinueJumpTarget.Block; // can happen for "while(...)
> ;"
> else if (Block) {
> if (!FinishBlock(BodyBlock))
> return 0;
> @@ -1463,19 +1807,24 @@
> assert(D->getBody());
>
> // Save the current values for Block, Succ, and continue and break
> targets
> - SaveAndRestore<CFGBlock*> save_Block(Block), save_Succ(Succ),
> - save_continue(ContinueTargetBlock),
> - save_break(BreakTargetBlock);
> + SaveAndRestore<CFGBlock*> save_Block(Block), save_Succ(Succ);
> + SaveAndRestore<JumpTarget> save_continue(ContinueJumpTarget),
> + save_break(BreakJumpTarget);
>
> // All continues within this loop should go to the condition block
> - ContinueTargetBlock = EntryConditionBlock;
> + ContinueJumpTarget = JumpTarget(EntryConditionBlock, ScopePos);
>
> // All breaks should go to the code following the loop.
> - BreakTargetBlock = LoopSuccessor;
> + BreakJumpTarget = JumpTarget(LoopSuccessor, ScopePos);
>
> // NULL out Block to force lazy instantiation of blocks for the body.
> Block = NULL;
>
> + // If body is not a compound statement create implicit scope
> + // and add destructors.
> + if (!isa<CompoundStmt>(D->getBody()))
> + addLocalScopeAndDtors(D->getBody());
> +
> // Create the body. The returned block is the entry to the loop body.
> BodyBlock = addStmt(D->getBody());
>
> @@ -1529,9 +1878,10 @@
>
> // If there is no target for the continue, then we are looking at an
> // incomplete AST. This means the CFG cannot be constructed.
> - if (ContinueTargetBlock)
> - AddSuccessor(Block, ContinueTargetBlock);
> - else
> + if (ContinueJumpTarget.Block) {
> + addAutomaticObjDtors(ScopePos, ContinueJumpTarget.ScopePos, C);
> + AddSuccessor(Block, ContinueJumpTarget.Block);
> + } else
> badCFG = true;
>
> return Block;
> @@ -1570,6 +1920,18 @@
> // block.
> CFGBlock* SwitchSuccessor = NULL;
>
> + // Save local scope position because in case of condition variable
> ScopePos
> + // won't be restored when traversing AST.
> + SaveAndRestore<LocalScope::const_iterator> save_scope_pos(ScopePos);
> +
> + // Create local scope for possible condition variable.
> + // Store scope position. Add implicit destructor.
> + if (VarDecl* VD = Terminator->getConditionVariable()) {
> + LocalScope::const_iterator SwitchBeginScopePos = ScopePos;
> + addLocalScopeForVarDecl(VD);
> + addAutomaticObjDtors(ScopePos, SwitchBeginScopePos, Terminator);
> + }
> +
> if (Block) {
> if (!FinishBlock(Block))
> return 0;
> @@ -1578,8 +1940,8 @@
>
> // Save the current "switch" context.
> SaveAndRestore<CFGBlock*> save_switch(SwitchTerminatedBlock),
> - save_break(BreakTargetBlock),
> save_default(DefaultCaseBlock);
> + SaveAndRestore<JumpTarget> save_break(BreakJumpTarget);
>
> // Set the "default" case to be the block after the switch statement. If
> the
> // switch statement contains a "default:", this value will be overwritten
> with
> @@ -1592,13 +1954,19 @@
> // Now process the switch body. The code after the switch is the
> implicit
> // successor.
> Succ = SwitchSuccessor;
> - BreakTargetBlock = SwitchSuccessor;
> + BreakJumpTarget = JumpTarget(SwitchSuccessor, ScopePos);
>
> // When visiting the body, the case statements should automatically get
> linked
> // up to the switch. We also don't keep a pointer to the body, since all
> // control-flow from the switch goes to case/default statements.
> assert(Terminator->getBody() && "switch must contain a non-NULL body");
> Block = NULL;
> +
> + // If body is not a compound statement create implicit scope
> + // and add destructors.
> + if (!isa<CompoundStmt>(Terminator->getBody()))
> + addLocalScopeAndDtors(Terminator->getBody());
> +
> CFGBlock *BodyBlock = addStmt(Terminator->getBody());
> if (Block) {
> if (!FinishBlock(BodyBlock))
> @@ -1776,6 +2144,18 @@
> // CXXCatchStmt are treated like labels, so they are the first statement
> in a
> // block.
>
> + // Save local scope position because in case of exception variable
> ScopePos
> + // won't be restored when traversing AST.
> + SaveAndRestore<LocalScope::const_iterator> save_scope_pos(ScopePos);
> +
> + // Create local scope for possible exception variable.
> + // Store scope position. Add implicit destructor.
> + if (VarDecl* VD = CS->getExceptionDecl()) {
> + LocalScope::const_iterator BeginScopePos = ScopePos;
> + addLocalScopeForVarDecl(VD);
> + addAutomaticObjDtors(ScopePos, BeginScopePos, CS);
> + }
> +
> if (CS->getHandlerBlock())
> addStmt(CS->getHandlerBlock());
>
> @@ -1847,11 +2227,9 @@
> /// buildCFG - Constructs a CFG from an AST. Ownership of the returned
> /// CFG is returned to the caller.
> CFG* CFG::buildCFG(const Decl *D, Stmt* Statement, ASTContext *C,
> - bool PruneTriviallyFalse,
> - bool AddEHEdges, bool AddScopes) {
> + CFG::BuildOptions BO) {
> CFGBuilder Builder;
> - return Builder.buildCFG(D, Statement, C, PruneTriviallyFalse,
> - AddEHEdges, AddScopes);
> + return Builder.buildCFG(D, Statement, C, BO);
> }
>
>
> //===----------------------------------------------------------------------===//
> @@ -1889,16 +2267,26 @@
> llvm::SmallPtrSet<Expr*,50> SubExprAssignments;
>
> for (CFG::iterator I=cfg.begin(), E=cfg.end(); I != E; ++I)
> - for (CFGBlock::iterator BI=(*I)->begin(), EI=(*I)->end(); BI != EI;
> ++BI)
> - FindSubExprAssignments(*BI, SubExprAssignments);
> + for (CFGBlock::iterator BI=(*I)->begin(), EI=(*I)->end(); BI != EI;
> ++BI) {
> + if (CFGStmt S = BI->getAs<CFGStmt>()) {
> + FindSubExprAssignments(S, SubExprAssignments);
> + } else if (CFGInitializer I = BI->getAs<CFGInitializer>()) {
> + Stmt* S = I.getInitializer()->getInit();
> + FindSubExprAssignments(S, SubExprAssignments);
> + }
> + }
>
> for (CFG::iterator I=cfg.begin(), E=cfg.end(); I != E; ++I) {
>
> // Iterate over the statements again on identify the Expr* and Stmt* at
> the
> // block-level that are block-level expressions.
>
> - for (CFGBlock::iterator BI=(*I)->begin(), EI=(*I)->end(); BI != EI;
> ++BI)
> - if (Expr* Exp = dyn_cast<Expr>(*BI)) {
> + for (CFGBlock::iterator BI=(*I)->begin(), EI=(*I)->end(); BI != EI;
> ++BI) {
> + if (!BI->isStmt())
> + continue;
> +
> + CFGStmt& SE = static_cast<CFGStmt&>(*BI);
> + if (Expr* Exp = dyn_cast<Expr>(SE.getStmt())) {
>
> if (BinaryOperator* B = dyn_cast<BinaryOperator>(Exp)) {
> // Assignment expressions that are not nested within another
> @@ -1919,6 +2307,7 @@
> unsigned x = M->size();
> (*M)[Exp] = x;
> }
> + }
>
> // Look at terminators. The condition is a block-level expression.
>
> @@ -1969,7 +2358,9 @@
>
> class StmtPrinterHelper : public PrinterHelper {
> typedef llvm::DenseMap<Stmt*,std::pair<unsigned,unsigned> > StmtMapTy;
> + typedef llvm::DenseMap<Decl*,std::pair<unsigned,unsigned> > DeclMapTy;
> StmtMapTy StmtMap;
> + DeclMapTy DeclMap;
> signed CurrentBlock;
> unsigned CurrentStmt;
> const LangOptions &LangOpts;
> @@ -1981,7 +2372,34 @@
> unsigned j = 1;
> for (CFGBlock::const_iterator BI = (*I)->begin(), BEnd = (*I)->end()
> ;
> BI != BEnd; ++BI, ++j )
> - StmtMap[*BI] = std::make_pair((*I)->getBlockID(),j);
> + if (CFGStmt SE = BI->getAs<CFGStmt>()) {
> + std::pair<unsigned, unsigned> P((*I)->getBlockID(), j);
> + StmtMap[SE.getStmt()] = P;
> +
> + if (DeclStmt* DS = dyn_cast<DeclStmt>(SE.getStmt())) {
> + DeclMap[DS->getSingleDecl()] = P;
> +
> + } else if (IfStmt* IS = dyn_cast<IfStmt>(SE.getStmt())) {
> + if (VarDecl* VD = IS->getConditionVariable())
> + DeclMap[VD] = P;
> +
> + } else if (ForStmt* FS = dyn_cast<ForStmt>(SE.getStmt())) {
> + if (VarDecl* VD = FS->getConditionVariable())
> + DeclMap[VD] = P;
> +
> + } else if (WhileStmt* WS = dyn_cast<WhileStmt>(SE.getStmt())) {
> + if (VarDecl* VD = WS->getConditionVariable())
> + DeclMap[VD] = P;
> +
> + } else if (SwitchStmt* SS = dyn_cast<SwitchStmt>(SE.getStmt()))
> {
> + if (VarDecl* VD = SS->getConditionVariable())
> + DeclMap[VD] = P;
> +
> + } else if (CXXCatchStmt* CS =
> dyn_cast<CXXCatchStmt>(SE.getStmt())) {
> + if (VarDecl* VD = CS->getExceptionDecl())
> + DeclMap[VD] = P;
> + }
> + }
> }
> }
>
> @@ -1991,10 +2409,9 @@
> void setBlockID(signed i) { CurrentBlock = i; }
> void setStmtID(unsigned i) { CurrentStmt = i; }
>
> - virtual bool handledStmt(Stmt* Terminator, llvm::raw_ostream& OS) {
> + virtual bool handledStmt(Stmt* S, llvm::raw_ostream& OS) {
> + StmtMapTy::iterator I = StmtMap.find(S);
>
> - StmtMapTy::iterator I = StmtMap.find(Terminator);
> -
> if (I == StmtMap.end())
> return false;
>
> @@ -2006,6 +2423,21 @@
> OS << "[B" << I->second.first << "." << I->second.second << "]";
> return true;
> }
> +
> + bool handleDecl(Decl* D, llvm::raw_ostream& OS) {
> + DeclMapTy::iterator I = DeclMap.find(D);
> +
> + if (I == DeclMap.end())
> + return false;
> +
> + if (CurrentBlock >= 0 && I->second.first == (unsigned) CurrentBlock
> + && I->second.second == CurrentStmt) {
> + return false;
> + }
> +
> + OS << "[B" << I->second.first << "." << I->second.second << "]";
> + return true;
> + }
> };
> } // end anonymous namespace
>
> @@ -2109,57 +2541,80 @@
> } // end anonymous namespace
>
>
> -static void print_stmt(llvm::raw_ostream &OS, StmtPrinterHelper* Helper,
> +static void print_elem(llvm::raw_ostream &OS, StmtPrinterHelper* Helper,
> const CFGElement &E) {
>
> - if (E.asStartScope()) {
> - OS << "start scope\n";
> - return;
> - }
> - if (E.asEndScope()) {
> - OS << "end scope\n";
> - return;
> - }
> + if (CFGStmt CS = E.getAs<CFGStmt>()) {
> + Stmt *S = CS;
> +
> + if (Helper) {
>
> - Stmt *S = E;
> -
> - if (Helper) {
> - // special printing for statement-expressions.
> - if (StmtExpr* SE = dyn_cast<StmtExpr>(S)) {
> - CompoundStmt* Sub = SE->getSubStmt();
> + // special printing for statement-expressions.
> + if (StmtExpr* SE = dyn_cast<StmtExpr>(S)) {
> + CompoundStmt* Sub = SE->getSubStmt();
>
> - if (Sub->child_begin() != Sub->child_end()) {
> - OS << "({ ... ; ";
> - Helper->handledStmt(*SE->getSubStmt()->body_rbegin(),OS);
> - OS << " })\n";
> - return;
> + if (Sub->child_begin() != Sub->child_end()) {
> + OS << "({ ... ; ";
> + Helper->handledStmt(*SE->getSubStmt()->body_rbegin(),OS);
> + OS << " })\n";
> + return;
> + }
> }
> - }
>
> - // special printing for comma expressions.
> - if (BinaryOperator* B = dyn_cast<BinaryOperator>(S)) {
> - if (B->getOpcode() == BO_Comma) {
> - OS << "... , ";
> - Helper->handledStmt(B->getRHS(),OS);
> - OS << '\n';
> - return;
> + // special printing for comma expressions.
> + if (BinaryOperator* B = dyn_cast<BinaryOperator>(S)) {
> + if (B->getOpcode() == BO_Comma) {
> + OS << "... , ";
> + Helper->handledStmt(B->getRHS(),OS);
> + OS << '\n';
> + return;
> + }
> }
> }
> - }
>
> - S->printPretty(OS, Helper, PrintingPolicy(Helper->getLangOpts()));
> -
> - if (isa<CXXOperatorCallExpr>(S)) {
> - OS << " (OperatorCall)";
> - }
> - else if (isa<CXXBindTemporaryExpr>(S)) {
> - OS << " (BindTemporary)";
> - }
> + S->printPretty(OS, Helper, PrintingPolicy(Helper->getLangOpts()));
>
> + if (isa<CXXOperatorCallExpr>(S)) {
> + OS << " (OperatorCall)";
> + }
> + else if (isa<CXXBindTemporaryExpr>(S)) {
> + OS << " (BindTemporary)";
> + }
>
> - // Expressions need a newline.
> - if (isa<Expr>(S))
> - OS << '\n';
> + // Expressions need a newline.
> + if (isa<Expr>(S))
> + OS << '\n';
> +
> + } else if (CFGInitializer IE = E.getAs<CFGInitializer>()) {
> + CXXBaseOrMemberInitializer* I = IE;
> + if (I->isBaseInitializer())
> + OS << I->getBaseClass()->getAsCXXRecordDecl()->getName();
> + else OS << I->getMember()->getName();
> +
> + OS << "(";
> + if (Expr* IE = I->getInit())
> + IE->printPretty(OS, Helper, PrintingPolicy(Helper->getLangOpts()));
> + OS << ")";
> +
> + if (I->isBaseInitializer())
> + OS << " (Base initializer)\n";
> + else OS << " (Member initializer)\n";
> +
> + } else if (CFGAutomaticObjDtor DE = E.getAs<CFGAutomaticObjDtor>()){
> + VarDecl* VD = DE.getVarDecl();
> + Helper->handleDecl(VD, OS);
> +
> + Type* T = VD->getType().getTypePtr();
> + if (const ReferenceType* RT = T->getAs<ReferenceType>())
> + T = RT->getPointeeType().getTypePtr();
> +
> + OS << ".~" << T->getAsCXXRecordDecl()->getName().str() << "()";
> + OS << " (Implicit destructor)\n";
> +
> + } else if (CFGImplicitDtor DE = E.getAs<CFGImplicitDtor>()) {
> + OS << "TODO: print implicit destructor desc\n";
> + (void)DE;
> + }
> }
>
> static void print_block(llvm::raw_ostream& OS, const CFG* cfg,
> @@ -2214,7 +2669,7 @@
> OS << ":\n";
> }
>
> - // Iterate through the statements in the block and print them.
> + // Iterate through the elements in the block and print them.
> unsigned j = 1;
>
> for (CFGBlock::const_iterator I = B.begin(), E = B.end() ;
> @@ -2229,7 +2684,7 @@
> if (Helper)
> Helper->setStmtID(j);
>
> - print_stmt(OS,Helper,*I);
> + print_elem(OS,Helper,*I);
> }
>
> // Print the terminator of this block.
> Index: lib/Analysis/ReachableCode.cpp
> ===================================================================
> --- lib/Analysis/ReachableCode.cpp (revision 112851)
> +++ lib/Analysis/ReachableCode.cpp (working copy)
> @@ -31,9 +31,14 @@
> R1 = R2 = SourceRange();
>
> top:
> - if (sn < b.size())
> - S = b[sn].getStmt();
> - else if (b.getTerminator())
> + if (sn < b.size()) {
> + // FIXME: (CFGElement) Should handle other CFGElement kinds here also.
> + if (!b[sn].isStmt()) {
> + ++sn;
> + goto top;
> + }
> + S = b[sn].getAs<CFGStmt>().getStmt();
> + } else if (b.getTerminator())
> S = b.getTerminator();
> else
> return SourceLocation();
> @@ -43,7 +48,8 @@
> const BinaryOperator *BO = cast<BinaryOperator>(S);
> if (BO->getOpcode() == BO_Comma) {
> if (sn+1 < b.size())
> - return b[sn+1].getStmt()->getLocStart();
> + // Here we can have only statement CFGElement.
> + return b[sn+1].getAs<CFGStmt>().getStmt()->getLocStart();
> const CFGBlock *n = &b;
> while (1) {
> if (n->getTerminator())
> @@ -54,7 +60,8 @@
> if (n->pred_size() != 1)
> return SourceLocation();
> if (!n->empty())
> - return n[0][0].getStmt()->getLocStart();
> + // Here we can have only statement CFGElement.
> + return n[0][0].getAs<CFGStmt>().getStmt()->getLocStart();
> }
> }
> R1 = BO->getLHS()->getSourceRange();
> Index: lib/Analysis/CFGStmtMap.cpp
> ===================================================================
> --- lib/Analysis/CFGStmtMap.cpp (revision 112851)
> +++ lib/Analysis/CFGStmtMap.cpp (working copy)
> @@ -49,8 +49,7 @@
> static void Accumulate(SMap &SM, CFGBlock *B) {
> // First walk the block-level expressions.
> for (CFGBlock::iterator I = B->begin(), E = B->end(); I != E; ++I) {
> - const CFGElement &CE = *I;
> - if (Stmt *S = CE.getStmt()) {
> + if (CFGStmt S = I->getAs<CFGStmt>()) {
> CFGBlock *&Entry = SM[S];
> // If 'Entry' is already initialized (e.g., a terminator was
> already),
> // skip.
> Index: lib/Analysis/AnalysisContext.cpp
> ===================================================================
> --- lib/Analysis/AnalysisContext.cpp (revision 112851)
> +++ lib/Analysis/AnalysisContext.cpp (working copy)
> @@ -59,7 +59,8 @@
> return getUnoptimizedCFG();
>
> if (!builtCFG) {
> - cfg = CFG::buildCFG(D, getBody(), &D->getASTContext(), true,
> AddEHEdges);
> + cfg = CFG::buildCFG(D, getBody(), &D->getASTContext(),
> + CFG::BuildOptions().setAddEHEdges(AddEHEdges));
> // Even when the cfg is not successfully built, we don't
> // want to try building it again.
> builtCFG = true;
> @@ -70,7 +71,8 @@
> CFG *AnalysisContext::getUnoptimizedCFG() {
> if (!builtCompleteCFG) {
> completeCFG = CFG::buildCFG(D, getBody(), &D->getASTContext(),
> - false, AddEHEdges);
> + CFG::BuildOptions().setAddEHEdges(AddEHEdges)
> + .setPruneTriviallyFalseEdges(false));
> // Even when the cfg is not successfully built, we don't
> // want to try building it again.
> builtCompleteCFG = true;
> Index: lib/Analysis/LiveVariables.cpp
> ===================================================================
> --- lib/Analysis/LiveVariables.cpp (revision 112851)
> +++ lib/Analysis/LiveVariables.cpp (working copy)
> @@ -86,7 +86,7 @@
> getAnalysisData().killAtAssign = killAtAssign;
>
> RegisterDecls R(getAnalysisData());
> - cfg.VisitBlockStmts(R);
> + cfg.VisitCFGElements(R);
>
> // Register all parameters even if they didn't occur in the function
> body.
> if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(AC.getDecl()))
> Index: lib/Analysis/UninitializedValues.cpp
> ===================================================================
> --- lib/Analysis/UninitializedValues.cpp (revision 112851)
> +++ lib/Analysis/UninitializedValues.cpp (working copy)
> @@ -42,7 +42,7 @@
>
> void UninitializedValues::InitializeValues(const CFG& cfg) {
> RegisterDecls R(getAnalysisData());
> - cfg.VisitBlockStmts(R);
> + cfg.VisitCFGElements(R);
> }
>
>
> //===----------------------------------------------------------------------===//
> Index: lib/Sema/AnalysisBasedWarnings.cpp
> ===================================================================
> --- lib/Sema/AnalysisBasedWarnings.cpp (revision 112851)
> +++ lib/Sema/AnalysisBasedWarnings.cpp (working copy)
> @@ -116,6 +116,8 @@
> if (!live[B.getBlockID()])
> continue;
> if (B.size() == 0) {
> + // FIXME: (CFGElement) This should be reimplemented when (or if)
> scopes
> + // will be added to CFG.
> if (B.getTerminator() && isa<CXXTryStmt>(B.getTerminator())) {
> HasAbnormalEdge = true;
> continue;
> @@ -125,66 +127,77 @@
> HasPlainEdge = true;
> continue;
> }
> - Stmt *S = B[B.size()-1];
> - if (isa<ReturnStmt>(S)) {
> - HasLiveReturn = true;
> - continue;
> - }
> - if (isa<ObjCAtThrowStmt>(S)) {
> - HasFakeEdge = true;
> - continue;
> - }
> - if (isa<CXXThrowExpr>(S)) {
> - HasFakeEdge = true;
> - continue;
> - }
> - if (const AsmStmt *AS = dyn_cast<AsmStmt>(S)) {
> - if (AS->isMSAsm()) {
> - HasFakeEdge = true;
> + CFGElement E = B[B.size() - 1];
> + if (CFGStmt SE = E.getAs<CFGStmt>()) {
> + Stmt* S = SE;
> + if (isa<ReturnStmt>(S)) {
> HasLiveReturn = true;
> continue;
> }
> - }
> - if (isa<CXXTryStmt>(S)) {
> - HasAbnormalEdge = true;
> - continue;
> - }
> -
> - bool NoReturnEdge = false;
> - if (CallExpr *C = dyn_cast<CallExpr>(S)) {
> - if (std::find(B.succ_begin(), B.succ_end(), &cfg->getExit())
> - == B.succ_end()) {
> - HasAbnormalEdge = true;
> + if (isa<ObjCAtThrowStmt>(S)) {
> + HasFakeEdge = true;
> continue;
> }
> - Expr *CEE = C->getCallee()->IgnoreParenCasts();
> - if (getFunctionExtInfo(CEE->getType()).getNoReturn()) {
> - NoReturnEdge = true;
> + if (isa<CXXThrowExpr>(S)) {
> HasFakeEdge = true;
> - } else if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(CEE)) {
> - ValueDecl *VD = DRE->getDecl();
> - if (VD->hasAttr<NoReturnAttr>()) {
> - NoReturnEdge = true;
> + continue;
> + }
> + if (const AsmStmt *AS = dyn_cast<AsmStmt>(S)) {
> + if (AS->isMSAsm()) {
> HasFakeEdge = true;
> + HasLiveReturn = true;
> + continue;
> }
> }
> - }
> - // FIXME: Remove this hack once temporaries and their destructors are
> - // modeled correctly by the CFG.
> - if (CXXExprWithTemporaries *E = dyn_cast<CXXExprWithTemporaries>(S)) {
> - for (unsigned I = 0, N = E->getNumTemporaries(); I != N; ++I) {
> - const FunctionDecl *FD = E->getTemporary(I)->getDestructor();
> - if (FD->hasAttr<NoReturnAttr>() ||
> - FD->getType()->getAs<FunctionType>()->getNoReturnAttr()) {
> + if (isa<CXXTryStmt>(S)) {
> + HasAbnormalEdge = true;
> + continue;
> + }
> +
> + bool NoReturnEdge = false;
> + if (CallExpr *C = dyn_cast<CallExpr>(S)) {
> + if (std::find(B.succ_begin(), B.succ_end(), &cfg->getExit())
> + == B.succ_end()) {
> + HasAbnormalEdge = true;
> + continue;
> + }
> + Expr *CEE = C->getCallee()->IgnoreParenCasts();
> + if (getFunctionExtInfo(CEE->getType()).getNoReturn()) {
> NoReturnEdge = true;
> HasFakeEdge = true;
> - break;
> + } else if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(CEE)) {
> + ValueDecl *VD = DRE->getDecl();
> + if (VD->hasAttr<NoReturnAttr>()) {
> + NoReturnEdge = true;
> + HasFakeEdge = true;
> + }
> }
> }
> + // FIXME: Remove this hack once temporaries and their destructors
> are
> + // modeled correctly by the CFG.
> + if (CXXExprWithTemporaries *E = dyn_cast<CXXExprWithTemporaries>(S))
> {
> + for (unsigned I = 0, N = E->getNumTemporaries(); I != N; ++I) {
> + const FunctionDecl *FD = E->getTemporary(I)->getDestructor();
> + if (FD->hasAttr<NoReturnAttr>() ||
> + FD->getType()->getAs<FunctionType>()->getNoReturnAttr()) {
> + NoReturnEdge = true;
> + HasFakeEdge = true;
> + break;
> + }
> + }
> + }
> + // FIXME: Add noreturn message sends.
> + if (NoReturnEdge == false)
> + HasPlainEdge = true;
> +
> + } else if (CFGInitializer IE = E.getAs<CFGInitializer>()) {
> + // FIXME: (CFGElement) Handle initializer as last element.
> + (void)IE;
> +
> + } else if (CFGImplicitDtor DE = E.getAs<CFGImplicitDtor>()) {
> + // FIXME: (CFGElement) Handle implicit destructor call as last
> element.
> + (void)DE;
> }
> - // FIXME: Add noreturn message sends.
> - if (NoReturnEdge == false)
> - HasPlainEdge = true;
> }
> if (!HasPlainEdge) {
> if (HasLiveReturn)
> Index: lib/Checker/UnreachableCodeChecker.cpp
> ===================================================================
> --- lib/Checker/UnreachableCodeChecker.cpp (revision 112851)
> +++ lib/Checker/UnreachableCodeChecker.cpp (working copy)
> @@ -116,10 +116,12 @@
> // FIXME: This should be extended to include other unreachable markers,
> // such as llvm_unreachable.
> if (!CB->empty()) {
> - const Stmt *First = CB->front();
> - if (const CallExpr *CE = dyn_cast<CallExpr>(First)) {
> - if (CE->isBuiltinCall(Ctx) == Builtin::BI__builtin_unreachable)
> - continue;
> + CFGElement First = CB->front();
> + if (CFGStmt S = First.getAs<CFGStmt>()) {
> + if (const CallExpr *CE = dyn_cast<CallExpr>(S.getStmt())) {
> + if (CE->isBuiltinCall(Ctx) == Builtin::BI__builtin_unreachable)
> + continue;
> + }
> }
> }
>
> Looks good.
>
>
> @@ -173,12 +175,14 @@
>
> // Find the Stmt* in a CFGBlock for reporting a warning
> const Stmt *UnreachableCodeChecker::getUnreachableStmt(const CFGBlock *CB)
> {
> - if (CB->size() > 0)
> - return CB->front().getStmt();
> - else if (const Stmt *S = CB->getTerminator())
> + // FIXME: (CFGElement) Should we handle other CFGElement kinds here as
> well?
> + for (CFGBlock::const_iterator I = CB->begin(), E = CB->end(); I != E;
> ++I) {
> + if (CFGStmt S = I->getAs<CFGStmt>())
> + return S;
> + }
> + if (const Stmt *S = CB->getTerminator())
> return S;
> - else
> - return 0;
> + return 0;
> }
>
> // Determines if the path to this CFGBlock contained an element that
> infers this
> Index: lib/Checker/CheckDeadStores.cpp
> ===================================================================
> --- lib/Checker/CheckDeadStores.cpp (revision 112851)
> +++ lib/Checker/CheckDeadStores.cpp (working copy)
> @@ -283,7 +283,7 @@
> void clang::CheckDeadStores(CFG &cfg, LiveVariables &L, ParentMap &pmap,
> BugReporter& BR) {
> FindEscaped FS(&cfg);
> - FS.getCFG().VisitBlockStmts(FS);
> + FS.getCFG().VisitCFGElements(FS);
> DeadStoreObs A(BR.getContext(), BR, pmap, FS.Escaped);
> L.runOnAllBlocks(cfg, &A);
> }
>
> Looks great.
>
>
> Index: lib/Checker/BugReporter.cpp
> ===================================================================
> --- lib/Checker/BugReporter.cpp (revision 112851)
> +++ lib/Checker/BugReporter.cpp (working copy)
> @@ -1165,7 +1165,7 @@
> }
>
> if (const BlockEntrance *BE = dyn_cast<BlockEntrance>(&P)) {
> - if (const Stmt* S = BE->getFirstStmt()) {
> + if (CFGStmt S = BE->getFirstElement().getAs<CFGStmt>()) {
> if (IsControlFlowExpr(S)) {
> // Add the proper context for '&&', '||', and '?'.
> EB.addContext(S);
>
> Looks great.
>
> Index: lib/Checker/GRExprEngine.cpp
> ===================================================================
> --- lib/Checker/GRExprEngine.cpp (revision 112851)
> +++ lib/Checker/GRExprEngine.cpp (working copy)
> @@ -633,7 +633,7 @@
> }
>
> void GRExprEngine::ProcessStmt(const CFGElement CE,GRStmtNodeBuilder&
> builder) {
> - CurrentStmt = CE.getStmt();
> + CurrentStmt = CE.getAs<CFGStmt>();
> PrettyStackTraceLoc CrashInfo(getContext().getSourceManager(),
> CurrentStmt->getLocStart(),
> "Error evaluating statement");
> @@ -720,7 +720,7 @@
> Builder->SetCleanedState(*I == EntryNode ? CleanedState :
> GetState(*I));
>
> // Visit the statement.
> - if (CE.asLValue())
> + if (CE.getAs<CFGStmt>().asLValue())
>
> Looks great.
>
> VisitLValue(cast<Expr>(CurrentStmt), *I, Dst);
> else
> Visit(CurrentStmt, *I, Dst);
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20100915/142bdc56/attachment.html>
More information about the cfe-dev
mailing list