PR40642: Fix determination of whether the final statement of a statement

expression is a discarded-value expression.

Summary:
We used to get this wrong in three ways:

1) During parsing, an expression-statement followed by the }) ending a
   statement expression was always treated as producing the value of the
   statement expression. That's wrong for ({ if (1) expr; })
2) During template instantiation, various kinds of statement (most
   statements not appearing directly in a compound-statement) were not
   treated as discarded-value expressions, resulting in missing volatile
   loads (etc).
3) In all contexts, an expression-statement with attributes was not
   treated as producing the value of the statement expression, eg
   ({ [[attr]] expr; }).

Also fix incorrect enforcement of OpenMP rule that directives can "only
be placed in the program at a position where ignoring or deleting the
directive would result in a program with correct syntax". In particular,
a label (be it goto, case, or default) should not affect whether
directives are permitted.

Reviewers: aaron.ballman, rjmccall

Subscribers: cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D57984

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

21 files changed, 387 insertions, 218 deletions

diff --git a/include/clang/AST/Expr.h b/include/clang/AST/Expr.h
index 4fec8efd2d..95804173c8 100644
--- a/include/clang/AST/Expr.h
+++ b/include/clang/AST/Expr.h
@@ -105,13 +105,13 @@ struct SubobjectAdjustment {
 /// This represents one expression.  Note that Expr's are subclasses of Stmt.
 /// This allows an expression to be transparently used any place a Stmt is
 /// required.
-class Expr : public Stmt {
+class Expr : public ValueStmt {
   QualType TR;
 
 protected:
   Expr(StmtClass SC, QualType T, ExprValueKind VK, ExprObjectKind OK,
        bool TD, bool VD, bool ID, bool ContainsUnexpandedParameterPack)
-    : Stmt(SC)
+    : ValueStmt(SC)
   {
     ExprBits.TypeDependent = TD;
     ExprBits.ValueDependent = VD;
@@ -124,7 +124,7 @@ protected:
   }
 
   /// Construct an empty expression.
-  explicit Expr(StmtClass SC, EmptyShell) : Stmt(SC) { }
+  explicit Expr(StmtClass SC, EmptyShell) : ValueStmt(SC) { }
 
 public:
   QualType getType() const { return TR; }
diff --git a/include/clang/AST/Stmt.h b/include/clang/AST/Stmt.h
index b892d85206..e63354c263 100644
--- a/include/clang/AST/Stmt.h
+++ b/include/clang/AST/Stmt.h
@@ -1584,21 +1584,44 @@ Stmt *SwitchCase::getSubStmt() {
   llvm_unreachable("SwitchCase is neither a CaseStmt nor a DefaultStmt!");
 }
 
+/// Represents a statement that could possibly have a value and type. This
+/// covers expression-statements, as well as labels and attributed statements.
+///
+/// Value statements have a special meaning when they are the last non-null
+/// statement in a GNU statement expression, where they determine the value
+/// of the statement expression.
+class ValueStmt : public Stmt {
+protected:
+  using Stmt::Stmt;
+
+public:
+  const Expr *getExprStmt() const;
+  Expr *getExprStmt() {
+    const ValueStmt *ConstThis = this;
+    return const_cast<Expr*>(ConstThis->getExprStmt());
+  }
+
+  static bool classof(const Stmt *T) {
+    return T->getStmtClass() >= firstValueStmtConstant &&
+           T->getStmtClass() <= lastValueStmtConstant;
+  }
+};
+
 /// LabelStmt - Represents a label, which has a substatement.  For example:
 ///    foo: return;
-class LabelStmt : public Stmt {
+class LabelStmt : public ValueStmt {
   LabelDecl *TheDecl;
   Stmt *SubStmt;
 
 public:
   /// Build a label statement.
   LabelStmt(SourceLocation IL, LabelDecl *D, Stmt *substmt)
-      : Stmt(LabelStmtClass), TheDecl(D), SubStmt(substmt) {
+      : ValueStmt(LabelStmtClass), TheDecl(D), SubStmt(substmt) {
     setIdentLoc(IL);
   }
 
   /// Build an empty label statement.
-  explicit LabelStmt(EmptyShell Empty) : Stmt(LabelStmtClass, Empty) {}
+  explicit LabelStmt(EmptyShell Empty) : ValueStmt(LabelStmtClass, Empty) {}
 
   SourceLocation getIdentLoc() const { return LabelStmtBits.IdentLoc; }
   void setIdentLoc(SourceLocation L) { LabelStmtBits.IdentLoc = L; }
@@ -1627,7 +1650,7 @@ public:
 /// Represents an attribute applied to a statement. For example:
 ///   [[omp::for(...)]] for (...) { ... }
 class AttributedStmt final
-    : public Stmt,
+    : public ValueStmt,
       private llvm::TrailingObjects<AttributedStmt, const Attr *> {
   friend class ASTStmtReader;
   friend TrailingObjects;
@@ -1636,14 +1659,14 @@ class AttributedStmt final
 
   AttributedStmt(SourceLocation Loc, ArrayRef<const Attr *> Attrs,
                  Stmt *SubStmt)
-      : Stmt(AttributedStmtClass), SubStmt(SubStmt) {
+      : ValueStmt(AttributedStmtClass), SubStmt(SubStmt) {
     AttributedStmtBits.NumAttrs = Attrs.size();
     AttributedStmtBits.AttrLoc = Loc;
     std::copy(Attrs.begin(), Attrs.end(), getAttrArrayPtr());
   }
 
   explicit AttributedStmt(EmptyShell Empty, unsigned NumAttrs)
-      : Stmt(AttributedStmtClass, Empty) {
+      : ValueStmt(AttributedStmtClass, Empty) {
     AttributedStmtBits.NumAttrs = NumAttrs;
     AttributedStmtBits.AttrLoc = SourceLocation{};
     std::fill_n(getAttrArrayPtr(), NumAttrs, nullptr);
diff --git a/include/clang/Basic/StmtNodes.td b/include/clang/Basic/StmtNodes.td
index 9054fb11a6..2dbbee7bbf 100644
--- a/include/clang/Basic/StmtNodes.td
+++ b/include/clang/Basic/StmtNodes.td
@@ -11,8 +11,6 @@ class DStmt<Stmt base, bit abstract = 0> : Stmt<abstract> {
 // Statements
 def NullStmt : Stmt;
 def CompoundStmt : Stmt;
-def LabelStmt : Stmt;
-def AttributedStmt : Stmt;
 def IfStmt : Stmt;
 def SwitchStmt : Stmt;
 def WhileStmt : Stmt;
@@ -29,6 +27,12 @@ def CaseStmt : DStmt<SwitchCase>;
 def DefaultStmt : DStmt<SwitchCase>;
 def CapturedStmt : Stmt;
 
+// Statements that might produce a value (for example, as the last non-null
+// statement in a GNU statement-expression).
+def ValueStmt : Stmt<1>;
+def LabelStmt : DStmt<ValueStmt>;
+def AttributedStmt : DStmt<ValueStmt>;
+
 // Asm statements
 def AsmStmt : Stmt<1>;
 def GCCAsmStmt : DStmt<AsmStmt>;
@@ -53,7 +57,7 @@ def CoroutineBodyStmt : Stmt;
 def CoreturnStmt : Stmt;
 
 // Expressions
-def Expr : Stmt<1>;
+def Expr : DStmt<ValueStmt, 1>;
 def PredefinedExpr : DStmt<Expr>;
 def DeclRefExpr : DStmt<Expr>;
 def IntegerLiteral : DStmt<Expr>;
diff --git a/include/clang/Parse/Parser.h b/include/clang/Parse/Parser.h
index f5c70e71b8..db0217324b 100644
--- a/include/clang/Parse/Parser.h
+++ b/include/clang/Parse/Parser.h
@@ -363,10 +363,28 @@ class Parser : public CodeCompletionHandler {
   /// just a regular sub-expression.
   SourceLocation ExprStatementTokLoc;
 
-  /// Tests whether an expression value is discarded based on token lookahead.
-  /// It will return true if the lexer is currently processing the })
-  /// terminating a GNU statement expression and false otherwise.
-  bool isExprValueDiscarded();
+  /// Flags describing a context in which we're parsing a statement.
+  enum class ParsedStmtContext {
+    /// This context permits declarations in language modes where declarations
+    /// are not statements.
+    AllowDeclarationsInC = 0x1,
+    /// This context permits standalone OpenMP directives.
+    AllowStandaloneOpenMPDirectives = 0x2,
+    /// This context is at the top level of a GNU statement expression.
+    InStmtExpr = 0x4,
+
+    /// The context of a regular substatement.
+    SubStmt = 0,
+    /// The context of a compound-statement.
+    Compound = AllowDeclarationsInC | AllowStandaloneOpenMPDirectives,
+
+    LLVM_MARK_AS_BITMASK_ENUM(InStmtExpr)
+  };
+
+  /// Act on an expression statement that might be the last statement in a
+  /// GNU statement expression. Checks whether we are actually at the end of
+  /// a statement expression and builds a suitable expression statement.
+  StmtResult handleExprStmt(ExprResult E, ParsedStmtContext StmtCtx);
 
 public:
   Parser(Preprocessor &PP, Sema &Actions, bool SkipFunctionBodies);
@@ -1873,29 +1891,24 @@ private:
   /// A SmallVector of types.
   typedef SmallVector<ParsedType, 12> TypeVector;
 
-  StmtResult ParseStatement(SourceLocation *TrailingElseLoc = nullptr,
-                            bool AllowOpenMPStandalone = false);
-  enum AllowedConstructsKind {
-    /// Allow any declarations, statements, OpenMP directives.
-    ACK_Any,
-    /// Allow only statements and non-standalone OpenMP directives.
-    ACK_StatementsOpenMPNonStandalone,
-    /// Allow statements and all executable OpenMP directives
-    ACK_StatementsOpenMPAnyExecutable
-  };
   StmtResult
-  ParseStatementOrDeclaration(StmtVector &Stmts, AllowedConstructsKind Allowed,
-                              SourceLocation *TrailingElseLoc = nullptr);
+  ParseStatement(SourceLocation *TrailingElseLoc = nullptr,
+                 ParsedStmtContext StmtCtx = ParsedStmtContext::SubStmt);
+  StmtResult ParseStatementOrDeclaration(
+      StmtVector &Stmts, ParsedStmtContext StmtCtx,
+      SourceLocation *TrailingElseLoc = nullptr);
   StmtResult ParseStatementOrDeclarationAfterAttributes(
                                          StmtVector &Stmts,
-                                         AllowedConstructsKind Allowed,
+                                         ParsedStmtContext StmtCtx,
                                          SourceLocation *TrailingElseLoc,
                                          ParsedAttributesWithRange &Attrs);
-  StmtResult ParseExprStatement();
-  StmtResult ParseLabeledStatement(ParsedAttributesWithRange &attrs);
-  StmtResult ParseCaseStatement(bool MissingCase = false,
+  StmtResult ParseExprStatement(ParsedStmtContext StmtCtx);
+  StmtResult ParseLabeledStatement(ParsedAttributesWithRange &attrs,
+                                   ParsedStmtContext StmtCtx);
+  StmtResult ParseCaseStatement(ParsedStmtContext StmtCtx,
+                                bool MissingCase = false,
                                 ExprResult Expr = ExprResult());
-  StmtResult ParseDefaultStatement();
+  StmtResult ParseDefaultStatement(ParsedStmtContext StmtCtx);
   StmtResult ParseCompoundStatement(bool isStmtExpr = false);
   StmtResult ParseCompoundStatement(bool isStmtExpr,
                                     unsigned ScopeFlags);
@@ -1918,7 +1931,7 @@ private:
   StmtResult ParseAsmStatement(bool &msAsm);
   StmtResult ParseMicrosoftAsmStatement(SourceLocation AsmLoc);
   StmtResult ParsePragmaLoopHint(StmtVector &Stmts,
-                                 AllowedConstructsKind Allowed,
+                                 ParsedStmtContext StmtCtx,
                                  SourceLocation *TrailingElseLoc,
                                  ParsedAttributesWithRange &Attrs);
 
@@ -1984,7 +1997,8 @@ private:
   //===--------------------------------------------------------------------===//
   // Objective-C Statements
 
-  StmtResult ParseObjCAtStatement(SourceLocation atLoc);
+  StmtResult ParseObjCAtStatement(SourceLocation atLoc,
+                                  ParsedStmtContext StmtCtx);
   StmtResult ParseObjCTryStmt(SourceLocation atLoc);
   StmtResult ParseObjCThrowStmt(SourceLocation atLoc);
   StmtResult ParseObjCSynchronizedStmt(SourceLocation atLoc);
@@ -2824,13 +2838,9 @@ private:
       bool AllowScopeSpecifier);
   /// Parses declarative or executable directive.
   ///
-  /// \param Allowed ACK_Any, if any directives are allowed,
-  /// ACK_StatementsOpenMPAnyExecutable - if any executable directives are
-  /// allowed, ACK_StatementsOpenMPNonStandalone - if only non-standalone
-  /// executable directives are allowed.
-  ///
+  /// \param StmtCtx The context in which we're parsing the directive.
   StmtResult
-  ParseOpenMPDeclarativeOrExecutableDirective(AllowedConstructsKind Allowed);
+  ParseOpenMPDeclarativeOrExecutableDirective(ParsedStmtContext StmtCtx);
   /// Parses clause of kind \a CKind for directive of a kind \a Kind.
   ///
   /// \param DKind Kind of current directive.
diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 0c2479a020..10eff5333f 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -1403,7 +1403,6 @@ public:
   void PopCompoundScope();
 
   sema::CompoundScopeInfo &getCurCompoundScope() const;
-  bool isCurCompoundStmtAStmtExpr() const;
 
   bool hasAnyUnrecoverableErrorsInThisFunction() const;
 
@@ -4533,6 +4532,8 @@ public:
   void ActOnStartStmtExpr();
   ExprResult ActOnStmtExpr(SourceLocation LPLoc, Stmt *SubStmt,
                            SourceLocation RPLoc); // "({..})"
+  // Handle the final expression in a statement expression.
+  ExprResult ActOnStmtExprResult(ExprResult E);
   void ActOnStmtExprError();
 
   // __builtin_offsetof(type, identifier(.identifier|[expr])*)
diff --git a/lib/AST/Stmt.cpp b/lib/AST/Stmt.cpp
index d1a4dc074b..f2bb289719 100644
--- a/lib/AST/Stmt.cpp
+++ b/lib/AST/Stmt.cpp
@@ -320,6 +320,23 @@ CompoundStmt *CompoundStmt::CreateEmpty(const ASTContext &C,
   return New;
 }
 
+const Expr *ValueStmt::getExprStmt() const {
+  const Stmt *S = this;
+  do {
+    if (const auto *E = dyn_cast<Expr>(S))
+      return E;
+
+    if (const auto *LS = dyn_cast<LabelStmt>(S))
+      S = LS->getSubStmt();
+    else if (const auto *AS = dyn_cast<AttributedStmt>(S))
+      S = AS->getSubStmt();
+    else
+      llvm_unreachable("unknown kind of ValueStmt");
+  } while (isa<ValueStmt>(S));
+
+  return nullptr;
+}
+
 const char *LabelStmt::getName() const {
   return getDecl()->getIdentifier()->getNameStart();
 }
diff --git a/lib/CodeGen/CGStmt.cpp b/lib/CodeGen/CGStmt.cpp
index db3f4f142d..5b5113b1e7 100644
--- a/lib/CodeGen/CGStmt.cpp
+++ b/lib/CodeGen/CGStmt.cpp
@@ -391,26 +391,35 @@ CodeGenFunction::EmitCompoundStmtWithoutScope(const CompoundStmt &S,
     // at the end of a statement expression, they yield the value of their
     // subexpression.  Handle this by walking through all labels we encounter,
     // emitting them before we evaluate the subexpr.
+    // Similar issues arise for attributed statements.
     const Stmt *LastStmt = S.body_back();
-    while (const LabelStmt *LS = dyn_cast<LabelStmt>(LastStmt)) {
-      EmitLabel(LS->getDecl());
-      LastStmt = LS->getSubStmt();
+    while (!isa<Expr>(LastStmt)) {
+      if (const auto *LS = dyn_cast<LabelStmt>(LastStmt)) {
+        EmitLabel(LS->getDecl());
+        LastStmt = LS->getSubStmt();
+      } else if (const auto *AS = dyn_cast<AttributedStmt>(LastStmt)) {
+        // FIXME: Update this if we ever have attributes that affect the
+        // semantics of an expression.
+        LastStmt = AS->getSubStmt();
+      } else {
+        llvm_unreachable("unknown value statement");
+      }
     }
 
     EnsureInsertPoint();
 
-    QualType ExprTy = cast<Expr>(LastStmt)->getType();
+    const Expr *E = cast<Expr>(LastStmt);
+    QualType ExprTy = E->getType();
     if (hasAggregateEvaluationKind(ExprTy)) {
-      EmitAggExpr(cast<Expr>(LastStmt), AggSlot);
+      EmitAggExpr(E, AggSlot);
     } else {
       // We can't return an RValue here because there might be cleanups at
       // the end of the StmtExpr.  Because of that, we have to emit the result
       // here into a temporary alloca.
       RetAlloca = CreateMemTemp(ExprTy);
-      EmitAnyExprToMem(cast<Expr>(LastStmt), RetAlloca, Qualifiers(),
-                       /*IsInit*/false);
+      EmitAnyExprToMem(E, RetAlloca, Qualifiers(),
+                       /*IsInit*/ false);
     }
-
   }
 
   return RetAlloca;
diff --git a/lib/Parse/ParseObjc.cpp b/lib/Parse/ParseObjc.cpp
index 0a1dbf7bff..5ac01f6f0a 100644
--- a/lib/Parse/ParseObjc.cpp
+++ b/lib/Parse/ParseObjc.cpp
@@ -2703,7 +2703,8 @@ Decl *Parser::ParseObjCMethodDefinition() {
   return MDecl;
 }
 
-StmtResult Parser::ParseObjCAtStatement(SourceLocation AtLoc) {
+StmtResult Parser::ParseObjCAtStatement(SourceLocation AtLoc,
+                                        ParsedStmtContext StmtCtx) {
   if (Tok.is(tok::code_completion)) {
     Actions.CodeCompleteObjCAtStatement(getCurScope());
     cutOffParsing();
@@ -2740,7 +2741,7 @@ StmtResult Parser::ParseObjCAtStatement(SourceLocation AtLoc) {
 
   // Otherwise, eat the semicolon.
   ExpectAndConsumeSemi(diag::err_expected_semi_after_expr);
-  return Actions.ActOnExprStmt(Res, isExprValueDiscarded());
+  return handleExprStmt(Res, StmtCtx);
 }
 
 ExprResult Parser::ParseObjCAtExpression(SourceLocation AtLoc) {
diff --git a/lib/Parse/ParseOpenMP.cpp b/lib/Parse/ParseOpenMP.cpp
index 5b8670e928..d0463416f5 100644
--- a/lib/Parse/ParseOpenMP.cpp
+++ b/lib/Parse/ParseOpenMP.cpp
@@ -1132,8 +1132,8 @@ Parser::DeclGroupPtrTy Parser::ParseOpenMPDeclarativeDirectiveWithExtDecl(
 ///         'target teams distribute simd' {clause}
 ///         annot_pragma_openmp_end
 ///
-StmtResult Parser::ParseOpenMPDeclarativeOrExecutableDirective(
-    AllowedConstructsKind Allowed) {
+StmtResult
+Parser::ParseOpenMPDeclarativeOrExecutableDirective(ParsedStmtContext StmtCtx) {
   assert(Tok.is(tok::annot_pragma_openmp) && "Not an OpenMP directive!");
   ParenBraceBracketBalancer BalancerRAIIObj(*this);
   SmallVector<OMPClause *, 5> Clauses;
@@ -1152,7 +1152,9 @@ StmtResult Parser::ParseOpenMPDeclarativeOrExecutableDirective(
 
   switch (DKind) {
   case OMPD_threadprivate: {
-    if (Allowed != ACK_Any) {
+    // FIXME: Should this be permitted in C++?
+    if ((StmtCtx & ParsedStmtContext::AllowDeclarationsInC) ==
+        ParsedStmtContext()) {
       Diag(Tok, diag::err_omp_immediate_directive)
           << getOpenMPDirectiveName(DKind) << 0;
     }
@@ -1219,7 +1221,8 @@ StmtResult Parser::ParseOpenMPDeclarativeOrExecutableDirective(
   case OMPD_target_enter_data:
   case OMPD_target_exit_data:
   case OMPD_target_update:
-    if (Allowed == ACK_StatementsOpenMPNonStandalone) {
+    if ((StmtCtx & ParsedStmtContext::AllowStandaloneOpenMPDirectives) ==
+        ParsedStmtContext()) {
       Diag(Tok, diag::err_omp_immediate_directive)
           << getOpenMPDirectiveName(DKind) << 0;
     }
@@ -1322,7 +1325,8 @@ StmtResult Parser::ParseOpenMPDeclarativeOrExecutableDirective(
     // If the depend clause is specified, the ordered construct is a stand-alone
     // directive.
     if (DKind == OMPD_ordered && FirstClauses[OMPC_depend].getInt()) {
-      if (Allowed == ACK_StatementsOpenMPNonStandalone) {
+      if ((StmtCtx & ParsedStmtContext::AllowStandaloneOpenMPDirectives) ==
+          ParsedStmtContext()) {
         Diag(Loc, diag::err_omp_immediate_directive)
             << getOpenMPDirectiveName(DKind) << 1
             << getOpenMPClauseName(OMPC_depend);
diff --git a/lib/Parse/ParseStmt.cpp b/lib/Parse/ParseStmt.cpp
index 3fc29253f0..267003b518 100644
--- a/lib/Parse/ParseStmt.cpp
+++ b/lib/Parse/ParseStmt.cpp
@@ -29,17 +29,14 @@ using namespace clang;
 /// Parse a standalone statement (for instance, as the body of an 'if',
 /// 'while', or 'for').
 StmtResult Parser::ParseStatement(SourceLocation *TrailingElseLoc,
-                                  bool AllowOpenMPStandalone) {
+                                  ParsedStmtContext StmtCtx) {
   StmtResult Res;
 
   // We may get back a null statement if we found a #pragma. Keep going until
   // we get an actual statement.
   do {
     StmtVector Stmts;
-    Res = ParseStatementOrDeclaration(
-        Stmts, AllowOpenMPStandalone ? ACK_StatementsOpenMPAnyExecutable
-                                     : ACK_StatementsOpenMPNonStandalone,
-        TrailingElseLoc);
+    Res = ParseStatementOrDeclaration(Stmts, StmtCtx, TrailingElseLoc);
   } while (!Res.isInvalid() && !Res.get());
 
   return Res;
@@ -96,7 +93,7 @@ StmtResult Parser::ParseStatement(SourceLocation *TrailingElseLoc,
 ///
 StmtResult
 Parser::ParseStatementOrDeclaration(StmtVector &Stmts,
-                                    AllowedConstructsKind Allowed,
+                                    ParsedStmtContext StmtCtx,
                                     SourceLocation *TrailingElseLoc) {
 
   ParenBraceBracketBalancer BalancerRAIIObj(*this);
@@ -107,7 +104,7 @@ Parser::ParseStatementOrDeclaration(StmtVector &Stmts,
     return StmtError();
 
   StmtResult Res = ParseStatementOrDeclarationAfterAttributes(
-      Stmts, Allowed, TrailingElseLoc, Attrs);
+      Stmts, StmtCtx, TrailingElseLoc, Attrs);
 
   assert((Attrs.empty() || Res.isInvalid() || Res.isUsable()) &&
          "attributes on empty statement");
@@ -147,10 +144,9 @@ private:
 };
 }
 
-StmtResult
-Parser::ParseStatementOrDeclarationAfterAttributes(StmtVector &Stmts,
-          AllowedConstructsKind Allowed, SourceLocation *TrailingElseLoc,
-          ParsedAttributesWithRange &Attrs) {
+StmtResult Parser::ParseStatementOrDeclarationAfterAttributes(
+    StmtVector &Stmts, ParsedStmtContext StmtCtx,
+    SourceLocation *TrailingElseLoc, ParsedAttributesWithRange &Attrs) {
   const char *SemiError = nullptr;
   StmtResult Res;
 
@@ -165,7 +161,7 @@ Retry:
     {
       ProhibitAttributes(Attrs); // TODO: is it correct?
       AtLoc = ConsumeToken();  // consume @
-      return ParseObjCAtStatement(AtLoc);
+      return ParseObjCAtStatement(AtLoc, StmtCtx);
     }
 
   case tok::code_completion:
@@ -177,7 +173,7 @@ Retry:
     Token Next = NextToken();
     if (Next.is(tok::colon)) { // C99 6.8.1: labeled-statement
       // identifier ':' statement
-      return ParseLabeledStatement(Attrs);
+      return ParseLabeledStatement(Attrs, StmtCtx);
     }
 
     // Look up the identifier, and typo-correct it to a keyword if it's not
@@ -207,7 +203,8 @@ Retry:
 
   default: {
     if ((getLangOpts().CPlusPlus || getLangOpts().MicrosoftExt ||
-         Allowed == ACK_Any) &&
+         (StmtCtx & ParsedStmtContext::AllowDeclarationsInC) !=
+             ParsedStmtContext()) &&
         isDeclarationStatement()) {
       SourceLocation DeclStart = Tok.getLocation(), DeclEnd;
       DeclGroupPtrTy Decl = ParseDeclaration(DeclaratorContext::BlockContext,
@@ -220,13 +217,13 @@ Retry:
       return StmtError();
     }
 
-    return ParseExprStatement();
+    return ParseExprStatement(StmtCtx);
   }
 
   case tok::kw_case:                // C99 6.8.1: labeled-statement
-    return ParseCaseStatement();
+    return ParseCaseStatement(StmtCtx);
   case tok::kw_default:             // C99 6.8.1: labeled-statement
-    return ParseDefaultStatement();
+    return ParseDefaultStatement(StmtCtx);
 
   case tok::l_brace:                // C99 6.8.2: compound-statement
     return ParseCompoundStatement();
@@ -363,7 +360,7 @@ Retry:
 
   case tok::annot_pragma_openmp:
     ProhibitAttributes(Attrs);
-    return ParseOpenMPDeclarativeOrExecutableDirective(Allowed);
+    return ParseOpenMPDeclarativeOrExecutableDirective(StmtCtx);
 
   case tok::annot_pragma_ms_pointers_to_members:
     ProhibitAttributes(Attrs);
@@ -382,7 +379,7 @@ Retry:
 
   case tok::annot_pragma_loop_hint:
     ProhibitAttributes(Attrs);
-    return ParsePragmaLoopHint(Stmts, Allowed, TrailingElseLoc, Attrs);
+    return ParsePragmaLoopHint(Stmts, StmtCtx, TrailingElseLoc, Attrs);
 
   case tok::annot_pragma_dump:
     HandlePragmaDump();
@@ -407,7 +404,7 @@ Retry:
 }
 
 /// Parse an expression statement.
-StmtResult Parser::ParseExprStatement() {
+StmtResult Parser::ParseExprStatement(ParsedStmtContext StmtCtx) {
   // If a case keyword is missing, this is where it should be inserted.
   Token OldToken = Tok;
 
@@ -433,12 +430,12 @@ StmtResult Parser::ParseExprStatement() {
       << FixItHint::CreateInsertion(OldToken.getLocation(), "case ");
 
     // Recover parsing as a case statement.
-    return ParseCaseStatement(/*MissingCase=*/true, Expr);
+    return ParseCaseStatement(StmtCtx, /*MissingCase=*/true, Expr);
   }
 
   // Otherwise, eat the semicolon.
   ExpectAndConsumeSemi(diag::err_expected_semi_after_expr);
-  return Actions.ActOnExprStmt(Expr, isExprValueDiscarded());
+  return handleExprStmt(Expr, StmtCtx);
 }
 
 /// ParseSEHTryBlockCommon
@@ -577,10 +574,15 @@ StmtResult Parser::ParseSEHLeaveStatement() {
 ///         identifier ':' statement
 /// [GNU]   identifier ':' attributes[opt] statement
 ///
-StmtResult Parser::ParseLabeledStatement(ParsedAttributesWithRange &attrs) {
+StmtResult Parser::ParseLabeledStatement(ParsedAttributesWithRange &attrs,
+                                         ParsedStmtContext StmtCtx) {
   assert(Tok.is(tok::identifier) && Tok.getIdentifierInfo() &&
          "Not an identifier!");
 
+  // The substatement is always a 'statement', not a 'declaration', but is
+  // otherwise in the same context as the labeled-statement.
+  StmtCtx &= ~ParsedStmtContext::AllowDeclarationsInC;
+
   Token IdentTok = Tok;  // Save the whole token.
   ConsumeToken();  // eat the identifier.
 
@@ -610,9 +612,8 @@ StmtResult Parser::ParseLabeledStatement(ParsedAttributesWithRange &attrs) {
       // statement, but that doesn't work correctly (because ProhibitAttributes
       // can't handle GNU attributes), so only call it in the one case where
       // GNU attributes are allowed.
-      SubStmt = ParseStatementOrDeclarationAfterAttributes(
-          Stmts, /*Allowed=*/ACK_StatementsOpenMPNonStandalone, nullptr,
-          TempAttrs);
+      SubStmt = ParseStatementOrDeclarationAfterAttributes(Stmts, StmtCtx,
+                                                           nullptr, TempAttrs);
       if (!TempAttrs.empty() && !SubStmt.isInvalid())
         SubStmt = Actions.ProcessStmtAttributes(SubStmt.get(), TempAttrs,
                                                 TempAttrs.Range);
@@ -623,7 +624,7 @@ StmtResult Parser::ParseLabeledStatement(ParsedAttributesWithRange &attrs) {
 
   // If we've not parsed a statement yet, parse one now.
   if (!SubStmt.isInvalid() && !SubStmt.isUsable())
-    SubStmt = ParseStatement();
+    SubStmt = ParseStatement(nullptr, StmtCtx);
 
   // Broken substmt shouldn't prevent the label from being added to the AST.
   if (SubStmt.isInvalid())
@@ -643,9 +644,14 @@ StmtResult Parser::ParseLabeledStatement(ParsedAttributesWithRange &attrs) {
 ///         'case' constant-expression ':' statement
 /// [GNU]   'case' constant-expression '...' constant-expression ':' statement
 ///
-StmtResult Parser::ParseCaseStatement(bool MissingCase, ExprResult Expr) {
+StmtResult Parser::ParseCaseStatement(ParsedStmtContext StmtCtx,
+                                      bool MissingCase, ExprResult Expr) {
   assert((MissingCase || Tok.is(tok::kw_case)) && "Not a case stmt!");
 
+  // The substatement is always a 'statement', not a 'declaration', but is
+  // otherwise in the same context as the labeled-statement.
+  StmtCtx &= ~ParsedStmtContext::AllowDeclarationsInC;
+
   // It is very very common for code to contain many case statements recursively
   // nested, as in (but usually without indentation):
   //  case 1:
@@ -737,8 +743,7 @@ StmtResult Parser::ParseCaseStatement(bool MissingCase, ExprResult Expr) {
     // continue parsing the sub-stmt.
     if (Case.isInvalid()) {
       if (TopLevelCase.isInvalid())  // No parsed case stmts.
-        return ParseStatement(/*TrailingElseLoc=*/nullptr,
-                              /*AllowOpenMPStandalone=*/true);
+        return ParseStatement(/*TrailingElseLoc=*/nullptr, StmtCtx);
       // Otherwise, just don't add it as a nested case.
     } else {
       // If this is the first case statement we parsed, it becomes TopLevelCase.
@@ -758,8 +763,7 @@ StmtResult Parser::ParseCaseStatement(bool MissingCase, ExprResult Expr) {
   StmtResult SubStmt;
 
   if (Tok.isNot(tok::r_brace)) {
-    SubStmt = ParseStatement(/*TrailingElseLoc=*/nullptr,
-                             /*AllowOpenMPStandalone=*/true);
+    SubStmt = ParseStatement(/*TrailingElseLoc=*/nullptr, StmtCtx);
   } else {
     // Nicely diagnose the common error "switch (X) { case 4: }", which is
     // not valid.  If ColonLoc doesn't point to a valid text location, there was
@@ -789,8 +793,13 @@ StmtResult Parser::ParseCaseStatement(bool MissingCase, ExprResult Expr) {
 ///         'default' ':' statement
 /// Note that this does not parse the 'statement' at the end.
 ///
-StmtResult Parser::ParseDefaultStatement() {
+StmtResult Parser::ParseDefaultStatement(ParsedStmtContext StmtCtx) {
   assert(Tok.is(tok::kw_default) && "Not a default stmt!");
+
+  // The substatement is always a 'statement', not a 'declaration', but is
+  // otherwise in the same context as the labeled-statement.
+  StmtCtx &= ~ParsedStmtContext::AllowDeclarationsInC;
+
   SourceLocation DefaultLoc = ConsumeToken();  // eat the 'default'.
 
   SourceLocation ColonLoc;
@@ -811,8 +820,7 @@ StmtResult Parser::ParseDefaultStatement() {
   StmtResult SubStmt;
 
   if (Tok.isNot(tok::r_brace)) {
-    SubStmt = ParseStatement(/*TrailingElseLoc=*/nullptr,
-                             /*AllowOpenMPStandalone=*/true);
+    SubStmt = ParseStatement(/*TrailingElseLoc=*/nullptr, StmtCtx);
   } else {
     // Diagnose the common error "switch (X) {... default: }", which is
     // not valid.
@@ -943,7 +951,8 @@ bool Parser::ConsumeNullStmt(StmtVector &Stmts) {
     EndLoc = Tok.getLocation();
 
     // Don't just ConsumeToken() this tok::semi, do store it in AST.
-    StmtResult R = ParseStatementOrDeclaration(Stmts, ACK_Any);
+    StmtResult R =
+        ParseStatementOrDeclaration(Stmts, ParsedStmtContext::SubStmt);
     if (R.isUsable())
       Stmts.push_back(R.get());
   }
@@ -957,14 +966,18 @@ bool Parser::ConsumeNullStmt(StmtVector &Stmts) {
   return true;
 }
 
-bool Parser::isExprValueDiscarded() {
-  if (Actions.isCurCompoundStmtAStmtExpr()) {
-    // Look to see if the next two tokens close the statement expression;
+StmtResult Parser::handleExprStmt(ExprResult E, ParsedStmtContext StmtCtx) {
+  bool IsStmtExprResult = false;
+  if ((StmtCtx & ParsedStmtContext::InStmtExpr) != ParsedStmtContext()) {
+    // Look ahead to see if the next two tokens close the statement expression;
     // if so, this expression statement is the last statement in a
     // statment expression.
-    return Tok.isNot(tok::r_brace) || NextToken().isNot(tok::r_paren);
+    IsStmtExprResult = Tok.is(tok::r_brace) && NextToken().is(tok::r_paren);
   }
-  return true;
+
+  if (IsStmtExprResult)
+    E = Actions.ActOnStmtExprResult(E);
+  return Actions.ActOnExprStmt(E, /*DiscardedValue=*/!IsStmtExprResult);
 }
 
 /// ParseCompoundStatementBody - Parse a sequence of statements and invoke the
@@ -1022,6 +1035,10 @@ StmtResult Parser::ParseCompoundStatementBody(bool isStmtExpr) {
       Stmts.push_back(R.get());
   }
 
+  ParsedStmtContext SubStmtCtx =
+      ParsedStmtContext::Compound |
+      (isStmtExpr ? ParsedStmtContext::InStmtExpr : ParsedStmtContext());
+
   while (!tryParseMisplacedModuleImport() && Tok.isNot(tok::r_brace) &&
          Tok.isNot(tok::eof)) {
     if (Tok.is(tok::annot_pragma_unused)) {
@@ -1034,7 +1051,7 @@ StmtResult Parser::ParseCompoundStatementBody(bool isStmtExpr) {
 
     StmtResult R;
     if (Tok.isNot(tok::kw___extension__)) {
-      R = ParseStatementOrDeclaration(Stmts, ACK_Any);
+      R = ParseStatementOrDeclaration(Stmts, SubStmtCtx);
     } else {
       // __extension__ can start declarations and it can also be a unary
       // operator for expressions.  Consume multiple __extension__ markers here
@@ -1067,11 +1084,12 @@ StmtResult Parser::ParseCompoundStatementBody(bool isStmtExpr) {
           continue;
         }
 
-        // FIXME: Use attributes?
         // Eat the semicolon at the end of stmt and convert the expr into a
         // statement.
         ExpectAndConsumeSemi(diag::err_expected_semi_after_expr);
-        R = Actions.ActOnExprStmt(Res, isExprValueDiscarded());
+        R = handleExprStmt(Res, SubStmtCtx);
+        if (R.isUsable())
+          R = Actions.ProcessStmtAttributes(R.get(), attrs, attrs.Range);
       }
     }
 
@@ -2001,7 +2019,7 @@ StmtResult Parser::ParseReturnStatement() {
 }
 
 StmtResult Parser::ParsePragmaLoopHint(StmtVector &Stmts,
-                                       AllowedConstructsKind Allowed,
+                                       ParsedStmtContext StmtCtx,
                                        SourceLocation *TrailingElseLoc,
                                        ParsedAttributesWithRange &Attrs) {
   // Create temporary attribute list.
@@ -2024,7 +2042,7 @@ StmtResult Parser::ParsePragmaLoopHint(StmtVector &Stmts,
   MaybeParseCXX11Attributes(Attrs);
 
   StmtResult S = ParseStatementOrDeclarationAfterAttributes(
-      Stmts, Allowed, TrailingElseLoc, Attrs);
+      Stmts, StmtCtx, TrailingElseLoc, Attrs);
 
   Attrs.takeAllFrom(TempAttrs);
   return S;
@@ -2329,7 +2347,8 @@ void Parser::ParseMicrosoftIfExistsStatement(StmtVector &Stmts) {
 
   // Condition is true, parse the statements.
   while (Tok.isNot(tok::r_brace)) {
-    StmtResult R = ParseStatementOrDeclaration(Stmts, ACK_Any);
+    StmtResult R =
+        ParseStatementOrDeclaration(Stmts, ParsedStmtContext::Compound);
     if (R.isUsable())
       Stmts.push_back(R.get());
   }
diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index b992f3387e..00b4cddc6b 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -13301,29 +13301,6 @@ ExprResult Sema::ActOnAddrLabel(SourceLocation OpLoc, SourceLocation LabLoc,
                                      Context.getPointerType(Context.VoidTy));
 }
 
-/// Given the last statement in a statement-expression, check whether
-/// the result is a producing expression (like a call to an
-/// ns_returns_retained function) and, if so, rebuild it to hoist the
-/// release out of the full-expression.  Otherwise, return null.
-/// Cannot fail.
-static Expr *maybeRebuildARCConsumingStmt(Stmt *Statement) {
-  // Should always be wrapped with one of these.
-  ExprWithCleanups *cleanups = dyn_cast<ExprWithCleanups>(Statement);
-  if (!cleanups) return nullptr;
-
-  ImplicitCastExpr *cast = dyn_cast<ImplicitCastExpr>(cleanups->getSubExpr());
-  if (!cast || cast->getCastKind() != CK_ARCConsumeObject)
-    return nullptr;
-
-  // Splice out the cast.  This shouldn't modify any interesting
-  // features of the statement.
-  Expr *producer = cast->getSubExpr();
-  assert(producer->getType() == cast->getType());
-  assert(producer->getValueKind() == cast->getValueKind());
-  cleanups->setSubExpr(producer);
-  return cleanups;
-}
-
 void Sema::ActOnStartStmtExpr() {
   PushExpressionEvaluationContext(ExprEvalContexts.back().Context);
 }
@@ -13357,47 +13334,10 @@ Sema::ActOnStmtExpr(SourceLocation LPLoc, Stmt *SubStmt,
   QualType Ty = Context.VoidTy;
   bool StmtExprMayBindToTemp = false;
   if (!Compound->body_empty()) {
-    Stmt *LastStmt = Compound->body_back();
-    LabelStmt *LastLabelStmt = nullptr;
-    // If LastStmt is a label, skip down through into the body.
-    while (LabelStmt *Label = dyn_cast<LabelStmt>(LastStmt)) {
-      LastLabelStmt = Label;
-      LastStmt = Label->getSubStmt();
-    }
-
-    if (Expr *LastE = dyn_cast<Expr>(LastStmt)) {
-      // Do function/array conversion on the last expression, but not
-      // lvalue-to-rvalue.  However, initialize an unqualified type.
-      ExprResult LastExpr = DefaultFunctionArrayConversion(LastE);
-      if (LastExpr.isInvalid())
-        return ExprError();
-      Ty = LastExpr.get()->getType().getUnqualifiedType();
-
-      if (!Ty->isDependentType() && !LastExpr.get()->isTypeDependent()) {
-        // In ARC, if the final expression ends in a consume, splice
-        // the consume out and bind it later.  In the alternate case
-        // (when dealing with a retainable type), the result
-        // initialization will create a produce.  In both cases the
-        // result will be +1, and we'll need to balance that out with
-        // a bind.
-        if (Expr *rebuiltLastStmt
-              = maybeRebuildARCConsumingStmt(LastExpr.get())) {
-          LastExpr = rebuiltLastStmt;
-        } else {
-          LastExpr = PerformCopyInitialization(
-              InitializedEntity::InitializeStmtExprResult(LPLoc, Ty),
-              SourceLocation(), LastExpr);
-        }
-
-        if (LastExpr.isInvalid())
-          return ExprError();
-        if (LastExpr.get() != nullptr) {
-          if (!LastLabelStmt)
-            Compound->setLastStmt(LastExpr.get());
-          else
-            LastLabelStmt->setSubStmt(LastExpr.get());
-          StmtExprMayBindToTemp = true;
-        }
+    if (const auto *LastStmt = dyn_cast<ValueStmt>(Compound->body_back())) {
+      if (const Expr *Value = LastStmt->getExprStmt()) {
+        StmtExprMayBindToTemp = true;
+        Ty = Value->getType();
       }
     }
   }
@@ -13410,6 +13350,37 @@ Sema::ActOnStmtExpr(SourceLocation LPLoc, Stmt *SubStmt,
   return ResStmtExpr;
 }
 
+ExprResult Sema::ActOnStmtExprResult(ExprResult ER) {
+  if (ER.isInvalid())
+    return ExprError();
+
+  // Do function/array conversion on the last expression, but not
+  // lvalue-to-rvalue.  However, initialize an unqualified type.
+  ER = DefaultFunctionArrayConversion(ER.get());
+  if (ER.isInvalid())
+    return ExprError();
+  Expr *E = ER.get();
+
+  if (E->isTypeDependent())
+    return E;
+
+  // In ARC, if the final expression ends in a consume, splice
+  // the consume out and bind it later.  In the alternate case
+  // (when dealing with a retainable type), the result
+  // initialization will create a produce.  In both cases the
+  // result will be +1, and we'll need to balance that out with
+  // a bind.
+  auto *Cast = dyn_cast<ImplicitCastExpr>(E);
+  if (Cast && Cast->getCastKind() == CK_ARCConsumeObject)
+    return Cast->getSubExpr();
+
+  // FIXME: Provide a better location for the initialization.
+  return PerformCopyInitialization(
+      InitializedEntity::InitializeStmtExprResult(
+          E->getBeginLoc(), E->getType().getUnqualifiedType()),
+      SourceLocation(), E);
+}
+
 ExprResult Sema::BuildBuiltinOffsetOf(SourceLocation BuiltinLoc,
                                       TypeSourceInfo *TInfo,
                                       ArrayRef<OffsetOfComponent> Components,
@@ -14490,14 +14461,6 @@ namespace {
     // Make sure we redo semantic analysis
     bool AlwaysRebuild() { return true; }
 
-    // Make sure we handle LabelStmts correctly.
-    // FIXME: This does the right thing, but maybe we need a more general
-    // fix to TreeTransform?
-    StmtResult TransformLabelStmt(LabelStmt *S) {
-      S->getDecl()->setStmt(nullptr);
-      return BaseTransform::TransformLabelStmt(S);
-    }
-
     // We need to special-case DeclRefExprs referring to FieldDecls which
     // are not part of a member pointer formation; normal TreeTransforming
     // doesn't catch this case because of the way we represent them in the AST.
diff --git a/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp
index e4b4357653..5f04139090 100644
--- a/lib/Sema/SemaStmt.cpp
+++ b/lib/Sema/SemaStmt.cpp
@@ -346,10 +346,6 @@ sema::CompoundScopeInfo &Sema::getCurCompoundScope() const {
   return getCurFunction()->CompoundScopes.back();
 }
 
-bool Sema::isCurCompoundStmtAStmtExpr() const {
-  return getCurCompoundScope().IsStmtExpr;
-}
-
 StmtResult Sema::ActOnCompoundStmt(SourceLocation L, SourceLocation R,
                                    ArrayRef<Stmt *> Elts, bool isStmtExpr) {
   const unsigned NumElts = Elts.size();
diff --git a/lib/Sema/TreeTransform.h b/lib/Sema/TreeTransform.h
index e411a2ad47..ff33028ebb 100644
--- a/lib/Sema/TreeTransform.h
+++ b/lib/Sema/TreeTransform.h
@@ -318,6 +318,13 @@ public:
   TypeSourceInfo *TransformTypeWithDeducedTST(TypeSourceInfo *DI);
   /// @}
 
+  /// The reason why the value of a statement is not discarded, if any.
+  enum StmtDiscardKind {
+    SDK_Discarded,
+    SDK_NotDiscarded,
+    SDK_StmtExprResult,
+  };
+
   /// Transform the given statement.
   ///
   /// By default, this routine transforms a statement by delegating to the
@@ -327,7 +334,7 @@ public:
   /// other mechanism.
   ///
   /// \returns the transformed statement.
-  StmtResult TransformStmt(Stmt *S, bool DiscardedValue = false);
+  StmtResult TransformStmt(Stmt *S, StmtDiscardKind SDK = SDK_Discarded);
 
   /// Transform the given statement.
   ///
@@ -672,6 +679,9 @@ public:
 #define STMT(Node, Parent)                        \
   LLVM_ATTRIBUTE_NOINLINE \
   StmtResult Transform##Node(Node *S);
+#define VALUESTMT(Node, Parent)                   \
+  LLVM_ATTRIBUTE_NOINLINE \
+  StmtResult Transform##Node(Node *S, StmtDiscardKind SDK);
 #define EXPR(Node, Parent)                        \
   LLVM_ATTRIBUTE_NOINLINE \
   ExprResult Transform##Node(Node *E);
@@ -3270,7 +3280,7 @@ private:
 };
 
 template <typename Derived>
-StmtResult TreeTransform<Derived>::TransformStmt(Stmt *S, bool DiscardedValue) {
+StmtResult TreeTransform<Derived>::TransformStmt(Stmt *S, StmtDiscardKind SDK) {
   if (!S)
     return S;
 
@@ -3278,8 +3288,12 @@ StmtResult TreeTransform<Derived>::TransformStmt(Stmt *S, bool DiscardedValue) {
   case Stmt::NoStmtClass: break;
 
   // Transform individual statement nodes
+  // Pass SDK into statements that can produce a value
 #define STMT(Node, Parent)                                              \
   case Stmt::Node##Class: return getDerived().Transform##Node(cast<Node>(S));
+#define VALUESTMT(Node, Parent)                                         \
+  case Stmt::Node##Class:                                               \
+    return getDerived().Transform##Node(cast<Node>(S), SDK);
 #define ABSTRACT_STMT(Node)
 #define EXPR(Node, Parent)
 #include "clang/AST/StmtNodes.inc"
@@ -3291,10 +3305,10 @@ StmtResult TreeTransform<Derived>::TransformStmt(Stmt *S, bool DiscardedValue) {
 #include "clang/AST/StmtNodes.inc"
     {
       ExprResult E = getDerived().TransformExpr(cast<Expr>(S));
-      if (E.isInvalid())
-        return StmtError();
 
-      return getSema().ActOnExprStmt(E, DiscardedValue);
+      if (SDK == SDK_StmtExprResult)
+        E = getSema().ActOnStmtExprResult(E);
+      return getSema().ActOnExprStmt(E, SDK == SDK_Discarded);
     }
   }
 
@@ -6522,8 +6536,9 @@ TreeTransform<Derived>::TransformCompoundStmt(CompoundStmt *S,
   bool SubStmtChanged = false;
   SmallVector<Stmt*, 8> Statements;
   for (auto *B : S->body()) {
-    StmtResult Result =
-        getDerived().TransformStmt(B, !IsStmtExpr || B != S->body_back());
+    StmtResult Result = getDerived().TransformStmt(
+        B,
+        IsStmtExpr && B == S->body_back() ? SDK_StmtExprResult : SDK_Discarded);
 
     if (Result.isInvalid()) {
       // Immediately fail if this was a DeclStmt, since it's very
@@ -6586,7 +6601,8 @@ TreeTransform<Derived>::TransformCaseStmt(CaseStmt *S) {
     return StmtError();
 
   // Transform the statement following the case
-  StmtResult SubStmt = getDerived().TransformStmt(S->getSubStmt());
+  StmtResult SubStmt =
+      getDerived().TransformStmt(S->getSubStmt());
   if (SubStmt.isInvalid())
     return StmtError();
 
@@ -6594,11 +6610,11 @@ TreeTransform<Derived>::TransformCaseStmt(CaseStmt *S) {
   return getDerived().RebuildCaseStmtBody(Case.get(), SubStmt.get());
 }
 
-template<typename Derived>
-StmtResult
-TreeTransform<Derived>::TransformDefaultStmt(DefaultStmt *S) {
+template <typename Derived>
+StmtResult TreeTransform<Derived>::TransformDefaultStmt(DefaultStmt *S) {
   // Transform the statement following the default case
-  StmtResult SubStmt = getDerived().TransformStmt(S->getSubStmt());
+  StmtResult SubStmt =
+      getDerived().TransformStmt(S->getSubStmt());
   if (SubStmt.isInvalid())
     return StmtError();
 
@@ -6609,8 +6625,8 @@ TreeTransform<Derived>::TransformDefaultStmt(DefaultStmt *S) {
 
 template<typename Derived>
 StmtResult
-TreeTransform<Derived>::TransformLabelStmt(LabelStmt *S) {
-  StmtResult SubStmt = getDerived().TransformStmt(S->getSubStmt());
+TreeTransform<Derived>::TransformLabelStmt(LabelStmt *S, StmtDiscardKind SDK) {
+  StmtResult SubStmt = getDerived().TransformStmt(S->getSubStmt(), SDK);
   if (SubStmt.isInvalid())
     return StmtError();
 
@@ -6619,6 +6635,11 @@ TreeTransform<Derived>::TransformLabelStmt(LabelStmt *S) {
   if (!LD)
     return StmtError();
 
+  // If we're transforming "in-place" (we're not creating new local
+  // declarations), assume we're replacing the old label statement
+  // and clear out the reference to it.
+  if (LD == S->getDecl())
+    S->getDecl()->setStmt(nullptr);
 
   // FIXME: Pass the real colon location in.
   return getDerived().RebuildLabelStmt(S->getIdentLoc(),
@@ -6644,7 +6665,9 @@ const Attr *TreeTransform<Derived>::TransformAttr(const Attr *R) {
 }
 
 template <typename Derived>
-StmtResult TreeTransform<Derived>::TransformAttributedStmt(AttributedStmt *S) {
+StmtResult
+TreeTransform<Derived>::TransformAttributedStmt(AttributedStmt *S,
+                                                StmtDiscardKind SDK) {
   bool AttrsChanged = false;
   SmallVector<const Attr *, 1> Attrs;
 
@@ -6655,7 +6678,7 @@ StmtResult TreeTransform<Derived>::TransformAttributedStmt(AttributedStmt *S) {
     Attrs.push_back(R);
   }
 
-  StmtResult SubStmt = getDerived().TransformStmt(S->getSubStmt());
+  StmtResult SubStmt = getDerived().TransformStmt(S->getSubStmt(), SDK);
   if (SubStmt.isInvalid())
     return StmtError();
 
@@ -7360,7 +7383,8 @@ StmtResult
 TreeTransform<Derived>::TransformObjCForCollectionStmt(
                                                   ObjCForCollectionStmt *S) {
   // Transform the element statement.
-  StmtResult Element = getDerived().TransformStmt(S->getElement());
+  StmtResult Element =
+      getDerived().TransformStmt(S->getElement(), SDK_NotDiscarded);
   if (Element.isInvalid())
     return StmtError();
 
diff --git a/test/CodeGenCXX/stmtexpr.cpp b/test/CodeGenCXX/stmtexpr.cpp
index 4586a3c38f..fe5ff2c7de 100644
--- a/test/CodeGenCXX/stmtexpr.cpp
+++ b/test/CodeGenCXX/stmtexpr.cpp
@@ -190,3 +190,79 @@ extern "C" int cleanup_exit_complex(bool b) {
 // CHECK: %[[v2:[^ ]*]] = load float, float* %[[tmp2]]
 // CHECK: store float %[[v1]], float* %v.realp
 // CHECK: store float %[[v2]], float* %v.imagp
+
+extern "C" void then(int);
+
+// CHECK-LABEL: @{{.*}}volatile_load
+void volatile_load() {
+  volatile int n;
+
+  // CHECK-NOT: load volatile
+  // CHECK: load volatile
+  // CHECK-NOT: load volatile
+  ({n;});
+
+  // CHECK-LABEL: @then(i32 1)
+  then(1);
+
+  // CHECK-NOT: load volatile
+  // CHECK: load volatile
+  // CHECK-NOT: load volatile
+  ({goto lab; lab: n;});
+
+  // CHECK-LABEL: @then(i32 2)
+  then(2);
+
+  // CHECK-NOT: load volatile
+  // CHECK: load volatile
+  // CHECK-NOT: load volatile
+  ({[[gsl::suppress("foo")]] n;});
+
+  // CHECK-LABEL: @then(i32 3)
+  then(3);
+
+  // CHECK-NOT: load volatile
+  // CHECK: load volatile
+  // CHECK-NOT: load volatile
+  ({if (true) n;});
+
+  // CHECK: }
+}
+
+// CHECK-LABEL: @{{.*}}volatile_load_template
+template<typename T>
+void volatile_load_template() {
+  volatile T n;
+
+  // CHECK-NOT: load volatile
+  // CHECK: load volatile
+  // CHECK-NOT: load volatile
+  ({n;});
+
+  // CHECK-LABEL: @then(i32 1)
+  then(1);
+
+  // CHECK-NOT: load volatile
+  // CHECK: load volatile
+  // CHECK-NOT: load volatile
+  ({goto lab; lab: n;});
+
+  // CHECK-LABEL: @then(i32 2)
+  then(2);
+
+  // CHECK-NOT: load volatile
+  // CHECK: load volatile
+  // CHECK-NOT: load volatile
+  ({[[gsl::suppress("foo")]] n;});
+
+  // CHECK-LABEL: @then(i32 3)
+  then(3);
+
+  // CHECK-NOT: load volatile
+  // CHECK: load volatile
+  // CHECK-NOT: load volatile
+  ({if (true) n;});
+
+  // CHECK: }
+}
+template void volatile_load_template<int>();
diff --git a/test/CodeGenCXX/volatile.cpp b/test/CodeGenCXX/volatile.cpp
index 9c0271b21d..cf49aed4de 100644
--- a/test/CodeGenCXX/volatile.cpp
+++ b/test/CodeGenCXX/volatile.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin10 -emit-llvm -std=c++98 -o - | FileCheck %s
+// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin10 -emit-llvm -std=c++98 -o - | FileCheck -check-prefix=CHECK -check-prefix=CHECK98 %s
 // RUN: %clang_cc1 %s -triple=x86_64-apple-darwin10 -emit-llvm -std=c++11 -o - | FileCheck -check-prefix=CHECK -check-prefix=CHECK11 %s
 
 // Check that IR gen doesn't try to do an lvalue-to-rvalue conversion
@@ -33,3 +33,19 @@ namespace test1 {
     *x;
   }
 }
+
+namespace PR40642 {
+  template <class T> struct S {
+    // CHECK-LABEL: define {{.*}} @_ZN7PR406421SIiE3fooEv(
+    void foo() {
+      // CHECK98-NOT: load volatile
+      // CHECK11: load volatile
+      if (true)
+        reinterpret_cast<const volatile unsigned char *>(m_ptr)[0];
+      // CHECK: }
+    }
+    int *m_ptr;
+  };
+
+  void f(S<int> *x) { x->foo(); }
+}
diff --git a/test/OpenMP/barrier_messages.cpp b/test/OpenMP/barrier_messages.cpp
index 137453a893..42edc9f46a 100644
--- a/test/OpenMP/barrier_messages.cpp
+++ b/test/OpenMP/barrier_messages.cpp
@@ -29,7 +29,7 @@ T tmain(T argc) {
 #pragma omp barrier // expected-error {{'#pragma omp barrier' cannot be an immediate substatement}}
     switch (argc)
     case 1:
-#pragma omp barrier
+#pragma omp barrier // expected-error {{'#pragma omp barrier' cannot be an immediate substatement}}
   switch (argc)
   case 1: {
 #pragma omp barrier
@@ -49,7 +49,7 @@ T tmain(T argc) {
 #pragma omp barrier
     }
 label:
-#pragma omp barrier // expected-error {{'#pragma omp barrier' cannot be an immediate substatement}}
+#pragma omp barrier
 label1 : {
 #pragma omp barrier
 }
@@ -83,7 +83,7 @@ int main(int argc, char **argv) {
 #pragma omp barrier // expected-error {{'#pragma omp barrier' cannot be an immediate substatement}}
     switch (argc)
     case 1:
-#pragma omp barrier
+#pragma omp barrier // expected-error {{'#pragma omp barrier' cannot be an immediate substatement}}
   switch (argc)
   case 1: {
 #pragma omp barrier
@@ -103,7 +103,7 @@ int main(int argc, char **argv) {
 #pragma omp barrier
     }
 label:
-#pragma omp barrier // expected-error {{'#pragma omp barrier' cannot be an immediate substatement}}
+#pragma omp barrier
 label1 : {
 #pragma omp barrier
 }
diff --git a/test/OpenMP/cancel_messages.cpp b/test/OpenMP/cancel_messages.cpp
index b14c42f395..f78b28c1cb 100644
--- a/test/OpenMP/cancel_messages.cpp
+++ b/test/OpenMP/cancel_messages.cpp
@@ -63,7 +63,7 @@ int main(int argc, char **argv) {
 #pragma omp cancel parallel // expected-error {{'#pragma omp cancel' cannot be an immediate substatement}} expected-error {{orphaned 'omp cancel' directives are prohibited; perhaps you forget to enclose the directive into a region?}}
     switch (argc)
     case 1:
-#pragma omp cancel sections // expected-error {{orphaned 'omp cancel' directives are prohibited; perhaps you forget to enclose the directive into a region?}}
+#pragma omp cancel sections // expected-error {{'#pragma omp cancel' cannot be an immediate substatement}} expected-error {{orphaned 'omp cancel' directives are prohibited; perhaps you forget to enclose the directive into a region?}}
   switch (argc)
   case 1: {
 #pragma omp cancel for // expected-error {{orphaned 'omp cancel' directives are prohibited; perhaps you forget to enclose the directive into a region?}}
@@ -83,7 +83,7 @@ int main(int argc, char **argv) {
 #pragma omp cancel taskgroup // expected-error {{orphaned 'omp cancel' directives are prohibited; perhaps you forget to enclose the directive into a region?}}
     }
 label:
-#pragma omp cancel parallel // expected-error {{'#pragma omp cancel' cannot be an immediate substatement}} expected-error {{orphaned 'omp cancel' directives are prohibited; perhaps you forget to enclose the directive into a region?}}
+#pragma omp cancel parallel // expected-error {{orphaned 'omp cancel' directives are prohibited; perhaps you forget to enclose the directive into a region?}}
 label1 : {
 #pragma omp cancel sections // expected-error {{orphaned 'omp cancel' directives are prohibited; perhaps you forget to enclose the directive into a region?}}
 }
diff --git a/test/OpenMP/cancellation_point_messages.cpp b/test/OpenMP/cancellation_point_messages.cpp
index 2bf667c407..058818df71 100644
--- a/test/OpenMP/cancellation_point_messages.cpp
+++ b/test/OpenMP/cancellation_point_messages.cpp
@@ -63,7 +63,7 @@ int main(int argc, char **argv) {
 #pragma omp cancellation point parallel // expected-error {{'#pragma omp cancellation point' cannot be an immediate substatement}} expected-error {{orphaned 'omp cancellation point' directives are prohibited; perhaps you forget to enclose the directive into a region?}}
     switch (argc)
     case 1:
-#pragma omp cancellation point sections // expected-error {{orphaned 'omp cancellation point' directives are prohibited; perhaps you forget to enclose the directive into a region?}}
+#pragma omp cancellation point sections // expected-error {{'#pragma omp cancellation point' cannot be an immediate substatement}} expected-error {{orphaned 'omp cancellation point' directives are prohibited; perhaps you forget to enclose the directive into a region?}}
   switch (argc)
   case 1: {
 #pragma omp cancellation point for // expected-error {{orphaned 'omp cancellation point' directives are prohibited; perhaps you forget to enclose the directive into a region?}}
@@ -83,7 +83,7 @@ int main(int argc, char **argv) {
 #pragma omp cancellation point taskgroup // expected-error {{orphaned 'omp cancellation point' directives are prohibited; perhaps you forget to enclose the directive into a region?}}
     }
 label:
-#pragma omp cancellation point parallel // expected-error {{'#pragma omp cancellation point' cannot be an immediate substatement}} expected-error {{orphaned 'omp cancellation point' directives are prohibited; perhaps you forget to enclose the directive into a region?}}
+#pragma omp cancellation point parallel // expected-error {{orphaned 'omp cancellation point' directives are prohibited; perhaps you forget to enclose the directive into a region?}}
 label1 : {
 #pragma omp cancellation point sections // expected-error {{orphaned 'omp cancellation point' directives are prohibited; perhaps you forget to enclose the directive into a region?}}
 }
diff --git a/test/OpenMP/flush_messages.cpp b/test/OpenMP/flush_messages.cpp
index da1d0c2412..f4456489a9 100644
--- a/test/OpenMP/flush_messages.cpp
+++ b/test/OpenMP/flush_messages.cpp
@@ -33,7 +33,7 @@ T tmain(T argc) {
 #pragma omp flush // expected-error {{'#pragma omp flush' cannot be an immediate substatement}}
     switch (argc)
     case 1:
-#pragma omp flush
+#pragma omp flush // expected-error {{'#pragma omp flush' cannot be an immediate substatement}}
   switch (argc)
   case 1: {
 #pragma omp flush
@@ -53,7 +53,7 @@ T tmain(T argc) {
 #pragma omp flush
     }
 label:
-#pragma omp flush // expected-error {{'#pragma omp flush' cannot be an immediate substatement}}
+#pragma omp flush
 label1 : {
 #pragma omp flush
 }
@@ -97,7 +97,7 @@ int main(int argc, char **argv) {
 #pragma omp flush // expected-error {{'#pragma omp flush' cannot be an immediate substatement}}
     switch (argc)
     case 1:
-#pragma omp flush
+#pragma omp flush // expected-error {{'#pragma omp flush' cannot be an immediate substatement}}
   switch (argc)
   case 1: {
 #pragma omp flush
@@ -117,7 +117,7 @@ int main(int argc, char **argv) {
 #pragma omp flush
     }
 label:
-#pragma omp flush // expected-error {{'#pragma omp flush' cannot be an immediate substatement}}
+#pragma omp flush
 label1 : {
 #pragma omp flush
 }
diff --git a/test/OpenMP/taskwait_messages.cpp b/test/OpenMP/taskwait_messages.cpp
index 46efff5972..55bf2ddcad 100644
--- a/test/OpenMP/taskwait_messages.cpp
+++ b/test/OpenMP/taskwait_messages.cpp
@@ -29,7 +29,7 @@ T tmain(T argc) {
 #pragma omp taskwait // expected-error {{'#pragma omp taskwait' cannot be an immediate substatement}}
     switch (argc)
     case 1:
-#pragma omp taskwait
+#pragma omp taskwait // expected-error {{'#pragma omp taskwait' cannot be an immediate substatement}}
   switch (argc)
   case 1: {
 #pragma omp taskwait
@@ -49,7 +49,7 @@ T tmain(T argc) {
 #pragma omp taskwait
     }
 label:
-#pragma omp taskwait // expected-error {{'#pragma omp taskwait' cannot be an immediate substatement}}
+#pragma omp taskwait
 label1 : {
 #pragma omp taskwait
 }
@@ -83,7 +83,7 @@ int main(int argc, char **argv) {
 #pragma omp taskwait // expected-error {{'#pragma omp taskwait' cannot be an immediate substatement}}
     switch (argc)
     case 1:
-#pragma omp taskwait
+#pragma omp taskwait // expected-error {{'#pragma omp taskwait' cannot be an immediate substatement}}
   switch (argc)
   case 1: {
 #pragma omp taskwait
@@ -103,7 +103,7 @@ int main(int argc, char **argv) {
 #pragma omp taskwait
     }
 label:
-#pragma omp taskwait // expected-error {{'#pragma omp taskwait' cannot be an immediate substatement}}
+#pragma omp taskwait
 label1 : {
 #pragma omp taskwait
 }
diff --git a/test/OpenMP/taskyield_messages.cpp b/test/OpenMP/taskyield_messages.cpp
index fd98a8786f..48f741df91 100644
--- a/test/OpenMP/taskyield_messages.cpp
+++ b/test/OpenMP/taskyield_messages.cpp
@@ -29,7 +29,7 @@ T tmain(T argc) {
 #pragma omp taskyield // expected-error {{'#pragma omp taskyield' cannot be an immediate substatement}}
     switch (argc)
     case 1:
-#pragma omp taskyield
+#pragma omp taskyield // expected-error {{'#pragma omp taskyield' cannot be an immediate substatement}}
   switch (argc)
   case 1: {
 #pragma omp taskyield
@@ -49,10 +49,13 @@ T tmain(T argc) {
 #pragma omp taskyield
     }
 label:
-#pragma omp taskyield // expected-error {{'#pragma omp taskyield' cannot be an immediate substatement}}
+#pragma omp taskyield
 label1 : {
 #pragma omp taskyield
 }
+if (1)
+  label2:
+#pragma omp taskyield // expected-error {{'#pragma omp taskyield' cannot be an immediate substatement}}
 
   return T();
 }
@@ -83,7 +86,7 @@ int main(int argc, char **argv) {
 #pragma omp taskyield // expected-error {{'#pragma omp taskyield' cannot be an immediate substatement}}
     switch (argc)
     case 1:
-#pragma omp taskyield
+#pragma omp taskyield // expected-error {{'#pragma omp taskyield' cannot be an immediate substatement}}
   switch (argc)
   case 1: {
 #pragma omp taskyield
@@ -103,10 +106,13 @@ int main(int argc, char **argv) {
 #pragma omp taskyield
     }
 label:
-#pragma omp taskyield // expected-error {{'#pragma omp taskyield' cannot be an immediate substatement}}
+#pragma omp taskyield
 label1 : {
 #pragma omp taskyield
 }
+if (1)
+  label2:
+#pragma omp taskyield // expected-error {{'#pragma omp taskyield' cannot be an immediate substatement}}
 
   return tmain(argc);
 }