There's no good reason to track temporaries in ExprWithCleanups,

but it is sometimes useful to track blocks.  Do so.  Also
optimize the storage of these expressions.



git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@144263 91177308-0d34-0410-b5e6-96231b3b80d8

10 files changed, 95 insertions, 82 deletions

diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
index c766577ab0..095491aafe 100644
--- a/lib/AST/Decl.cpp
+++ b/lib/AST/Decl.cpp
@@ -1412,21 +1412,6 @@ Expr *ParmVarDecl::getDefaultArg() {
   return Arg;
 }
 
-unsigned ParmVarDecl::getNumDefaultArgTemporaries() const {
-  if (const ExprWithCleanups *E = dyn_cast<ExprWithCleanups>(getInit()))
-    return E->getNumTemporaries();
-
-  return 0;
-}
-
-CXXTemporary *ParmVarDecl::getDefaultArgTemporary(unsigned i) {
-  assert(getNumDefaultArgTemporaries() && 
-         "Default arguments does not have any temporaries!");
-
-  ExprWithCleanups *E = cast<ExprWithCleanups>(getInit());
-  return E->getTemporary(i);
-}
-
 SourceRange ParmVarDecl::getDefaultArgRange() const {
   if (const Expr *E = getInit())
     return E->getSourceRange();
diff --git a/lib/AST/ExprCXX.cpp b/lib/AST/ExprCXX.cpp
index ad5ec8b833..99e9002a48 100644
--- a/lib/AST/ExprCXX.cpp
+++ b/lib/AST/ExprCXX.cpp
@@ -695,35 +695,37 @@ CXXConstructExpr::CXXConstructExpr(ASTContext &C, StmtClass SC, QualType T,
   }
 }
 
-ExprWithCleanups::ExprWithCleanups(ASTContext &C,
-                                   Expr *subexpr,
-                                   CXXTemporary **temps,
-                                   unsigned numtemps)
+ExprWithCleanups::ExprWithCleanups(Expr *subexpr,
+                                   ArrayRef<CleanupObject> objects)
   : Expr(ExprWithCleanupsClass, subexpr->getType(),
          subexpr->getValueKind(), subexpr->getObjectKind(),
          subexpr->isTypeDependent(), subexpr->isValueDependent(),
          subexpr->isInstantiationDependent(),
          subexpr->containsUnexpandedParameterPack()),
-    SubExpr(subexpr), Temps(0), NumTemps(0) {
-  if (numtemps) {
-    setNumTemporaries(C, numtemps);
-    for (unsigned i = 0; i != numtemps; ++i)
-      Temps[i] = temps[i];
-  }
+    SubExpr(subexpr) {
+  ExprWithCleanupsBits.NumObjects = objects.size();
+  for (unsigned i = 0, e = objects.size(); i != e; ++i)
+    getObjectsBuffer()[i] = objects[i];
 }
 
-void ExprWithCleanups::setNumTemporaries(ASTContext &C, unsigned N) {
-  assert(Temps == 0 && "Cannot resize with this");
-  NumTemps = N;
-  Temps = new (C) CXXTemporary*[NumTemps];
+ExprWithCleanups *ExprWithCleanups::Create(ASTContext &C, Expr *subexpr,
+                                           ArrayRef<CleanupObject> objects) {
+  size_t size = sizeof(ExprWithCleanups)
+              + objects.size() * sizeof(CleanupObject);
+  void *buffer = C.Allocate(size, llvm::alignOf<ExprWithCleanups>());
+  return new (buffer) ExprWithCleanups(subexpr, objects);
 }
 
+ExprWithCleanups::ExprWithCleanups(EmptyShell empty, unsigned numObjects)
+  : Expr(ExprWithCleanupsClass, empty) {
+  ExprWithCleanupsBits.NumObjects = numObjects;
+}
 
-ExprWithCleanups *ExprWithCleanups::Create(ASTContext &C,
-                                           Expr *SubExpr,
-                                           CXXTemporary **Temps,
-                                           unsigned NumTemps) {
-  return new (C) ExprWithCleanups(C, SubExpr, Temps, NumTemps);
+ExprWithCleanups *ExprWithCleanups::Create(ASTContext &C, EmptyShell empty,
+                                           unsigned numObjects) {
+  size_t size = sizeof(ExprWithCleanups) + numObjects * sizeof(CleanupObject);
+  void *buffer = C.Allocate(size, llvm::alignOf<ExprWithCleanups>());
+  return new (buffer) ExprWithCleanups(empty, numObjects);
 }
 
 CXXUnresolvedConstructExpr::CXXUnresolvedConstructExpr(TypeSourceInfo *Type,
diff --git a/lib/AST/StmtDumper.cpp b/lib/AST/StmtDumper.cpp
index 2248343199..bfbacf4f30 100644
--- a/lib/AST/StmtDumper.cpp
+++ b/lib/AST/StmtDumper.cpp
@@ -590,10 +590,12 @@ void StmtDumper::VisitCXXBindTemporaryExpr(CXXBindTemporaryExpr *Node) {
 void StmtDumper::VisitExprWithCleanups(ExprWithCleanups *Node) {
   DumpExpr(Node);
   ++IndentLevel;
-  for (unsigned i = 0, e = Node->getNumTemporaries(); i != e; ++i) {
+  for (unsigned i = 0, e = Node->getNumObjects(); i != e; ++i) {
     OS << "\n";
     Indent();
-    DumpCXXTemporary(Node->getTemporary(i));
+    OS << "(cleanup ";
+    DumpDeclRef(Node->getObject(i));
+    OS << ")";
   }
   --IndentLevel;
 }
diff --git a/lib/Sema/Sema.cpp b/lib/Sema/Sema.cpp
index 7d28026402..d36b67aed8 100644
--- a/lib/Sema/Sema.cpp
+++ b/lib/Sema/Sema.cpp
@@ -97,7 +97,7 @@ Sema::Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer,
     CollectStats(false), ExternalSource(0), CodeCompleter(CodeCompleter),
     CurContext(0), OriginalLexicalContext(0),
     PackContext(0), MSStructPragmaOn(false), VisContext(0),
-    ExprNeedsCleanups(0), LateTemplateParser(0), OpaqueParser(0),
+    ExprNeedsCleanups(false), LateTemplateParser(0), OpaqueParser(0),
     IdResolver(pp), CXXTypeInfoDecl(0), MSVCGuidDecl(0),
     GlobalNewDeleteDeclared(false), 
     ObjCShouldCallSuperDealloc(false),
diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index 6fbdb5163c..ad290fa568 100644
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -2676,6 +2676,9 @@ static Expr *EvalAddr(Expr *E, SmallVectorImpl<DeclRefExpr *> &refVars) {
   case Stmt::AddrLabelExprClass:
     return E; // address of label.
 
+  case Stmt::ExprWithCleanupsClass:
+    return EvalAddr(cast<ExprWithCleanups>(E)->getSubExpr(), refVars);
+
   // For casts, we need to handle conversions from arrays to
   // pointer values, and pointer-to-pointer conversions.
   case Stmt::ImplicitCastExprClass:
@@ -2752,6 +2755,9 @@ do {
     return NULL;
   }
 
+  case Stmt::ExprWithCleanupsClass:
+    return EvalVal(cast<ExprWithCleanups>(E)->getSubExpr(), refVars);
+
   case Stmt::DeclRefExprClass: {
     // When we hit a DeclRefExpr we are looking at code that refers to a
     // variable's name. If it's not a reference variable we check if it has
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 8419cbebc4..f59fa5bb47 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -7190,8 +7190,7 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body,
     // deletion in some later function.
     if (PP.getDiagnostics().hasErrorOccurred() ||
         PP.getDiagnostics().getSuppressAllDiagnostics()) {
-      ExprTemporaries.clear();
-      ExprNeedsCleanups = false;
+      DiscardCleanupsInEvaluationContext();
     } else if (!isa<FunctionTemplateDecl>(dcl)) {
       // Since the body is valid, issue any analysis-based warnings that are
       // enabled.
@@ -7202,7 +7201,7 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body,
         !CheckConstexprFunctionBody(FD, Body))
       FD->setInvalidDecl();
 
-    assert(ExprTemporaries.empty() && "Leftover temporaries in function");
+    assert(ExprCleanupObjects.empty() && "Leftover temporaries in function");
     assert(!ExprNeedsCleanups && "Unaccounted cleanups in function");
   }
   
@@ -7215,8 +7214,7 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body,
   // been leftover. This ensures that these temporaries won't be picked up for
   // deletion in some later function.
   if (getDiagnostics().hasErrorOccurred()) {
-    ExprTemporaries.clear();
-    ExprNeedsCleanups = false;
+    DiscardCleanupsInEvaluationContext();
   }
 
   return dcl;
diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 4ba294cb8a..bc422db8b9 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -3291,12 +3291,18 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc,
   // be properly destroyed.
   // FIXME: We should really be rebuilding the default argument with new
   // bound temporaries; see the comment in PR5810.
-  for (unsigned i = 0, e = Param->getNumDefaultArgTemporaries(); i != e; ++i) {
-    CXXTemporary *Temporary = Param->getDefaultArgTemporary(i);
-    MarkDeclarationReferenced(Param->getDefaultArg()->getLocStart(), 
-                    const_cast<CXXDestructorDecl*>(Temporary->getDestructor()));
-    ExprTemporaries.push_back(Temporary);
+  // We don't need to do that with block decls, though, because
+  // blocks in default argument expression can never capture anything.
+  if (isa<ExprWithCleanups>(Param->getInit())) {
+    // Set the "needs cleanups" bit regardless of whether there are
+    // any explicit objects.
     ExprNeedsCleanups = true;
+
+    // Append all the objects to the cleanup list.  Right now, this
+    // should always be a no-op, because blocks in default argument
+    // expressions should never be able to capture anything.
+    assert(!cast<ExprWithCleanups>(Param->getInit())->getNumObjects() &&
+           "default argument expression has capturing blocks?");
   }
 
   // We already type-checked the argument, so we know it works. 
@@ -8828,6 +8834,13 @@ ExprResult Sema::ActOnBlockStmtExpr(SourceLocation CaretLoc,
   const AnalysisBasedWarnings::Policy &WP = AnalysisWarnings.getDefaultPolicy();
   PopFunctionOrBlockScope(&WP, Result->getBlockDecl(), Result);
 
+  // If the block isn't obviously global, i.e. it captures anything at
+  // all, mark this full-expression as needing a cleanup.
+  if (Result->getBlockDecl()->hasCaptures()) {
+    ExprCleanupObjects.push_back(Result->getBlockDecl());
+    ExprNeedsCleanups = true;
+  }
+
   return Owned(Result);
 }
 
@@ -9158,7 +9171,7 @@ void
 Sema::PushExpressionEvaluationContext(ExpressionEvaluationContext NewContext) {
   ExprEvalContexts.push_back(
              ExpressionEvaluationContextRecord(NewContext,
-                                               ExprTemporaries.size(),
+                                               ExprCleanupObjects.size(),
                                                ExprNeedsCleanups));
   ExprNeedsCleanups = false;
 }
@@ -9195,8 +9208,8 @@ void Sema::PopExpressionEvaluationContext() {
   // the expression in that context: they aren't relevant because they
   // will never be constructed.
   if (Rec.Context == Unevaluated) {
-    ExprTemporaries.erase(ExprTemporaries.begin() + Rec.NumTemporaries,
-                          ExprTemporaries.end());
+    ExprCleanupObjects.erase(ExprCleanupObjects.begin() + Rec.NumCleanupObjects,
+                             ExprCleanupObjects.end());
     ExprNeedsCleanups = Rec.ParentNeedsCleanups;
 
   // Otherwise, merge the contexts together.
@@ -9209,9 +9222,9 @@ void Sema::PopExpressionEvaluationContext() {
 }
 
 void Sema::DiscardCleanupsInEvaluationContext() {
-  ExprTemporaries.erase(
-              ExprTemporaries.begin() + ExprEvalContexts.back().NumTemporaries,
-              ExprTemporaries.end());
+  ExprCleanupObjects.erase(
+         ExprCleanupObjects.begin() + ExprEvalContexts.back().NumCleanupObjects,
+         ExprCleanupObjects.end());
   ExprNeedsCleanups = false;
 }
 
@@ -9454,6 +9467,12 @@ namespace {
       Inherited::VisitMemberExpr(E);
     }
     
+    void VisitCXXBindTemporaryExpr(CXXBindTemporaryExpr *E) {
+      S.MarkDeclarationReferenced(E->getLocStart(),
+            const_cast<CXXDestructorDecl*>(E->getTemporary()->getDestructor()));
+      Visit(E->getSubExpr());
+    }
+    
     void VisitCXXNewExpr(CXXNewExpr *E) {
       if (E->getConstructor())
         S.MarkDeclarationReferenced(E->getLocStart(), E->getConstructor());
diff --git a/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp
index 7249400965..6c533c4ed4 100644
--- a/lib/Sema/SemaExprCXX.cpp
+++ b/lib/Sema/SemaExprCXX.cpp
@@ -4122,37 +4122,37 @@ ExprResult Sema::MaybeBindToTemporary(Expr *E) {
                           PDiag(diag::err_access_dtor_temp)
                             << E->getType());
 
-    ExprTemporaries.push_back(Temp);
+    // We need a cleanup, but we don't need to remember the temporary.
     ExprNeedsCleanups = true;
   }
   return Owned(CXXBindTemporaryExpr::Create(Context, Temp, E));
 }
 
+ExprResult
+Sema::MaybeCreateExprWithCleanups(ExprResult SubExpr) {
+  if (SubExpr.isInvalid())
+    return ExprError();
+
+  return Owned(MaybeCreateExprWithCleanups(SubExpr.take()));
+}
+
 Expr *Sema::MaybeCreateExprWithCleanups(Expr *SubExpr) {
   assert(SubExpr && "sub expression can't be null!");
 
-  unsigned FirstTemporary = ExprEvalContexts.back().NumTemporaries;
-  assert(ExprTemporaries.size() >= FirstTemporary);
-  assert(ExprNeedsCleanups || ExprTemporaries.size() == FirstTemporary);
+  unsigned FirstCleanup = ExprEvalContexts.back().NumCleanupObjects;
+  assert(ExprCleanupObjects.size() >= FirstCleanup);
+  assert(ExprNeedsCleanups || ExprCleanupObjects.size() == FirstCleanup);
   if (!ExprNeedsCleanups)
     return SubExpr;
 
-  Expr *E = ExprWithCleanups::Create(Context, SubExpr,
-                                     ExprTemporaries.begin() + FirstTemporary,
-                                     ExprTemporaries.size() - FirstTemporary);
-  ExprTemporaries.erase(ExprTemporaries.begin() + FirstTemporary,
-                        ExprTemporaries.end());
-  ExprNeedsCleanups = false;
+  ArrayRef<ExprWithCleanups::CleanupObject> Cleanups
+    = llvm::makeArrayRef(ExprCleanupObjects.begin() + FirstCleanup,
+                         ExprCleanupObjects.size() - FirstCleanup);
 
-  return E;
-}
-
-ExprResult
-Sema::MaybeCreateExprWithCleanups(ExprResult SubExpr) {
-  if (SubExpr.isInvalid())
-    return ExprError();
+  Expr *E = ExprWithCleanups::Create(Context, SubExpr, Cleanups);
+  DiscardCleanupsInEvaluationContext();
 
-  return Owned(MaybeCreateExprWithCleanups(SubExpr.take()));
+  return E;
 }
 
 Stmt *Sema::MaybeCreateStmtWithCleanups(Stmt *SubStmt) {
diff --git a/lib/Serialization/ASTReaderStmt.cpp b/lib/Serialization/ASTReaderStmt.cpp
index ff306b06c9..cd51b06a5a 100644
--- a/lib/Serialization/ASTReaderStmt.cpp
+++ b/lib/Serialization/ASTReaderStmt.cpp
@@ -1195,13 +1195,13 @@ void ASTStmtReader::VisitCXXPseudoDestructorExpr(CXXPseudoDestructorExpr *E) {
 
 void ASTStmtReader::VisitExprWithCleanups(ExprWithCleanups *E) {
   VisitExpr(E);
-  unsigned NumTemps = Record[Idx++];
-  if (NumTemps) {
-    E->setNumTemporaries(Reader.getContext(), NumTemps);
-    for (unsigned i = 0; i != NumTemps; ++i)
-      E->setTemporary(i, Reader.ReadCXXTemporary(F, Record, Idx));
-  }
-  E->setSubExpr(Reader.ReadSubExpr());
+
+  unsigned NumObjects = Record[Idx++];
+  assert(NumObjects == E->getNumObjects());
+  for (unsigned i = 0; i != NumObjects; ++i)
+    E->getObjectsBuffer()[i] = ReadDeclAs<BlockDecl>(Record, Idx);
+
+  E->SubExpr = Reader.ReadSubExpr();
 }
 
 void
@@ -1974,7 +1974,8 @@ Stmt *ASTReader::ReadStmtFromStream(Module &F) {
       break;
         
     case EXPR_EXPR_WITH_CLEANUPS:
-      S = new (Context) ExprWithCleanups(Empty);
+      S = ExprWithCleanups::Create(Context, Empty,
+                                   Record[ASTStmtReader::NumExprFields]);
       break;
       
     case EXPR_CXX_DEPENDENT_SCOPE_MEMBER:
diff --git a/lib/Serialization/ASTWriterStmt.cpp b/lib/Serialization/ASTWriterStmt.cpp
index a8c76b5114..e25a20b353 100644
--- a/lib/Serialization/ASTWriterStmt.cpp
+++ b/lib/Serialization/ASTWriterStmt.cpp
@@ -1180,9 +1180,9 @@ void ASTStmtWriter::VisitCXXPseudoDestructorExpr(CXXPseudoDestructorExpr *E) {
 
 void ASTStmtWriter::VisitExprWithCleanups(ExprWithCleanups *E) {
   VisitExpr(E);
-  Record.push_back(E->getNumTemporaries());
-  for (unsigned i = 0, e = E->getNumTemporaries(); i != e; ++i)
-    Writer.AddCXXTemporary(E->getTemporary(i), Record);
+  Record.push_back(E->getNumObjects());
+  for (unsigned i = 0, e = E->getNumObjects(); i != e; ++i)
+    Writer.AddDeclRef(E->getObject(i), Record);
   
   Writer.AddStmt(E->getSubExpr());
   Code = serialization::EXPR_EXPR_WITH_CLEANUPS;