Minor suggestion below. On Aug 7, 2009, at 6:41 PM, Douglas Gregor wrote:
> Author: dgregor > Date: Fri Aug 7 20:41:12 2009 > New Revision: 78450 > > URL: http://llvm.org/viewvc/llvm-project?rev=78450&view=rev > Log: > Introduce reference counting for statements and expressions, using it > to allow sharing of nodes. Simplifies some aspects of template > instantiation, and fixes both PR3444 and <rdar://problem/6757457>. > > Modified: > cfe/trunk/include/clang/AST/Expr.h > cfe/trunk/include/clang/AST/Stmt.h > cfe/trunk/lib/AST/Stmt.cpp > cfe/trunk/lib/Frontend/PCHReaderStmt.cpp > cfe/trunk/lib/Sema/Sema.h > cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp > cfe/trunk/lib/Sema/SemaTemplateInstantiateExpr.cpp > > Modified: cfe/trunk/include/clang/AST/Expr.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=78450&r1=78449&r2=78450&view=diff > > = > = > = > = > = > = > = > = > ====================================================================== > --- cfe/trunk/include/clang/AST/Expr.h (original) > +++ cfe/trunk/include/clang/AST/Expr.h Fri Aug 7 20:41:12 2009 > @@ -67,6 +67,15 @@ > explicit Expr(StmtClass SC, EmptyShell) : Stmt(SC) { } > > public: > + /// \brief Increases the reference count for this expression. > + /// > + /// Invoke the Retain() operation when this expression > + /// is being shared by another owner. > + Expr *Retain() { > + Stmt::Retain(); > + return this; > + } > + > QualType getType() const { return TR; } > void setType(QualType t) { > // In C++, the type of an expression is always adjusted so that it > > Modified: cfe/trunk/include/clang/AST/Stmt.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Stmt.h?rev=78450&r1=78449&r2=78450&view=diff > > = > = > = > = > = > = > = > = > ====================================================================== > --- cfe/trunk/include/clang/AST/Stmt.h (original) > +++ cfe/trunk/include/clang/AST/Stmt.h Fri Aug 7 20:41:12 2009 > @@ -105,7 +105,11 @@ > #include "clang/AST/StmtNodes.def" > }; > private: > - const StmtClass sClass; > + /// \brief The statement class. > + const unsigned sClass : 8; > + > + /// \brief The reference count for this statement. > + unsigned RefCount : 24; > > // Make vanilla 'new' and 'delete' illegal for Stmts. > protected: > @@ -151,7 +155,7 @@ > void DestroyChildren(ASTContext& Ctx); > > /// \brief Construct an empty statement. > - explicit Stmt(StmtClass SC, EmptyShell) : sClass(SC) { > + explicit Stmt(StmtClass SC, EmptyShell) : sClass(SC), RefCount(1) { > if (Stmt::CollectingStats()) Stmt::addStmtClass(SC); > } > > @@ -163,15 +167,27 @@ > virtual void DoDestroy(ASTContext &Ctx); > > public: > - Stmt(StmtClass SC) : sClass(SC) { > + Stmt(StmtClass SC) : sClass(SC), RefCount(1) { > if (Stmt::CollectingStats()) Stmt::addStmtClass(SC); > } > virtual ~Stmt() {} > > /// \brief Destroy the current statement and its children. > - void Destroy(ASTContext &Ctx) { DoDestroy(Ctx); } > + void Destroy(ASTContext &Ctx) { assert(RefCount != 0 && ...); > > + if (--RefCount == 0) > + DoDestroy(Ctx); > + } > > - StmtClass getStmtClass() const { return sClass; } > + /// \brief Increases the reference count for this statement. > + /// > + /// Invoke the Retain() operation when this statement or expression > + /// is being shared by another owner. > + Stmt *Retain() { assert (RefCount >= 1 && ...); - Fariborz > > + ++RefCount; > + return this; > + } > + > + StmtClass getStmtClass() const { return (StmtClass)sClass; } > const char *getStmtClassName() const; > > /// SourceLocation tokens are not useful in isolation - they are > low level > @@ -643,6 +659,10 @@ > // This points to a linked list of case and default statements. > SwitchCase *FirstCase; > SourceLocation SwitchLoc; > + > +protected: > + virtual void DoDestroy(ASTContext &Ctx); > + > public: > SwitchStmt(Expr *cond) : Stmt(SwitchStmtClass), FirstCase(0) { > SubExprs[COND] = reinterpret_cast<Stmt*>(cond); > @@ -661,6 +681,11 @@ > Stmt *getBody() { return SubExprs[BODY]; } > void setBody(Stmt *S) { SubExprs[BODY] = S; } > SwitchCase *getSwitchCaseList() { return FirstCase; } > + > + /// \brief Set the case list for this switch statement. > + /// > + /// The caller is responsible for incrementing the retain counts on > + /// all of the SwitchCase statements in this list. > void setSwitchCaseList(SwitchCase *SC) { FirstCase = SC; } > > SourceLocation getSwitchLoc() const { return SwitchLoc; } > @@ -672,6 +697,7 @@ > } > void addSwitchCase(SwitchCase *SC) { > assert(!SC->getNextSwitchCase() && "case/default already added > to a switch"); > + SC->Retain(); > SC->setNextSwitchCase(FirstCase); > FirstCase = SC; > } > > Modified: cfe/trunk/lib/AST/Stmt.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Stmt.cpp?rev=78450&r1=78449&r2=78450&view=diff > > = > = > = > = > = > = > = > = > ====================================================================== > --- cfe/trunk/lib/AST/Stmt.cpp (original) > +++ cfe/trunk/lib/AST/Stmt.cpp Fri Aug 7 20:41:12 2009 > @@ -43,7 +43,7 @@ > } > > const char *Stmt::getStmtClassName() const { > - return getStmtInfoTableEntry(sClass).Name; > + return getStmtInfoTableEntry((StmtClass)sClass).Name; > } > > void Stmt::DestroyChildren(ASTContext &C) { > @@ -104,6 +104,20 @@ > return new (C) BreakStmt(BreakLoc); > } > > +void SwitchStmt::DoDestroy(ASTContext &Ctx) { > + // Destroy the SwitchCase statements in this switch. In the normal > + // case, this loop will merely decrement the reference counts from > + // the Retain() calls in addSwitchCase(); > + SwitchCase *SC = FirstCase; > + while (SC) { > + SwitchCase *Next = SC->getNextSwitchCase(); > + SC->Destroy(Ctx); > + SC = Next; > + } > + > + Stmt::DoDestroy(Ctx); > +} > + > void CompoundStmt::setStmts(ASTContext &C, Stmt **Stmts, unsigned > NumStmts) { > if (this->Body) > C.Deallocate(Body); > > Modified: cfe/trunk/lib/Frontend/PCHReaderStmt.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/PCHReaderStmt.cpp?rev=78450&r1=78449&r2=78450&view=diff > > = > = > = > = > = > = > = > = > ====================================================================== > --- cfe/trunk/lib/Frontend/PCHReaderStmt.cpp (original) > +++ cfe/trunk/lib/Frontend/PCHReaderStmt.cpp Fri Aug 7 20:41:12 2009 > @@ -194,6 +194,10 @@ > PrevSC->setNextSwitchCase(SC); > else > S->setSwitchCaseList(SC); > + > + // Retain this SwitchCase, since SwitchStmt::addSwitchCase() > would > + // normally retain it (but we aren't calling addSwitchCase). > + SC->Retain(); > PrevSC = SC; > } > return 2; > > Modified: cfe/trunk/lib/Sema/Sema.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.h?rev=78450&r1=78449&r2=78450&view=diff > > = > = > = > = > = > = > = > = > ====================================================================== > --- cfe/trunk/lib/Sema/Sema.h (original) > +++ cfe/trunk/lib/Sema/Sema.h Fri Aug 7 20:41:12 2009 > @@ -2821,14 +2821,6 @@ > > NamedDecl *InstantiateCurrentDeclRef(NamedDecl *D); > > - // Simple function for cloning expressions. > - template<typename T> > - OwningExprResult Clone(T *E) { > - assert(!E->isValueDependent() && !E->isTypeDependent() && > - "expression is value or type dependent!"); > - return Owned(E->Clone(Context)); > - } > - > // Objective-C declarations. > virtual DeclPtrTy ActOnStartClassInterface(SourceLocation > AtInterfaceLoc, > IdentifierInfo > *ClassName, > > Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=78450&r1=78449&r2=78450&view=diff > > = > = > = > = > = > = > = > = > ====================================================================== > --- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original) > +++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Fri Aug 7 > 20:41:12 2009 > @@ -222,7 +222,8 @@ > if (InstantiatedAssertExpr.isInvalid()) > return 0; > > - OwningExprResult Message = SemaRef.Clone(D->getMessage()); > + OwningExprResult Message(SemaRef, D->getMessage()); > + D->getMessage()->Retain(); > Decl *StaticAssert > = SemaRef.ActOnStaticAssertDeclaration(D->getLocation(), > > move(InstantiatedAssertExpr), > > Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateExpr.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateExpr.cpp?rev=78450&r1=78449&r2=78450&view=diff > > = > = > = > = > = > = > = > = > ====================================================================== > --- cfe/trunk/lib/Sema/SemaTemplateInstantiateExpr.cpp (original) > +++ cfe/trunk/lib/Sema/SemaTemplateInstantiateExpr.cpp Fri Aug 7 > 20:41:12 2009 > @@ -57,53 +57,53 @@ > > Sema::OwningExprResult > TemplateExprInstantiator::VisitPredefinedExpr(PredefinedExpr *E) { > - return SemaRef.Clone(E); > + return SemaRef.Owned(E->Retain()); > } > > Sema::OwningExprResult > TemplateExprInstantiator::VisitIntegerLiteral(IntegerLiteral *E) { > - return SemaRef.Clone(E); > + return SemaRef.Owned(E->Retain()); > } > > Sema::OwningExprResult > TemplateExprInstantiator::VisitFloatingLiteral(FloatingLiteral *E) { > - return SemaRef.Clone(E); > + return SemaRef.Owned(E->Retain()); > } > > Sema::OwningExprResult > TemplateExprInstantiator::VisitStringLiteral(StringLiteral *E) { > - return SemaRef.Clone(E); > + return SemaRef.Owned(E->Retain()); > } > > Sema::OwningExprResult > TemplateExprInstantiator::VisitCharacterLiteral(CharacterLiteral *E) { > - return SemaRef.Clone(E); > + return SemaRef.Owned(E->Retain()); > } > > Sema::OwningExprResult > TemplateExprInstantiator::VisitImaginaryLiteral(ImaginaryLiteral *E) { > - return SemaRef.Clone(E); > + return SemaRef.Owned(E->Retain()); > } > > Sema::OwningExprResult > TemplateExprInstantiator::VisitCXXBoolLiteralExpr(CXXBoolLiteralExpr > *E) { > - return SemaRef.Clone(E); > + return SemaRef.Owned(E->Retain()); > } > > Sema::OwningExprResult > TemplateExprInstantiator > ::VisitCXXNullPtrLiteralExpr(CXXNullPtrLiteralExpr *E) { > - return SemaRef.Clone(E); > + return SemaRef.Owned(E->Retain()); > } > > Sema::OwningExprResult > TemplateExprInstantiator::VisitGNUNullExpr(GNUNullExpr *E) { > - return SemaRef.Clone(E); > + return SemaRef.Owned(E->Retain()); > } > > Sema::OwningExprResult > TemplateExprInstantiator::VisitUnresolvedFunctionNameExpr( > > UnresolvedFunctionNameExpr *E) { > - return SemaRef.Clone(E); > + return SemaRef.Owned(E->Retain()); > } > > Sema::OwningExprResult > @@ -706,7 +706,8 @@ > > ImplicitValueInitExpr *E) { > assert(!E->isTypeDependent() && !E->isValueDependent() && > "ImplicitValueInitExprs are never dependent"); > - return SemaRef.Clone(E); > + E->Retain(); > + return SemaRef.Owned(E); > } > > Sema::OwningExprResult > @@ -1110,7 +1111,7 @@ > > Sema::OwningExprResult > TemplateExprInstantiator > ::VisitCXXZeroInitValueExpr(CXXZeroInitValueExpr *E) { > - return SemaRef.Clone(E); > + return SemaRef.Owned(E->Retain()); > } > > Sema::OwningExprResult > @@ -1289,7 +1290,7 @@ > //---------------------------------------------------------------------------- > Sema::OwningExprResult > TemplateExprInstantiator::VisitObjCStringLiteral(ObjCStringLiteral > *E) { > - return SemaRef.Owned(E->Clone(SemaRef.Context)); > + return SemaRef.Owned(E->Retain()); > } > > Sema::OwningExprResult > @@ -1314,12 +1315,12 @@ > > Sema::OwningExprResult > TemplateExprInstantiator::VisitObjCSelectorExpr(ObjCSelectorExpr *E) { > - return SemaRef.Owned(E->Clone(SemaRef.Context)); > + return SemaRef.Owned(E->Retain()); > } > > Sema::OwningExprResult > TemplateExprInstantiator::VisitObjCProtocolExpr(ObjCProtocolExpr *E) { > - return SemaRef.Owned(E->Clone(SemaRef.Context)); > + return SemaRef.Owned(E->Retain()); > } > > Sema::OwningExprResult > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
