Fix a crash-on-valid where a block capture copy expression was

picking up cleanups from earlier in the statement.  Also fix a
crash-on-invalid where a reference to an invalid decl from an
enclosing scope was causing an expression to fail to build, but
only *after* a cleanup was registered from that statement,
causing an assertion downstream.

The crash-on-valid is rdar://13459289.

llvm-svn: 177692

clang/include/clang/Sema/Sema.h

@@ -2637,6 +2637,7 @@ class Sema {
   }
 
   StmtResult ActOnExprStmt(ExprResult Arg);
+  StmtResult ActOnExprStmtError();
 
   StmtResult ActOnNullStmt(SourceLocation SemiLoc,
                            bool HasLeadingEmptyMacro = false);

clang/lib/Parse/ParseStmt.cpp

@@ -319,7 +319,7 @@ StmtResult Parser::ParseExprStatement() {
     SkipUntil(tok::r_brace, /*StopAtSemi=*/true, /*DontConsume=*/true);
     if (Tok.is(tok::semi))
       ConsumeToken();
-    return StmtError();
+    return Actions.ActOnExprStmtError();
   }
 
   if (Tok.is(tok::colon) && getCurScope()->isSwitchScope() &&

clang/lib/Sema/SemaDecl.cpp

@@ -7916,6 +7916,7 @@ void Sema::CheckCompleteVariableDeclaration(VarDecl *var) {
     // Regardless, we don't want to ignore array nesting when
     // constructing this copy.
     if (type->isStructureOrClassType()) {
+      EnterExpressionEvaluationContext scope(*this, PotentiallyEvaluated);
       SourceLocation poi = var->getLocation();
       Expr *varRef =new (Context) DeclRefExpr(var, false, type, VK_LValue, poi);
       ExprResult result

clang/lib/Sema/SemaDeclCXX.cpp

@@ -10288,6 +10288,9 @@ VarDecl *Sema::BuildExceptionDeclaration(Scope *S,
 
   if (!Invalid && !ExDeclType->isDependentType()) {
     if (const RecordType *recordType = ExDeclType->getAs<RecordType>()) {
+      // Insulate this from anything else we might currently be parsing.
+      EnterExpressionEvaluationContext scope(*this, PotentiallyEvaluated);
+
       // C++ [except.handle]p16:
       //   The object declared in an exception-declaration or, if the 
       //   exception-declaration does not specify a name, a temporary (12.2) is 

clang/lib/Sema/SemaExpr.cpp

@@ -10698,7 +10698,7 @@ static ExprResult captureInLambda(Sema &S, LambdaScopeInfo *LSI,
   // Introduce a new evaluation context for the initialization, so
   // that temporaries introduced as part of the capture are retained
   // to be re-"exported" from the lambda expression itself.
-  S.PushExpressionEvaluationContext(Sema::PotentiallyEvaluated);
+  EnterExpressionEvaluationContext scope(S, Sema::PotentiallyEvaluated);
 
   // C++ [expr.prim.labda]p12:
   //   An entity captured by a lambda-expression is odr-used (3.2) in
@@ -10749,7 +10749,6 @@ static ExprResult captureInLambda(Sema &S, LambdaScopeInfo *LSI,
     if (Subscript.isInvalid()) {
       S.CleanupVarDeclMarking();
       S.DiscardCleanupsInEvaluationContext();
-      S.PopExpressionEvaluationContext();
       return ExprError();
     }
 
@@ -10785,7 +10784,6 @@ static ExprResult captureInLambda(Sema &S, LambdaScopeInfo *LSI,
   // Exit the expression evaluation context used for the capture.
   S.CleanupVarDeclMarking();
   S.DiscardCleanupsInEvaluationContext();
-  S.PopExpressionEvaluationContext();
   return Result;
 }
 
@@ -10972,6 +10970,10 @@ bool Sema::tryCaptureVariable(VarDecl *Var, SourceLocation Loc,
             if (isa<ParmVarDecl>(Var))
               FinalizeVarWithDestructor(Var, Record);
 
+            // Enter a new evaluation context to insulate the copy
+            // full-expression.
+            EnterExpressionEvaluationContext scope(*this, PotentiallyEvaluated);
+
             // According to the blocks spec, the capture of a variable from
             // the stack requires a const copy constructor.  This is not true
             // of the copy/move done to move a __block variable to the heap.

clang/lib/Sema/SemaStmt.cpp

@@ -54,6 +54,11 @@ StmtResult Sema::ActOnExprStmt(ExprResult FE) {
 }
 
 
+StmtResult Sema::ActOnExprStmtError() {
+  DiscardCleanupsInEvaluationContext();
+  return StmtError();
+}
+
 StmtResult Sema::ActOnNullStmt(SourceLocation SemiLoc,
                                bool HasLeadingEmptyMacro) {
   return Owned(new (Context) NullStmt(SemiLoc, HasLeadingEmptyMacro));

clang/test/CodeGenCXX/blocks.cpp

@@ -228,3 +228,28 @@ namespace test8 {
 
   template int X::foo<int>();
 }
+
+// rdar://13459289
+namespace test9 {
+  struct B {
+    void *p;
+    B();
+    B(const B&);
+    ~B();
+  };
+
+  void use_block(void (^)());
+  void use_block_2(void (^)(), const B &a);
+
+  // Ensuring that creating a non-trivial capture copy expression
+  // doesn't end up stealing the block registration for the block we
+  // just parsed.  That block must have captures or else it won't
+  // force registration.  Must occur within a block for some reason.
+  void test() {
+    B x;
+    use_block(^{
+        int y;
+        use_block_2(^{ (void)y; }, x);
+    });
+  }
+}

clang/test/SemaCXX/blocks.cpp

@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s -fblocks
-// expected-no-diagnostics
 
 void tovoid(void*);
 
@@ -82,3 +81,22 @@ void move_block() {
   __block MoveOnly mo;
 }
 
+// Don't crash after failing to build a block due to a capture of an
+// invalid declaration.
+namespace test5 {
+  struct B { // expected-note 2 {{candidate constructor}}
+    void *p;
+    B(int); // expected-note {{candidate constructor}}
+  };
+
+  void use_block(void (^)());
+  void use_block_2(void (^)(), const B &a);
+
+  void test() {
+    B x; // expected-error {{no matching constructor for initialization}}
+    use_block(^{
+        int y;
+        use_block_2(^{ (void) y; }, x);
+      });
+  }
+}