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 54c04c2c4d7bd3110ce3f0625bf81883a08ccd6a..f593f76e784a4e305deb334f059814ca3a99c8af 100644 (file)
--- 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 <ASTDestroyer Destroyer>
+  class ASTOwningVector : public llvm::SmallVector<void *, 8> {
+#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<typename T> 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 f5c4a7bb63aed7c3008226ea372dfe8b2a60a2aa..e16f544abbb3e8d129516e9ac57507b766158b6b 100644 (file)
--- 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 099ce278f1f452cd530c49d4bb0ae0183e03db41..a93ad1132a6a36b3f68af44b944a00437ba1fd7d 100644 (file)
--- 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<Expr*, 8> Args;
+  ASTOwningVector<&ActionBase::DeleteExpr> Args(SemaRef);
   llvm::SmallVector<SourceLocation, 4> 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<Expr *, 8> 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<Expr>());
   }
@@ -537,7 +529,7 @@ TemplateExprInstantiator::VisitShuffleVectorExpr(ShuffleVectorExpr *E) {
 
   // Build the CallExpr 
   CallExpr *TheCall = new (SemaRef.Context) CallExpr(SemaRef.Context, Callee,
-                                                     &SubExprs[0], 
+                                                     SubExprs.takeAs<Expr>(),
                                                      SubExprs.size(),
                                                      Builtin->getResultType(),
                                                      E->getRParenLoc());
@@ -656,47 +648,34 @@ TemplateExprInstantiator::VisitCXXTemporaryObjectExpr(
       return SemaRef.ExprError();
   }
 
-  llvm::SmallVector<Expr *, 16> 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<Expr *, 8> 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<Expr>());
   }
 
 
-  VarDecl *Var = 0;
-  if (!Invalid) {
-    Var = cast_or_null<VarDecl>(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<VarDecl>(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<Expr>(), 
+                                                Args.size()));
 }
 
 Sema::OwningExprResult 
@@ -937,17 +904,14 @@ TemplateExprInstantiator::VisitCXXUnresolvedConstructExpr(
   if (T.isNull())
     return SemaRef.ExprError();
 
-  llvm::SmallVector<Expr *, 8> Args;
+  ASTOwningVector<&ActionBase::DeleteExpr> Args(SemaRef);
   llvm::SmallVector<SourceLocation, 8> 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 c136d25447833d719d99fba0cc802627394c4e13..e83718e33de2544e0146b39cde0cb01a485df909 100644 (file)
--- 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<Stmt *, 16> 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<Stmt>());
   }
 
-  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<Stmt *, 4> 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<Stmt>());
   }
 
   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 6182e1fe8882a2a94bf587b710bf249fe68f7d00..be8c0e4cbb4d8c9094dc8447d06642e888495022 100644 (file)
--- a/test/SemaTemplate/instantiate-function-1.cpp
+++ b/test/SemaTemplate/instantiate-function-1.cpp
@@ -2,7 +2,7 @@
 template<typename T, typename U>
 struct X0 {
   void f(T x, U y) { 
-    x + y; // expected-error{{invalid operands}}
+    (void)(x + y); // expected-error{{invalid operands}}
   }
 };