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.

llvm-svn: 144263

5 files changed, 58 insertions, 35 deletions

diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 7d28026402a..d36b67aed85 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/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/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 6fbdb5163cf..ad290fa5687 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/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/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 8419cbebc4b..f59fa5bb47d 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/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/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 4ba294cb8a1..bc422db8b91 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/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/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 72494009658..6c533c4ed49 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/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) {