From: Douglas Gregor Date: Wed, 20 May 2009 22:33:37 +0000 (+0000) Subject: Introduce a new kind of RAII class, ASTOwningVector, which is an X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=d7e2705961bacb9df5d9119403c01c9b04aff97f;p=clang Introduce a new kind of RAII class, ASTOwningVector, which is an llvm::SmallVector that owns all of the AST nodes inside of it. This RAII class is used to ensure proper destruction of AST nodes when template instantiation fails. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@72186 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Parse/Ownership.h b/include/clang/Parse/Ownership.h index 54c04c2c4d..f593f76e78 100644 --- a/include/clang/Parse/Ownership.h +++ b/include/clang/Parse/Ownership.h @@ -14,6 +14,7 @@ #ifndef LLVM_CLANG_PARSE_OWNERSHIP_H #define LLVM_CLANG_PARSE_OWNERSHIP_H +#include "llvm/ADT/SmallVector.h" #include "llvm/ADT/PointerIntPair.h" //===----------------------------------------------------------------------===// @@ -722,6 +723,44 @@ namespace clang { } }; + /// \brief A small vector that owns a set of AST nodes. + template + class ASTOwningVector : public llvm::SmallVector { +#if !defined(DISABLE_SMART_POINTERS) + ActionBase &Actions; + bool Owned; +#endif + + ASTOwningVector(ASTOwningVector &); // do not implement + ASTOwningVector &operator=(ASTOwningVector &); // do not implement + + public: + explicit ASTOwningVector(ActionBase &Actions) +#if !defined(DISABLE_SMART_POINTERS) + : Actions(Actions), Owned(true) +#endif + { } + +#if !defined(DISABLE_SMART_POINTERS) + ~ASTOwningVector() { + if (!Owned) + return; + + for (unsigned I = 0, N = this->size(); I != N; ++I) + (Actions.*Destroyer)((*this)[I]); + } +#endif + + void **take() { +#if !defined(DISABLE_SMART_POINTERS) + Owned = false; +#endif + return &this->front(); + } + + template T **takeAs() { return (T**)take(); } + }; + #if !defined(DISABLE_SMART_POINTERS) // Out-of-line implementations due to definition dependencies diff --git a/lib/Sema/Sema.h b/lib/Sema/Sema.h index f5c4a7bb63..e16f544abb 100644 --- a/lib/Sema/Sema.h +++ b/lib/Sema/Sema.h @@ -2747,7 +2747,7 @@ public: T& operator*() const { return *get(); } T* operator->() const { return get(); } }; - + } // end namespace clang #endif diff --git a/lib/Sema/SemaTemplateInstantiateExpr.cpp b/lib/Sema/SemaTemplateInstantiateExpr.cpp index 099ce278f1..a93ad1132a 100644 --- a/lib/Sema/SemaTemplateInstantiateExpr.cpp +++ b/lib/Sema/SemaTemplateInstantiateExpr.cpp @@ -266,15 +266,12 @@ Sema::OwningExprResult TemplateExprInstantiator::VisitCallExpr(CallExpr *E) { return SemaRef.ExprError(); // Instantiate arguments - llvm::SmallVector Args; + ASTOwningVector<&ActionBase::DeleteExpr> Args(SemaRef); llvm::SmallVector FakeCommaLocs; for (unsigned I = 0, N = E->getNumArgs(); I != N; ++I) { OwningExprResult Arg = Visit(E->getArg(I)); - if (Arg.isInvalid()) { - for (unsigned Victim = 0; Victim != I; ++Victim) - Args[Victim]->Destroy(SemaRef.Context); + if (Arg.isInvalid()) return SemaRef.ExprError(); - } FakeCommaLocs.push_back( SemaRef.PP.getLocForEndOfToken(E->getArg(I)->getSourceRange().getEnd())); @@ -286,8 +283,7 @@ Sema::OwningExprResult TemplateExprInstantiator::VisitCallExpr(CallExpr *E) { return SemaRef.ActOnCallExpr(/*Scope=*/0, move(Callee), /*FIXME:*/FakeLParenLoc, Sema::MultiExprArg(SemaRef, - (void **)&Args.front(), - Args.size()), + Args.take(), Args.size()), /*FIXME:*/&FakeCommaLocs.front(), E->getRParenLoc()); } @@ -507,15 +503,11 @@ TemplateExprInstantiator::VisitTypesCompatibleExpr(TypesCompatibleExpr *E) { Sema::OwningExprResult TemplateExprInstantiator::VisitShuffleVectorExpr(ShuffleVectorExpr *E) { - // FIXME: Better solution for this! - llvm::SmallVector SubExprs; + ASTOwningVector<&ActionBase::DeleteExpr> SubExprs(SemaRef); for (unsigned I = 0, N = E->getNumSubExprs(); I != N; ++I) { OwningExprResult SubExpr = Visit(E->getExpr(I)); - if (SubExpr.isInvalid()) { - for (unsigned Victim = 0; Victim != I; ++Victim) - SubExprs[I]->Destroy(SemaRef.Context); + if (SubExpr.isInvalid()) return SemaRef.ExprError(); - } SubExprs.push_back(SubExpr.takeAs()); } @@ -537,7 +529,7 @@ TemplateExprInstantiator::VisitShuffleVectorExpr(ShuffleVectorExpr *E) { // Build the CallExpr CallExpr *TheCall = new (SemaRef.Context) CallExpr(SemaRef.Context, Callee, - &SubExprs[0], + SubExprs.takeAs(), SubExprs.size(), Builtin->getResultType(), E->getRParenLoc()); @@ -656,47 +648,34 @@ TemplateExprInstantiator::VisitCXXTemporaryObjectExpr( return SemaRef.ExprError(); } - llvm::SmallVector Args; + ASTOwningVector<&ActionBase::DeleteExpr> Args(SemaRef); Args.reserve(E->getNumArgs()); - bool Invalid = false; for (CXXTemporaryObjectExpr::arg_iterator Arg = E->arg_begin(), ArgEnd = E->arg_end(); Arg != ArgEnd; ++Arg) { OwningExprResult InstantiatedArg = Visit(*Arg); - if (InstantiatedArg.isInvalid()) { - Invalid = true; - break; - } + if (InstantiatedArg.isInvalid()) + return SemaRef.ExprError(); Args.push_back((Expr *)InstantiatedArg.release()); } - if (!Invalid) { - SourceLocation CommaLoc; - // FIXME: HACK! - if (Args.size() > 1) - CommaLoc - = SemaRef.PP.getLocForEndOfToken(Args[0]->getSourceRange().getEnd()); - Sema::OwningExprResult Result( - SemaRef.ActOnCXXTypeConstructExpr(SourceRange(E->getTypeBeginLoc() - /*, FIXME*/), - T.getAsOpaquePtr(), - /*FIXME*/E->getTypeBeginLoc(), - Sema::MultiExprArg(SemaRef, - (void**)&Args[0], - Args.size()), - /*HACK*/&CommaLoc, - E->getSourceRange().getEnd())); - // At this point, Args no longer owns the arguments, no matter what. - return move(Result); + SourceLocation CommaLoc; + // FIXME: HACK! + if (Args.size() > 1) { + Expr *First = (Expr *)Args[0]; + CommaLoc + = SemaRef.PP.getLocForEndOfToken(First->getSourceRange().getEnd()); } - - // Clean up the instantiated arguments. - // FIXME: Would rather do this with RAII. - for (unsigned Idx = 0; Idx < Args.size(); ++Idx) - SemaRef.DeleteExpr(Args[Idx]); - - return SemaRef.ExprError(); + return SemaRef.ActOnCXXTypeConstructExpr(SourceRange(E->getTypeBeginLoc() + /*, FIXME*/), + T.getAsOpaquePtr(), + /*FIXME*/E->getTypeBeginLoc(), + Sema::MultiExprArg(SemaRef, + Args.take(), + Args.size()), + /*HACK*/&CommaLoc, + E->getSourceRange().getEnd()); } Sema::OwningExprResult TemplateExprInstantiator::VisitCastExpr(CastExpr *E) { @@ -847,42 +826,30 @@ TemplateExprInstantiator::VisitCXXConstructExpr(CXXConstructExpr *E) { if (T.isNull()) return SemaRef.ExprError(); - bool Invalid = false; - llvm::SmallVector Args; + ASTOwningVector<&ActionBase::DeleteExpr> Args(SemaRef); for (CXXConstructExpr::arg_iterator Arg = E->arg_begin(), ArgEnd = E->arg_end(); Arg != ArgEnd; ++Arg) { OwningExprResult ArgInst = Visit(*Arg); - if (ArgInst.isInvalid()) { - Invalid = true; - break; - } + if (ArgInst.isInvalid()) + return SemaRef.ExprError(); Args.push_back(ArgInst.takeAs()); } - VarDecl *Var = 0; - if (!Invalid) { - Var = cast_or_null(SemaRef.InstantiateDecl(E->getVarDecl(), - SemaRef.CurContext, - TemplateArgs)); - if (!Var) - Invalid = true; - } - - if (Invalid) { - for (unsigned I = 0, N = Args.size(); I != N; ++I) - Args[I]->Destroy(SemaRef.Context); - + VarDecl *Var = cast_or_null(SemaRef.InstantiateDecl(E->getVarDecl(), + SemaRef.CurContext, + TemplateArgs)); + if (!Var) return SemaRef.ExprError(); - } SemaRef.CurrentInstantiationScope->InstantiatedLocal(E->getVarDecl(), Var); return SemaRef.Owned(CXXConstructExpr::Create(SemaRef.Context, Var, T, E->getConstructor(), E->isElidable(), - &Args.front(), Args.size())); + Args.takeAs(), + Args.size())); } Sema::OwningExprResult @@ -937,17 +904,14 @@ TemplateExprInstantiator::VisitCXXUnresolvedConstructExpr( if (T.isNull()) return SemaRef.ExprError(); - llvm::SmallVector Args; + ASTOwningVector<&ActionBase::DeleteExpr> Args(SemaRef); llvm::SmallVector FakeCommaLocs; for (CXXUnresolvedConstructExpr::arg_iterator Arg = E->arg_begin(), ArgEnd = E->arg_end(); Arg != ArgEnd; ++Arg) { OwningExprResult InstArg = Visit(*Arg); - if (InstArg.isInvalid()) { - for (unsigned I = 0; I != Args.size(); ++I) - Args[I]->Destroy(SemaRef.Context); + if (InstArg.isInvalid()) return SemaRef.ExprError(); - } FakeCommaLocs.push_back( SemaRef.PP.getLocForEndOfToken((*Arg)->getSourceRange().getEnd())); @@ -961,7 +925,7 @@ TemplateExprInstantiator::VisitCXXUnresolvedConstructExpr( T.getAsOpaquePtr(), E->getLParenLoc(), Sema::MultiExprArg(SemaRef, - (void **)&Args.front(), + Args.take(), Args.size()), &FakeCommaLocs.front(), E->getRParenLoc()); diff --git a/lib/Sema/SemaTemplateInstantiateStmt.cpp b/lib/Sema/SemaTemplateInstantiateStmt.cpp index c136d25447..e83718e33d 100644 --- a/lib/Sema/SemaTemplateInstantiateStmt.cpp +++ b/lib/Sema/SemaTemplateInstantiateStmt.cpp @@ -136,29 +136,22 @@ TemplateStmtInstantiator::VisitReturnStmt(ReturnStmt *S) { Sema::OwningStmtResult TemplateStmtInstantiator::VisitCompoundStmt(CompoundStmt *S) { - // FIXME: We need an *easy* RAII way to delete these statements if something - // goes wrong. - llvm::SmallVector Statements; + ASTOwningVector<&ActionBase::DeleteStmt> Statements(SemaRef); for (CompoundStmt::body_iterator B = S->body_begin(), BEnd = S->body_end(); B != BEnd; ++B) { OwningStmtResult Result = Visit(*B); - if (Result.isInvalid()) { - // FIXME: This should be handled by an RAII destructor. - for (unsigned I = 0, N = Statements.size(); I != N; ++I) - Statements[I]->Destroy(SemaRef.Context); + if (Result.isInvalid()) return SemaRef.StmtError(); - } Statements.push_back(Result.takeAs()); } - return SemaRef.Owned( - new (SemaRef.Context) CompoundStmt(SemaRef.Context, - &Statements[0], - Statements.size(), - S->getLBracLoc(), - S->getRBracLoc())); + return SemaRef.ActOnCompoundStmt(S->getLBracLoc(), S->getRBracLoc(), + Sema::MultiStmtArg(SemaRef, + Statements.take(), + Statements.size()), + /*isStmtExpr=*/false); } Sema::OwningStmtResult @@ -326,22 +319,18 @@ TemplateStmtInstantiator::VisitCXXTryStmt(CXXTryStmt *S) { return SemaRef.StmtError(); // Instantiate the handlers. - llvm::SmallVector Handlers; + ASTOwningVector<&ActionBase::DeleteStmt> Handlers(SemaRef); for (unsigned I = 0, N = S->getNumHandlers(); I != N; ++I) { OwningStmtResult Handler = VisitCXXCatchStmt(S->getHandler(I)); - if (Handler.isInvalid()) { - // Destroy all of the previous handlers. - for (unsigned Victim = 0; Victim != I; ++Victim) - Handlers[Victim]->Destroy(SemaRef.Context); + if (Handler.isInvalid()) return SemaRef.StmtError(); - } Handlers.push_back(Handler.takeAs()); } return SemaRef.ActOnCXXTryBlock(S->getTryLoc(), move(TryBlock), Sema::MultiStmtArg(SemaRef, - (void**)&Handlers.front(), + Handlers.take(), Handlers.size())); } diff --git a/test/SemaTemplate/instantiate-function-1.cpp b/test/SemaTemplate/instantiate-function-1.cpp index 6182e1fe88..be8c0e4cbb 100644 --- a/test/SemaTemplate/instantiate-function-1.cpp +++ b/test/SemaTemplate/instantiate-function-1.cpp @@ -2,7 +2,7 @@ template struct X0 { void f(T x, U y) { - x + y; // expected-error{{invalid operands}} + (void)(x + y); // expected-error{{invalid operands}} } };