Revert r273548, "Rearrange condition handling so that semantic checks on a condition variable"

as it caused a regression in -Wfor-loop-analysis.

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

16 files changed, 374 insertions, 294 deletions

diff --git a/include/clang/Parse/Parser.h b/include/clang/Parse/Parser.h
index 3ddbcf5245..9cbad6b634 100644
--- a/include/clang/Parse/Parser.h
+++ b/include/clang/Parse/Parser.h
@@ -1588,8 +1588,8 @@ private:
 
   //===--------------------------------------------------------------------===//
   // C++ if/switch/while condition expression.
-  Sema::ConditionResult ParseCXXCondition(SourceLocation Loc,
-                                          Sema::ConditionKind CK);
+  bool ParseCXXCondition(ExprResult &ExprResult, Decl *&DeclResult,
+                         SourceLocation Loc, bool ConvertToBoolean);
 
   //===--------------------------------------------------------------------===//
   // C++ Coroutines
@@ -1680,9 +1680,10 @@ private:
                                     unsigned ScopeFlags);
   void ParseCompoundStatementLeadingPragmas();
   StmtResult ParseCompoundStatementBody(bool isStmtExpr = false);
-  bool ParseParenExprOrCondition(Sema::ConditionResult &CondResult,
+  bool ParseParenExprOrCondition(ExprResult &ExprResult,
+                                 Decl *&DeclResult,
                                  SourceLocation Loc,
-                                 Sema::ConditionKind CK);
+                                 bool ConvertToBoolean);
   StmtResult ParseIfStatement(SourceLocation *TrailingElseLoc);
   StmtResult ParseSwitchStatement(SourceLocation *TrailingElseLoc);
   StmtResult ParseWhileStatement(SourceLocation *TrailingElseLoc);
diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 7f2a942962..977ffb4dd2 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -3298,7 +3298,6 @@ public:
 public:
   class FullExprArg {
   public:
-    FullExprArg() : E(nullptr) { }
     FullExprArg(Sema &actions) : E(nullptr) { }
 
     ExprResult release() {
@@ -3392,23 +3391,27 @@ public:
                                  ArrayRef<const Attr*> Attrs,
                                  Stmt *SubStmt);
 
-  class ConditionResult;
-  StmtResult ActOnIfStmt(SourceLocation IfLoc, ConditionResult Cond,
-                         Stmt *ThenVal, SourceLocation ElseLoc, Stmt *ElseVal);
+  StmtResult ActOnIfStmt(SourceLocation IfLoc,
+                         FullExprArg CondVal, Decl *CondVar,
+                         Stmt *ThenVal,
+                         SourceLocation ElseLoc, Stmt *ElseVal);
   StmtResult ActOnStartOfSwitchStmt(SourceLocation SwitchLoc,
-                                    ConditionResult Cond);
+                                            Expr *Cond,
+                                            Decl *CondVar);
   StmtResult ActOnFinishSwitchStmt(SourceLocation SwitchLoc,
                                            Stmt *Switch, Stmt *Body);
-  StmtResult ActOnWhileStmt(SourceLocation WhileLoc, ConditionResult Cond,
-                            Stmt *Body);
+  StmtResult ActOnWhileStmt(SourceLocation WhileLoc,
+                            FullExprArg Cond,
+                            Decl *CondVar, Stmt *Body);
   StmtResult ActOnDoStmt(SourceLocation DoLoc, Stmt *Body,
-                         SourceLocation WhileLoc, SourceLocation CondLParen,
-                         Expr *Cond, SourceLocation CondRParen);
+                                 SourceLocation WhileLoc,
+                                 SourceLocation CondLParen, Expr *Cond,
+                                 SourceLocation CondRParen);
 
   StmtResult ActOnForStmt(SourceLocation ForLoc,
                           SourceLocation LParenLoc,
-                          Stmt *First,
-                          ConditionResult Second,
+                          Stmt *First, FullExprArg Second,
+                          Decl *SecondVar,
                           FullExprArg Third,
                           SourceLocation RParenLoc,
                           Stmt *Body);
@@ -4798,6 +4801,11 @@ public:
                             bool WarnOnNonAbstractTypes,
                             SourceLocation DtorLoc);
 
+  DeclResult ActOnCXXConditionDeclaration(Scope *S, Declarator &D);
+  ExprResult CheckConditionVariable(VarDecl *ConditionVar,
+                                    SourceLocation StmtLoc,
+                                    bool ConvertToBoolean);
+
   ExprResult ActOnNoexceptExpr(SourceLocation KeyLoc, SourceLocation LParen,
                                Expr *Operand, SourceLocation RParen);
   ExprResult BuildCXXNoexceptExpr(SourceLocation KeyLoc, Expr *Operand,
@@ -8915,46 +8923,6 @@ public:
   /// type, and if so, emit a note describing what happened.
   void EmitRelatedResultTypeNoteForReturn(QualType destType);
 
-  class ConditionResult {
-    Decl *ConditionVar;
-    FullExprArg Condition;
-    bool Invalid;
-
-    friend class Sema;
-    ConditionResult(Decl *ConditionVar, FullExprArg Condition)
-        : ConditionVar(ConditionVar), Condition(Condition), Invalid(false) {}
-    explicit ConditionResult(bool Invalid)
-        : ConditionVar(nullptr), Condition(nullptr), Invalid(Invalid) {}
-
-  public:
-    ConditionResult() : ConditionResult(false) {}
-    bool isInvalid() const { return Invalid; }
-    std::pair<VarDecl *, Expr *> get() const {
-      return std::make_pair(cast_or_null<VarDecl>(ConditionVar),
-                            Condition.get());
-    }
-  };
-  static ConditionResult ConditionError() { return ConditionResult(true); }
-
-  enum class ConditionKind {
-    Boolean, ///< A boolean condition, from 'if', 'while', 'for', or 'do'.
-    Switch   ///< An integral condition for a 'switch' statement.
-  };
-
-  ConditionResult ActOnCondition(Scope *S, SourceLocation Loc,
-                                 Expr *SubExpr, ConditionKind CK);
-
-  ConditionResult ActOnConditionVariable(Decl *ConditionVar,
-                                         SourceLocation StmtLoc,
-                                         ConditionKind CK);
-
-  DeclResult ActOnCXXConditionDeclaration(Scope *S, Declarator &D);
-
-  ExprResult CheckConditionVariable(VarDecl *ConditionVar,
-                                    SourceLocation StmtLoc,
-                                    ConditionKind CK);
-  ExprResult CheckSwitchCondition(SourceLocation SwitchLoc, Expr *Cond);
-
   /// CheckBooleanCondition - Diagnose problems involving the use of
   /// the given expression as a boolean condition (e.g. in an if
   /// statement).  Also performs the standard function and array
@@ -8963,7 +8931,10 @@ public:
   /// \param Loc - A location associated with the condition, e.g. the
   /// 'if' keyword.
   /// \return true iff there were any errors
-  ExprResult CheckBooleanCondition(SourceLocation Loc, Expr *E);
+  ExprResult CheckBooleanCondition(Expr *E, SourceLocation Loc);
+
+  ExprResult ActOnBooleanCondition(Scope *S, SourceLocation Loc,
+                                   Expr *SubExpr);
 
   /// DiagnoseAssignmentAsCondition - Given that an expression is
   /// being used as a boolean condition, warn if it's an assignment.
diff --git a/lib/Parse/ParseDeclCXX.cpp b/lib/Parse/ParseDeclCXX.cpp
index 5718e1e50e..d55b2d9442 100644
--- a/lib/Parse/ParseDeclCXX.cpp
+++ b/lib/Parse/ParseDeclCXX.cpp
@@ -3420,11 +3420,10 @@ Parser::tryParseExceptionSpecification(bool Delayed,
     NoexceptExpr = ParseConstantExpression();
     T.consumeClose();
     // The argument must be contextually convertible to bool. We use
-    // CheckBooleanCondition for this purpose.
-    // FIXME: Add a proper Sema entry point for this.
+    // ActOnBooleanCondition for this purpose.
     if (!NoexceptExpr.isInvalid()) {
-      NoexceptExpr =
-          Actions.CheckBooleanCondition(KeywordLoc, NoexceptExpr.get());
+      NoexceptExpr = Actions.ActOnBooleanCondition(getCurScope(), KeywordLoc,
+                                                   NoexceptExpr.get());
       NoexceptRange = SourceRange(KeywordLoc, T.getCloseLocation());
     } else {
       NoexceptType = EST_None;
diff --git a/lib/Parse/ParseExprCXX.cpp b/lib/Parse/ParseExprCXX.cpp
index 00f20e5e7b..b0740970d8 100644
--- a/lib/Parse/ParseExprCXX.cpp
+++ b/lib/Parse/ParseExprCXX.cpp
@@ -1726,19 +1726,27 @@ Parser::ParseCXXTypeConstructExpression(const DeclSpec &DS) {
 /// [GNU]   type-specifier-seq declarator simple-asm-expr[opt] attributes[opt]
 ///             '=' assignment-expression
 ///
+/// \param ExprOut if the condition was parsed as an expression, the parsed
+/// expression.
+///
+/// \param DeclOut if the condition was parsed as a declaration, the parsed
+/// declaration.
+///
 /// \param Loc The location of the start of the statement that requires this
 /// condition, e.g., the "for" in a for loop.
 ///
 /// \param ConvertToBoolean Whether the condition expression should be
 /// converted to a boolean value.
 ///
-/// \returns The parsed condition.
-Sema::ConditionResult Parser::ParseCXXCondition(SourceLocation Loc,
-                                                Sema::ConditionKind CK) {
+/// \returns true if there was a parsing, false otherwise.
+bool Parser::ParseCXXCondition(ExprResult &ExprOut,
+                               Decl *&DeclOut,
+                               SourceLocation Loc,
+                               bool ConvertToBoolean) {
   if (Tok.is(tok::code_completion)) {
     Actions.CodeCompleteOrdinaryName(getCurScope(), Sema::PCC_Condition);
     cutOffParsing();
-    return Sema::ConditionError();
+    return true;
   }
 
   ParsedAttributesWithRange attrs(AttrFactory);
@@ -1748,11 +1756,16 @@ Sema::ConditionResult Parser::ParseCXXCondition(SourceLocation Loc,
     ProhibitAttributes(attrs);
 
     // Parse the expression.
-    ExprResult Expr = ParseExpression(); // expression
-    if (Expr.isInvalid())
-      return Sema::ConditionError();
+    ExprOut = ParseExpression(); // expression
+    DeclOut = nullptr;
+    if (ExprOut.isInvalid())
+      return true;
 
-    return Actions.ActOnCondition(getCurScope(), Loc, Expr.get(), CK);
+    // If required, convert to a boolean value.
+    if (ConvertToBoolean)
+      ExprOut
+        = Actions.ActOnBooleanCondition(getCurScope(), Loc, ExprOut.get());
+    return ExprOut.isInvalid();
   }
 
   // type-specifier-seq
@@ -1770,7 +1783,7 @@ Sema::ConditionResult Parser::ParseCXXCondition(SourceLocation Loc,
     ExprResult AsmLabel(ParseSimpleAsm(&Loc));
     if (AsmLabel.isInvalid()) {
       SkipUntil(tok::semi, StopAtSemi);
-      return Sema::ConditionError();
+      return true;
     }
     DeclaratorInfo.setAsmLabel(AsmLabel.get());
     DeclaratorInfo.SetRangeEnd(Loc);
@@ -1782,9 +1795,8 @@ Sema::ConditionResult Parser::ParseCXXCondition(SourceLocation Loc,
   // Type-check the declaration itself.
   DeclResult Dcl = Actions.ActOnCXXConditionDeclaration(getCurScope(), 
                                                         DeclaratorInfo);
-  if (Dcl.isInvalid())
-    return Sema::ConditionError();
-  Decl *DeclOut = Dcl.get();
+  DeclOut = Dcl.get();
+  ExprOut = ExprError();
 
   // '=' assignment-expression
   // If a '==' or '+=' is found, suggest a fixit to '='.
@@ -1804,11 +1816,12 @@ Sema::ConditionResult Parser::ParseCXXCondition(SourceLocation Loc,
     SourceLocation LParen = ConsumeParen(), RParen = LParen;
     if (SkipUntil(tok::r_paren, StopAtSemi | StopBeforeMatch))
       RParen = ConsumeParen();
-    Diag(DeclOut->getLocation(),
+    Diag(DeclOut ? DeclOut->getLocation() : LParen,
          diag::err_expected_init_in_condition_lparen)
       << SourceRange(LParen, RParen);
   } else {
-    Diag(DeclOut->getLocation(), diag::err_expected_init_in_condition);
+    Diag(DeclOut ? DeclOut->getLocation() : Tok.getLocation(),
+         diag::err_expected_init_in_condition);
   }
 
   if (!InitExpr.isInvalid())
@@ -1821,7 +1834,8 @@ Sema::ConditionResult Parser::ParseCXXCondition(SourceLocation Loc,
   // (This is currently handled by Sema).
 
   Actions.FinalizeDeclaration(DeclOut);
-  return Actions.ActOnConditionVariable(DeclOut, Loc, CK);
+  
+  return false;
 }
 
 /// ParseCXXSimpleTypeSpecifier - [C++ 7.1.5.2] Simple type specifiers.
diff --git a/lib/Parse/ParseStmt.cpp b/lib/Parse/ParseStmt.cpp
index c849554238..8173d09cbb 100644
--- a/lib/Parse/ParseStmt.cpp
+++ b/lib/Parse/ParseStmt.cpp
@@ -1052,28 +1052,29 @@ StmtResult Parser::ParseCompoundStatementBody(bool isStmtExpr) {
 /// should try to recover harder.  It returns false if the condition is
 /// successfully parsed.  Note that a successful parse can still have semantic
 /// errors in the condition.
-bool Parser::ParseParenExprOrCondition(Sema::ConditionResult &Cond,
+bool Parser::ParseParenExprOrCondition(ExprResult &ExprResult,
+                                       Decl *&DeclResult,
                                        SourceLocation Loc,
-                                       Sema::ConditionKind CK) {
+                                       bool ConvertToBoolean) {
   BalancedDelimiterTracker T(*this, tok::l_paren);
   T.consumeOpen();
 
   if (getLangOpts().CPlusPlus)
-    Cond = ParseCXXCondition(Loc, CK);
+    ParseCXXCondition(ExprResult, DeclResult, Loc, ConvertToBoolean);
   else {
-    ExprResult CondExpr = ParseExpression();
+    ExprResult = ParseExpression();
+    DeclResult = nullptr;
 
     // If required, convert to a boolean value.
-    if (CondExpr.isInvalid())
-      Cond = Sema::ConditionError();
-    else
-      Cond = Actions.ActOnCondition(getCurScope(), Loc, CondExpr.get(), CK);
+    if (!ExprResult.isInvalid() && ConvertToBoolean)
+      ExprResult
+        = Actions.ActOnBooleanCondition(getCurScope(), Loc, ExprResult.get());
   }
 
   // If the parser was confused by the condition and we don't have a ')', try to
   // recover by skipping ahead to a semi and bailing out.  If condexp is
   // semantically invalid but we have well formed code, keep going.
-  if (Cond.isInvalid() && Tok.isNot(tok::r_paren)) {
+  if (ExprResult.isInvalid() && !DeclResult && Tok.isNot(tok::r_paren)) {
     SkipUntil(tok::semi);
     // Skipping may have stopped if it found the containing ')'.  If so, we can
     // continue parsing the if statement.
@@ -1131,10 +1132,13 @@ StmtResult Parser::ParseIfStatement(SourceLocation *TrailingElseLoc) {
   ParseScope IfScope(this, Scope::DeclScope | Scope::ControlScope, C99orCXX);
 
   // Parse the condition.
-  Sema::ConditionResult Cond;
-  if (ParseParenExprOrCondition(Cond, IfLoc, Sema::ConditionKind::Boolean))
+  ExprResult CondExp;
+  Decl *CondVar = nullptr;
+  if (ParseParenExprOrCondition(CondExp, CondVar, IfLoc, true))
     return StmtError();
 
+  FullExprArg FullCondExp(Actions.MakeFullExpr(CondExp.get(), IfLoc));
+
   // C99 6.8.4p3 - In C99, the body of the if statement is a scope, even if
   // there is no compound stmt.  C90 does not have this clause.  We only do this
   // if the body isn't a compound statement to avoid push/pop in common cases.
@@ -1217,8 +1221,8 @@ StmtResult Parser::ParseIfStatement(SourceLocation *TrailingElseLoc) {
   if (ElseStmt.isInvalid())
     ElseStmt = Actions.ActOnNullStmt(ElseStmtLoc);
 
-  return Actions.ActOnIfStmt(IfLoc, Cond, ThenStmt.get(), ElseLoc,
-                             ElseStmt.get());
+  return Actions.ActOnIfStmt(IfLoc, FullCondExp, CondVar, ThenStmt.get(),
+                             ElseLoc, ElseStmt.get());
 }
 
 /// ParseSwitchStatement
@@ -1255,11 +1259,13 @@ StmtResult Parser::ParseSwitchStatement(SourceLocation *TrailingElseLoc) {
   ParseScope SwitchScope(this, ScopeFlags);
 
   // Parse the condition.
-  Sema::ConditionResult Cond;
-  if (ParseParenExprOrCondition(Cond, SwitchLoc, Sema::ConditionKind::Switch))
+  ExprResult Cond;
+  Decl *CondVar = nullptr;
+  if (ParseParenExprOrCondition(Cond, CondVar, SwitchLoc, false))
     return StmtError();
 
-  StmtResult Switch = Actions.ActOnStartOfSwitchStmt(SwitchLoc, Cond);
+  StmtResult Switch
+    = Actions.ActOnStartOfSwitchStmt(SwitchLoc, Cond.get(), CondVar);
 
   if (Switch.isInvalid()) {
     // Skip the switch body.
@@ -1341,10 +1347,13 @@ StmtResult Parser::ParseWhileStatement(SourceLocation *TrailingElseLoc) {
   ParseScope WhileScope(this, ScopeFlags);
 
   // Parse the condition.
-  Sema::ConditionResult Cond;
-  if (ParseParenExprOrCondition(Cond, WhileLoc, Sema::ConditionKind::Boolean))
+  ExprResult Cond;
+  Decl *CondVar = nullptr;
+  if (ParseParenExprOrCondition(Cond, CondVar, WhileLoc, true))
     return StmtError();
 
+  FullExprArg FullCond(Actions.MakeFullExpr(Cond.get(), WhileLoc));
+
   // C99 6.8.5p5 - In C99, the body of the while statement is a scope, even if
   // there is no compound stmt.  C90 does not have this clause.  We only do this
   // if the body isn't a compound statement to avoid push/pop in common cases.
@@ -1365,10 +1374,10 @@ StmtResult Parser::ParseWhileStatement(SourceLocation *TrailingElseLoc) {
   InnerScope.Exit();
   WhileScope.Exit();
 
-  if (Cond.isInvalid() || Body.isInvalid())
+  if ((Cond.isInvalid() && !CondVar) || Body.isInvalid())
     return StmtError();
 
-  return Actions.ActOnWhileStmt(WhileLoc, Cond, Body.get());
+  return Actions.ActOnWhileStmt(WhileLoc, FullCond, CondVar, Body.get());
 }
 
 /// ParseDoStatement
@@ -1526,10 +1535,12 @@ StmtResult Parser::ParseForStatement(SourceLocation *TrailingElseLoc) {
 
   bool ForEach = false, ForRange = false;
   StmtResult FirstPart;
-  Sema::ConditionResult SecondPart;
+  bool SecondPartIsInvalid = false;
+  FullExprArg SecondPart(Actions);
   ExprResult Collection;
   ForRangeInit ForRangeInit;
   FullExprArg ThirdPart(Actions);
+  Decl *SecondVar = nullptr;
 
   if (Tok.is(tok::code_completion)) {
     Actions.CodeCompleteOrdinaryName(getCurScope(),
@@ -1634,7 +1645,7 @@ StmtResult Parser::ParseForStatement(SourceLocation *TrailingElseLoc) {
       Diag(Tok, diag::err_for_range_expected_decl)
         << FirstPart.get()->getSourceRange();
       SkipUntil(tok::r_paren, StopBeforeMatch);
-      SecondPart = Sema::ConditionError();
+      SecondPartIsInvalid = true;
     } else {
       if (!Value.isInvalid()) {
         Diag(Tok, diag::err_expected_semi_for);
@@ -1649,28 +1660,29 @@ StmtResult Parser::ParseForStatement(SourceLocation *TrailingElseLoc) {
 
   // Parse the second part of the for specifier.
   getCurScope()->AddFlags(Scope::BreakScope | Scope::ContinueScope);
-  if (!ForEach && !ForRange && !SecondPart.isInvalid()) {
+  if (!ForEach && !ForRange) {
+    assert(!SecondPart.get() && "Shouldn't have a second expression yet.");
     // Parse the second part of the for specifier.
     if (Tok.is(tok::semi)) {  // for (...;;
       // no second part.
     } else if (Tok.is(tok::r_paren)) {
       // missing both semicolons.
     } else {
+      ExprResult Second;
       if (getLangOpts().CPlusPlus)
-        SecondPart = ParseCXXCondition(ForLoc, Sema::ConditionKind::Boolean);
+        ParseCXXCondition(Second, SecondVar, ForLoc, true);
       else {
-        ExprResult SecondExpr = ParseExpression();
-        if (SecondExpr.isInvalid())
-          SecondPart = Sema::ConditionError();
-        else
-          SecondPart =
-              Actions.ActOnCondition(getCurScope(), ForLoc, SecondExpr.get(),
-                                     Sema::ConditionKind::Boolean);
+        Second = ParseExpression();
+        if (!Second.isInvalid())
+          Second = Actions.ActOnBooleanCondition(getCurScope(), ForLoc,
+                                                 Second.get());
       }
+      SecondPartIsInvalid = Second.isInvalid();
+      SecondPart = Actions.MakeFullExpr(Second.get(), ForLoc);
     }
 
     if (Tok.isNot(tok::semi)) {
-      if (!SecondPart.isInvalid())
+      if (!SecondPartIsInvalid || SecondVar)
         Diag(Tok, diag::err_expected_semi_for);
       else
         // Skip until semicolon or rparen, don't consume it.
@@ -1769,8 +1781,8 @@ StmtResult Parser::ParseForStatement(SourceLocation *TrailingElseLoc) {
     return Actions.FinishCXXForRangeStmt(ForRangeStmt.get(), Body.get());
 
   return Actions.ActOnForStmt(ForLoc, T.getOpenLocation(), FirstPart.get(),
-                              SecondPart, ThirdPart, T.getCloseLocation(),
-                              Body.get());
+                              SecondPart, SecondVar, ThirdPart,
+                              T.getCloseLocation(), Body.get());
 }
 
 /// ParseGotoStatement
diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index 8eeaef7df5..00d5651b14 100644
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -10092,10 +10092,10 @@ buildSingleCopyAssignRecursively(Sema &S, SourceLocation Loc, QualType T,
                                     SizeType, VK_LValue, OK_Ordinary, Loc);
 
   // Construct the loop that copies all elements of this array.
-  return S.ActOnForStmt(
-      Loc, Loc, InitStmt,
-      S.ActOnCondition(nullptr, Loc, Comparison, Sema::ConditionKind::Boolean),
-      S.MakeFullDiscardedValueExpr(Increment), Loc, Copy.get());
+  return S.ActOnForStmt(Loc, Loc, InitStmt, 
+                        S.MakeFullExpr(Comparison),
+                        nullptr, S.MakeFullDiscardedValueExpr(Increment),
+                        Loc, Copy.get());
 }
 
 static StmtResult
diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index f975b817e0..755d99d54b 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -14341,7 +14341,7 @@ void Sema::DiagnoseEqualityWithExtraParens(ParenExpr *ParenE) {
     }
 }
 
-ExprResult Sema::CheckBooleanCondition(SourceLocation Loc, Expr *E) {
+ExprResult Sema::CheckBooleanCondition(Expr *E, SourceLocation Loc) {
   DiagnoseAssignmentAsCondition(E);
   if (ParenExpr *parenE = dyn_cast<ParenExpr>(E))
     DiagnoseEqualityWithExtraParens(parenE);
@@ -14371,26 +14371,12 @@ ExprResult Sema::CheckBooleanCondition(SourceLocation Loc, Expr *E) {
   return E;
 }
 
-Sema::ConditionResult Sema::ActOnCondition(Scope *S, SourceLocation Loc,
-                                           Expr *SubExpr, ConditionKind CK) {
-  // Empty conditions are valid in for-statements.
+ExprResult Sema::ActOnBooleanCondition(Scope *S, SourceLocation Loc,
+                                       Expr *SubExpr) {
   if (!SubExpr)
-    return ConditionResult();
-
-  ExprResult Cond;
-  switch (CK) {
-  case ConditionKind::Boolean:
-    Cond = CheckBooleanCondition(Loc, SubExpr);
-    break;
-
-  case ConditionKind::Switch:
-    Cond = CheckSwitchCondition(Loc, SubExpr);
-    break;
-  }
-  if (Cond.isInvalid())
-    return ConditionError();
+    return ExprError();
 
-  return ConditionResult(nullptr, MakeFullExpr(Cond.get(), Loc));
+  return CheckBooleanCondition(SubExpr, Loc);
 }
 
 namespace {
diff --git a/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp
index f97b9c9d81..d5e944fa11 100644
--- a/lib/Sema/SemaExprCXX.cpp
+++ b/lib/Sema/SemaExprCXX.cpp
@@ -3054,21 +3054,11 @@ void Sema::CheckVirtualDtorCall(CXXDestructorDecl *dtor, SourceLocation Loc,
   }
 }
 
-Sema::ConditionResult Sema::ActOnConditionVariable(Decl *ConditionVar,
-                                                   SourceLocation StmtLoc,
-                                                   ConditionKind CK) {
-  ExprResult E =
-      CheckConditionVariable(cast<VarDecl>(ConditionVar), StmtLoc, CK);
-  if (E.isInvalid())
-    return ConditionError();
-  return ConditionResult(ConditionVar, MakeFullExpr(E.get(), StmtLoc));
-}
-
 /// \brief Check the use of the given variable as a C++ condition in an if,
 /// while, do-while, or switch statement.
 ExprResult Sema::CheckConditionVariable(VarDecl *ConditionVar,
                                         SourceLocation StmtLoc,
-                                        ConditionKind CK) {
+                                        bool ConvertToBoolean) {
   if (ConditionVar->isInvalidDecl())
     return ExprError();
 
@@ -3092,15 +3082,13 @@ ExprResult Sema::CheckConditionVariable(VarDecl *ConditionVar,
 
   MarkDeclRefReferenced(cast<DeclRefExpr>(Condition.get()));
 
-  switch (CK) {
-  case ConditionKind::Boolean:
-    return CheckBooleanCondition(StmtLoc, Condition.get());
-
-  case ConditionKind::Switch:
-    return CheckSwitchCondition(StmtLoc, Condition.get());
+  if (ConvertToBoolean) {
+    Condition = CheckBooleanCondition(Condition.get(), StmtLoc);
+    if (Condition.isInvalid())
+      return ExprError();
   }
 
-  llvm_unreachable("unexpected condition kind");
+  return Condition;
 }
 
 /// CheckCXXBooleanCondition - Returns true if a conversion to bool is invalid.
diff --git a/lib/Sema/SemaOpenMP.cpp b/lib/Sema/SemaOpenMP.cpp
index ca84b8cfed..5f28aa37e6 100644
--- a/lib/Sema/SemaOpenMP.cpp
+++ b/lib/Sema/SemaOpenMP.cpp
@@ -6826,11 +6826,12 @@ OMPClause *Sema::ActOnOpenMPIfClause(OpenMPDirectiveKind NameModifier,
   if (!Condition->isValueDependent() && !Condition->isTypeDependent() &&
       !Condition->isInstantiationDependent() &&
       !Condition->containsUnexpandedParameterPack()) {
-    ExprResult Val = CheckBooleanCondition(StartLoc, Condition);
+    ExprResult Val = ActOnBooleanCondition(DSAStack->getCurScope(),
+                                           Condition->getExprLoc(), Condition);
     if (Val.isInvalid())
       return nullptr;
 
-    ValExpr = MakeFullExpr(Val.get()).get();
+    ValExpr = Val.get();
   }
 
   return new (Context) OMPIfClause(NameModifier, ValExpr, StartLoc, LParenLoc,
@@ -6845,11 +6846,12 @@ OMPClause *Sema::ActOnOpenMPFinalClause(Expr *Condition,
   if (!Condition->isValueDependent() && !Condition->isTypeDependent() &&
       !Condition->isInstantiationDependent() &&
       !Condition->containsUnexpandedParameterPack()) {
-    ExprResult Val = CheckBooleanCondition(StartLoc, Condition);
+    ExprResult Val = ActOnBooleanCondition(DSAStack->getCurScope(),
+                                           Condition->getExprLoc(), Condition);
     if (Val.isInvalid())
       return nullptr;
 
-    ValExpr = MakeFullExpr(Val.get()).get();
+    ValExpr = Val.get();
   }
 
   return new (Context) OMPFinalClause(ValExpr, StartLoc, LParenLoc, EndLoc);
diff --git a/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp
index 56aa042a7b..7156cd2a9a 100644
--- a/lib/Sema/SemaStmt.cpp
+++ b/lib/Sema/SemaStmt.cpp
@@ -504,30 +504,39 @@ public:
 }
 
 StmtResult
-Sema::ActOnIfStmt(SourceLocation IfLoc, ConditionResult Cond,
+Sema::ActOnIfStmt(SourceLocation IfLoc, FullExprArg CondVal, Decl *CondVar,
                   Stmt *thenStmt, SourceLocation ElseLoc,
                   Stmt *elseStmt) {
-  auto CondVal = Cond.get();
-  if (Cond.isInvalid()) {
-    CondVal.first = nullptr;
-    CondVal.second = new (Context)
-        OpaqueValueExpr(SourceLocation(), Context.BoolTy, VK_RValue);
+  ExprResult CondResult(CondVal.release());
+
+  VarDecl *ConditionVar = nullptr;
+  if (CondVar) {
+    ConditionVar = cast<VarDecl>(CondVar);
+    CondResult = CheckConditionVariable(ConditionVar, IfLoc, true);
+    CondResult = ActOnFinishFullExpr(CondResult.get(), IfLoc);
   }
+  Expr *ConditionExpr = CondResult.getAs<Expr>();
+  if (ConditionExpr) {
 
-  if (!Diags.isIgnored(diag::warn_comma_operator,
-                       CondVal.second->getExprLoc()))
-    CommaVisitor(*this).Visit(CondVal.second);
+    if (!Diags.isIgnored(diag::warn_comma_operator,
+                         ConditionExpr->getExprLoc()))
+      CommaVisitor(*this).Visit(ConditionExpr);
 
-  DiagnoseUnusedExprResult(thenStmt);
+    DiagnoseUnusedExprResult(thenStmt);
 
-  if (!elseStmt) {
-    DiagnoseEmptyStmtBody(CondVal.second->getLocEnd(), thenStmt,
-                          diag::warn_empty_if_body);
-  }
+    if (!elseStmt) {
+      DiagnoseEmptyStmtBody(ConditionExpr->getLocEnd(), thenStmt,
+                            diag::warn_empty_if_body);
+    }
 
-  DiagnoseUnusedExprResult(elseStmt);
+    DiagnoseUnusedExprResult(elseStmt);
+  } else {
+    // Create a dummy Expr for the condition for error recovery
+    ConditionExpr = new (Context) OpaqueValueExpr(SourceLocation(),
+                                                  Context.BoolTy, VK_RValue);
+  }
 
-  return new (Context) IfStmt(Context, IfLoc, CondVal.first, CondVal.second,
+  return new (Context) IfStmt(Context, IfLoc, ConditionVar, ConditionExpr,
                               thenStmt, ElseLoc, elseStmt);
 }
 
@@ -590,7 +599,24 @@ static QualType GetTypeBeforeIntegralPromotion(Expr *&expr) {
   return expr->getType();
 }
 
-ExprResult Sema::CheckSwitchCondition(SourceLocation SwitchLoc, Expr *Cond) {
+StmtResult
+Sema::ActOnStartOfSwitchStmt(SourceLocation SwitchLoc, Expr *Cond,
+                             Decl *CondVar) {
+  ExprResult CondResult;
+
+  VarDecl *ConditionVar = nullptr;
+  if (CondVar) {
+    ConditionVar = cast<VarDecl>(CondVar);
+    CondResult = CheckConditionVariable(ConditionVar, SourceLocation(), false);
+    if (CondResult.isInvalid())
+      return StmtError();
+
+    Cond = CondResult.get();
+  }
+
+  if (!Cond)
+    return StmtError();
+
   class SwitchConvertDiagnoser : public ICEConvertDiagnoser {
     Expr *Cond;
 
@@ -638,24 +664,24 @@ ExprResult Sema::CheckSwitchCondition(SourceLocation SwitchLoc, Expr *Cond) {
     }
   } SwitchDiagnoser(Cond);
 
-  ExprResult CondResult =
+  CondResult =
       PerformContextualImplicitConversion(SwitchLoc, Cond, SwitchDiagnoser);
-  if (CondResult.isInvalid())
-    return ExprError();
+  if (CondResult.isInvalid()) return StmtError();
+  Cond = CondResult.get();
 
   // C99 6.8.4.2p5 - Integer promotions are performed on the controlling expr.
-  return UsualUnaryConversions(CondResult.get());
-}
+  CondResult = UsualUnaryConversions(Cond);
+  if (CondResult.isInvalid()) return StmtError();
+  Cond = CondResult.get();
 
-StmtResult
-Sema::ActOnStartOfSwitchStmt(SourceLocation SwitchLoc, ConditionResult Cond) {
-  if (Cond.isInvalid())
+  CondResult = ActOnFinishFullExpr(Cond, SwitchLoc);
+  if (CondResult.isInvalid())
     return StmtError();
+  Cond = CondResult.get();
 
   getCurFunction()->setHasBranchIntoScope();
 
-  SwitchStmt *SS =
-      new (Context) SwitchStmt(Context, Cond.get().first, Cond.get().second);
+  SwitchStmt *SS = new (Context) SwitchStmt(Context, ConditionVar, Cond);
   getCurFunction()->SwitchStack.push_back(SS);
   return SS;
 }
@@ -1216,17 +1242,27 @@ Sema::DiagnoseAssignmentEnum(QualType DstType, QualType SrcType,
     }
 }
 
-StmtResult Sema::ActOnWhileStmt(SourceLocation WhileLoc, ConditionResult Cond,
-                                Stmt *Body) {
-  if (Cond.isInvalid())
+StmtResult
+Sema::ActOnWhileStmt(SourceLocation WhileLoc, FullExprArg Cond,
+                     Decl *CondVar, Stmt *Body) {
+  ExprResult CondResult(Cond.release());
+
+  VarDecl *ConditionVar = nullptr;
+  if (CondVar) {
+    ConditionVar = cast<VarDecl>(CondVar);
+    CondResult = CheckConditionVariable(ConditionVar, WhileLoc, true);
+    CondResult = ActOnFinishFullExpr(CondResult.get(), WhileLoc);
+    if (CondResult.isInvalid())
+      return StmtError();
+  }
+  Expr *ConditionExpr = CondResult.get();
+  if (!ConditionExpr)
     return StmtError();
+  CheckBreakContinueBinding(ConditionExpr);
 
-  auto CondVal = Cond.get();
-  CheckBreakContinueBinding(CondVal.second);
-
-  if (CondVal.second &&
-      !Diags.isIgnored(diag::warn_comma_operator, CondVal.second->getExprLoc()))
-    CommaVisitor(*this).Visit(CondVal.second);
+  if (ConditionExpr &&
+      !Diags.isIgnored(diag::warn_comma_operator, ConditionExpr->getExprLoc()))
+    CommaVisitor(*this).Visit(ConditionExpr);
 
   DiagnoseUnusedExprResult(Body);
 
@@ -1234,7 +1270,7 @@ StmtResult Sema::ActOnWhileStmt(SourceLocation WhileLoc, ConditionResult Cond,
     getCurCompoundScope().setHasEmptyLoopBodies();
 
   return new (Context)
-      WhileStmt(Context, CondVal.first, CondVal.second, Body, WhileLoc);
+      WhileStmt(Context, ConditionVar, ConditionExpr, Body, WhileLoc);
 }
 
 StmtResult
@@ -1244,7 +1280,7 @@ Sema::ActOnDoStmt(SourceLocation DoLoc, Stmt *Body,
   assert(Cond && "ActOnDoStmt(): missing expression");
 
   CheckBreakContinueBinding(Cond);
-  ExprResult CondResult = CheckBooleanCondition(DoLoc, Cond);
+  ExprResult CondResult = CheckBooleanCondition(Cond, DoLoc);
   if (CondResult.isInvalid())
     return StmtError();
   Cond = CondResult.get();
@@ -1608,13 +1644,11 @@ void Sema::CheckBreakContinueBinding(Expr *E) {
   }
 }
 
-StmtResult Sema::ActOnForStmt(SourceLocation ForLoc, SourceLocation LParenLoc,
-                              Stmt *First, ConditionResult Second,
-                              FullExprArg third, SourceLocation RParenLoc,
-                              Stmt *Body) {
-  if (Second.isInvalid())
-    return StmtError();
-
+StmtResult
+Sema::ActOnForStmt(SourceLocation ForLoc, SourceLocation LParenLoc,
+                   Stmt *First, FullExprArg second, Decl *secondVar,
+                   FullExprArg third,
+                   SourceLocation RParenLoc, Stmt *Body) {
   if (!getLangOpts().CPlusPlus) {
     if (DeclStmt *DS = dyn_cast_or_null<DeclStmt>(First)) {
       // C99 6.8.5p3: The declaration part of a 'for' statement shall only
@@ -1632,17 +1666,26 @@ StmtResult Sema::ActOnForStmt(SourceLocation ForLoc, SourceLocation LParenLoc,
     }
   }
 
-  CheckBreakContinueBinding(Second.get().second);
+  CheckBreakContinueBinding(second.get());
   CheckBreakContinueBinding(third.get());
 
-  CheckForLoopConditionalStatement(*this, Second.get().second, third.get(),
-                                   Body);
+  CheckForLoopConditionalStatement(*this, second.get(), third.get(), Body);
   CheckForRedundantIteration(*this, third.get(), Body);
 
-  if (Second.get().second &&
+  ExprResult SecondResult(second.release());
+  VarDecl *ConditionVar = nullptr;
+  if (secondVar) {
+    ConditionVar = cast<VarDecl>(secondVar);
+    SecondResult = CheckConditionVariable(ConditionVar, ForLoc, true);
+    SecondResult = ActOnFinishFullExpr(SecondResult.get(), ForLoc);
+    if (SecondResult.isInvalid())
+      return StmtError();
+  }
+
+  if (SecondResult.get() &&
       !Diags.isIgnored(diag::warn_comma_operator,
-                       Second.get().second->getExprLoc()))
-    CommaVisitor(*this).Visit(Second.get().second);
+                       SecondResult.get()->getExprLoc()))
+    CommaVisitor(*this).Visit(SecondResult.get());
 
   Expr *Third  = third.release().getAs<Expr>();
 
@@ -1653,9 +1696,8 @@ StmtResult Sema::ActOnForStmt(SourceLocation ForLoc, SourceLocation LParenLoc,
   if (isa<NullStmt>(Body))
     getCurCompoundScope().setHasEmptyLoopBodies();
 
-  return new (Context)
-      ForStmt(Context, First, Second.get().second, Second.get().first, Third,
-              Body, ForLoc, LParenLoc, RParenLoc);
+  return new (Context) ForStmt(Context, First, SecondResult.get(), ConditionVar,
+                               Third, Body, ForLoc, LParenLoc, RParenLoc);
 }
 
 /// In an Objective C collection iteration statement:
@@ -2342,10 +2384,8 @@ Sema::BuildCXXForRangeStmt(SourceLocation ForLoc, SourceLocation CoawaitLoc,
     // Build and check __begin != __end expression.
     NotEqExpr = ActOnBinOp(S, ColonLoc, tok::exclaimequal,
                            BeginRef.get(), EndRef.get());
-    if (!NotEqExpr.isInvalid())
-      NotEqExpr = CheckBooleanCondition(ColonLoc, NotEqExpr.get());
-    if (!NotEqExpr.isInvalid())
-      NotEqExpr = ActOnFinishFullExpr(NotEqExpr.get());
+    NotEqExpr = ActOnBooleanCondition(S, ColonLoc, NotEqExpr.get());
+    NotEqExpr = ActOnFinishFullExpr(NotEqExpr.get());
     if (NotEqExpr.isInvalid()) {
       Diag(RangeLoc, diag::note_for_range_invalid_iterator)
         << RangeLoc << 0 << BeginRangeRef.get()->getType();
diff --git a/lib/Sema/TreeTransform.h b/lib/Sema/TreeTransform.h
index 5814197a7a..4bdd9e548a 100644
--- a/lib/Sema/TreeTransform.h
+++ b/lib/Sema/TreeTransform.h
@@ -410,14 +410,6 @@ public:
     return D;
   }
 
-  /// \brief Transform the specified condition.
-  ///
-  /// By default, this transforms the variable and expression and rebuilds
-  /// the condition.
-  Sema::ConditionResult TransformCondition(SourceLocation Loc, VarDecl *Var,
-                                           Expr *Expr,
-                                           Sema::ConditionKind Kind);
-
   /// \brief Transform the attributes associated with the given declaration and
   /// place them on the new declaration.
   ///
@@ -1174,9 +1166,10 @@ public:
   ///
   /// By default, performs semantic analysis to build the new statement.
   /// Subclasses may override this routine to provide different behavior.
-  StmtResult RebuildIfStmt(SourceLocation IfLoc, Sema::ConditionResult Cond,
-                           Stmt *Then, SourceLocation ElseLoc, Stmt *Else) {
-    return getSema().ActOnIfStmt(IfLoc, Cond, Then, ElseLoc, Else);
+  StmtResult RebuildIfStmt(SourceLocation IfLoc, Sema::FullExprArg Cond,
+                           VarDecl *CondVar, Stmt *Then,
+                           SourceLocation ElseLoc, Stmt *Else) {
+    return getSema().ActOnIfStmt(IfLoc, Cond, CondVar, Then, ElseLoc, Else);
   }
 
   /// \brief Start building a new switch statement.
@@ -1184,8 +1177,9 @@ public:
   /// By default, performs semantic analysis to build the new statement.
   /// Subclasses may override this routine to provide different behavior.
   StmtResult RebuildSwitchStmtStart(SourceLocation SwitchLoc,
-                                    Sema::ConditionResult Cond) {
-    return getSema().ActOnStartOfSwitchStmt(SwitchLoc, Cond);
+                                    Expr *Cond, VarDecl *CondVar) {
+    return getSema().ActOnStartOfSwitchStmt(SwitchLoc, Cond,
+                                            CondVar);
   }
 
   /// \brief Attach the body to the switch statement.
@@ -1201,9 +1195,9 @@ public:
   ///
   /// By default, performs semantic analysis to build the new statement.
   /// Subclasses may override this routine to provide different behavior.
-  StmtResult RebuildWhileStmt(SourceLocation WhileLoc,
-                              Sema::ConditionResult Cond, Stmt *Body) {
-    return getSema().ActOnWhileStmt(WhileLoc, Cond, Body);
+  StmtResult RebuildWhileStmt(SourceLocation WhileLoc, Sema::FullExprArg Cond,
+                              VarDecl *CondVar, Stmt *Body) {
+    return getSema().ActOnWhileStmt(WhileLoc, Cond, CondVar, Body);
   }
 
   /// \brief Build a new do-while statement.
@@ -1222,11 +1216,11 @@ public:
   /// By default, performs semantic analysis to build the new statement.
   /// Subclasses may override this routine to provide different behavior.
   StmtResult RebuildForStmt(SourceLocation ForLoc, SourceLocation LParenLoc,
-                            Stmt *Init, Sema::ConditionResult Cond,
-                            Sema::FullExprArg Inc, SourceLocation RParenLoc,
-                            Stmt *Body) {
+                            Stmt *Init, Sema::FullExprArg Cond,
+                            VarDecl *CondVar, Sema::FullExprArg Inc,
+                            SourceLocation RParenLoc, Stmt *Body) {
     return getSema().ActOnForStmt(ForLoc, LParenLoc, Init, Cond,
-                                  Inc, RParenLoc, Body);
+                                  CondVar, Inc, RParenLoc, Body);
   }
 
   /// \brief Build a new goto statement.
@@ -3363,31 +3357,6 @@ bool TreeTransform<Derived>::TransformExprs(Expr *const *Inputs,
   return false;
 }
 
-template <typename Derived>
-Sema::ConditionResult TreeTransform<Derived>::TransformCondition(
-    SourceLocation Loc, VarDecl *Var, Expr *Expr, Sema::ConditionKind Kind) {
-  if (Var) {
-    VarDecl *ConditionVar = cast_or_null<VarDecl>(
-        getDerived().TransformDefinition(Var->getLocation(), Var));
-
-    if (!ConditionVar)
-      return Sema::ConditionError();
-
-    return getSema().ActOnConditionVariable(ConditionVar, Loc, Kind);
-  }
-
-  if (Expr) {
-    ExprResult CondExpr = getDerived().TransformExpr(Expr);
-
-    if (CondExpr.isInvalid())
-      return Sema::ConditionError();
-
-    return getSema().ActOnCondition(nullptr, Loc, CondExpr.get(), Kind);
-  }
-
-  return Sema::ConditionResult();
-}
-
 template<typename Derived>
 NestedNameSpecifierLoc
 TreeTransform<Derived>::TransformNestedNameSpecifierLoc(
@@ -4993,8 +4962,8 @@ bool TreeTransform<Derived>::TransformExceptionSpec(
     if (NoexceptExpr.isInvalid())
       return true;
 
-    // FIXME: This is bogus, a noexcept expression is not a condition.
-    NoexceptExpr = getSema().CheckBooleanCondition(Loc, NoexceptExpr.get());
+    NoexceptExpr = getSema().CheckBooleanCondition(
+        NoexceptExpr.get(), NoexceptExpr.get()->getLocStart());
     if (NoexceptExpr.isInvalid())
       return true;
 
@@ -6226,10 +6195,35 @@ template<typename Derived>
 StmtResult
 TreeTransform<Derived>::TransformIfStmt(IfStmt *S) {
   // Transform the condition
-  Sema::ConditionResult Cond = getDerived().TransformCondition(
-      S->getIfLoc(), S->getConditionVariable(), S->getCond(),
-      Sema::ConditionKind::Boolean);
-  if (Cond.isInvalid())
+  ExprResult Cond;
+  VarDecl *ConditionVar = nullptr;
+  if (S->getConditionVariable()) {
+    ConditionVar
+      = cast_or_null<VarDecl>(
+                   getDerived().TransformDefinition(
+                                      S->getConditionVariable()->getLocation(),
+                                                    S->getConditionVariable()));
+    if (!ConditionVar)
+      return StmtError();
+  } else {
+    Cond = getDerived().TransformExpr(S->getCond());
+
+    if (Cond.isInvalid())
+      return StmtError();
+
+    // Convert the condition to a boolean value.
+    if (S->getCond()) {
+      ExprResult CondE = getSema().ActOnBooleanCondition(nullptr, S->getIfLoc(),
+                                                         Cond.get());
+      if (CondE.isInvalid())
+        return StmtError();
+
+      Cond = CondE.get();
+    }
+  }
+
+  Sema::FullExprArg FullCond(getSema().MakeFullExpr(Cond.get(), S->getIfLoc()));
+  if (!S->getConditionVariable() && S->getCond() && !FullCond.get())
     return StmtError();
 
   // Transform the "then" branch.
@@ -6243,12 +6237,14 @@ TreeTransform<Derived>::TransformIfStmt(IfStmt *S) {
     return StmtError();
 
   if (!getDerived().AlwaysRebuild() &&
-      Cond.get() == std::make_pair(S->getConditionVariable(), S->getCond()) &&
+      FullCond.get() == S->getCond() &&
+      ConditionVar == S->getConditionVariable() &&
       Then.get() == S->getThen() &&
       Else.get() == S->getElse())
     return S;
 
-  return getDerived().RebuildIfStmt(S->getIfLoc(), Cond, Then.get(),
+  return getDerived().RebuildIfStmt(S->getIfLoc(), FullCond, ConditionVar,
+                                    Then.get(),
                                     S->getElseLoc(), Else.get());
 }
 
@@ -6256,15 +6252,27 @@ template<typename Derived>
 StmtResult
 TreeTransform<Derived>::TransformSwitchStmt(SwitchStmt *S) {
   // Transform the condition.
-  Sema::ConditionResult Cond = getDerived().TransformCondition(
-      S->getSwitchLoc(), S->getConditionVariable(), S->getCond(),
-      Sema::ConditionKind::Switch);
-  if (Cond.isInvalid())
-    return StmtError();
+  ExprResult Cond;
+  VarDecl *ConditionVar = nullptr;
+  if (S->getConditionVariable()) {
+    ConditionVar
+      = cast_or_null<VarDecl>(
+                   getDerived().TransformDefinition(
+                                      S->getConditionVariable()->getLocation(),
+                                                    S->getConditionVariable()));
+    if (!ConditionVar)
+      return StmtError();
+  } else {
+    Cond = getDerived().TransformExpr(S->getCond());
+
+    if (Cond.isInvalid())
+      return StmtError();
+  }
 
   // Rebuild the switch statement.
   StmtResult Switch
-    = getDerived().RebuildSwitchStmtStart(S->getSwitchLoc(), Cond);
+    = getDerived().RebuildSwitchStmtStart(S->getSwitchLoc(), Cond.get(),
+                                          ConditionVar);
   if (Switch.isInvalid())
     return StmtError();
 
@@ -6282,10 +6290,36 @@ template<typename Derived>
 StmtResult
 TreeTransform<Derived>::TransformWhileStmt(WhileStmt *S) {
   // Transform the condition
-  Sema::ConditionResult Cond = getDerived().TransformCondition(
-      S->getWhileLoc(), S->getConditionVariable(), S->getCond(),
-      Sema::ConditionKind::Boolean);
-  if (Cond.isInvalid())
+  ExprResult Cond;
+  VarDecl *ConditionVar = nullptr;
+  if (S->getConditionVariable()) {
+    ConditionVar
+      = cast_or_null<VarDecl>(
+                   getDerived().TransformDefinition(
+                                      S->getConditionVariable()->getLocation(),
+                                                    S->getConditionVariable()));
+    if (!ConditionVar)
+      return StmtError();
+  } else {
+    Cond = getDerived().TransformExpr(S->getCond());
+
+    if (Cond.isInvalid())
+      return StmtError();
+
+    if (S->getCond()) {
+      // Convert the condition to a boolean value.
+      ExprResult CondE = getSema().ActOnBooleanCondition(nullptr,
+                                                         S->getWhileLoc(),
+                                                         Cond.get());
+      if (CondE.isInvalid())
+        return StmtError();
+      Cond = CondE;
+    }
+  }
+
+  Sema::FullExprArg FullCond(
+      getSema().MakeFullExpr(Cond.get(), S->getWhileLoc()));
+  if (!S->getConditionVariable() && S->getCond() && !FullCond.get())
     return StmtError();
 
   // Transform the body
@@ -6294,11 +6328,13 @@ TreeTransform<Derived>::TransformWhileStmt(WhileStmt *S) {
     return StmtError();
 
   if (!getDerived().AlwaysRebuild() &&
-      Cond.get() == std::make_pair(S->getConditionVariable(), S->getCond()) &&
+      FullCond.get() == S->getCond() &&
+      ConditionVar == S->getConditionVariable() &&
       Body.get() == S->getBody())
     return Owned(S);
 
-  return getDerived().RebuildWhileStmt(S->getWhileLoc(), Cond, Body.get());
+  return getDerived().RebuildWhileStmt(S->getWhileLoc(), FullCond,
+                                       ConditionVar, Body.get());
 }
 
 template<typename Derived>
@@ -6338,10 +6374,37 @@ TreeTransform<Derived>::TransformForStmt(ForStmt *S) {
     getSema().ActOnOpenMPLoopInitialization(S->getForLoc(), Init.get());
 
   // Transform the condition
-  Sema::ConditionResult Cond = getDerived().TransformCondition(
-      S->getForLoc(), S->getConditionVariable(), S->getCond(),
-      Sema::ConditionKind::Boolean);
-  if (Cond.isInvalid())
+  ExprResult Cond;
+  VarDecl *ConditionVar = nullptr;
+  if (S->getConditionVariable()) {
+    ConditionVar
+      = cast_or_null<VarDecl>(
+                   getDerived().TransformDefinition(
+                                      S->getConditionVariable()->getLocation(),
+                                                    S->getConditionVariable()));
+    if (!ConditionVar)
+      return StmtError();
+  } else {
+    Cond = getDerived().TransformExpr(S->getCond());
+
+    if (Cond.isInvalid())
+      return StmtError();
+
+    if (S->getCond()) {
+      // Convert the condition to a boolean value.
+      ExprResult CondE = getSema().ActOnBooleanCondition(nullptr,
+                                                         S->getForLoc(),
+                                                         Cond.get());
+      if (CondE.isInvalid())
+        return StmtError();
+
+      Cond = CondE.get();
+    }
+  }
+
+  Sema::FullExprArg FullCond(
+      getSema().MakeFullExpr(Cond.get(), S->getForLoc()));
+  if (!S->getConditionVariable() && S->getCond() && !FullCond.get())
     return StmtError();
 
   // Transform the increment
@@ -6360,14 +6423,14 @@ TreeTransform<Derived>::TransformForStmt(ForStmt *S) {
 
   if (!getDerived().AlwaysRebuild() &&
       Init.get() == S->getInit() &&
-      Cond.get() == std::make_pair(S->getConditionVariable(), S->getCond()) &&
+      FullCond.get() == S->getCond() &&
       Inc.get() == S->getInc() &&
       Body.get() == S->getBody())
     return S;
 
   return getDerived().RebuildForStmt(S->getForLoc(), S->getLParenLoc(),
-                                     Init.get(), Cond, FullInc,
-                                     S->getRParenLoc(), Body.get());
+                                     Init.get(), FullCond, ConditionVar,
+                                     FullInc, S->getRParenLoc(), Body.get());
 }
 
 template<typename Derived>
@@ -6861,7 +6924,7 @@ TreeTransform<Derived>::TransformCXXForRangeStmt(CXXForRangeStmt *S) {
   if (Cond.isInvalid())
     return StmtError();
   if (Cond.get())
-    Cond = SemaRef.CheckBooleanCondition(S->getColonLoc(), Cond.get());
+    Cond = SemaRef.CheckBooleanCondition(Cond.get(), S->getColonLoc());
   if (Cond.isInvalid())
     return StmtError();
   if (Cond.get())
diff --git a/test/FixIt/fixit-vexing-parse.cpp b/test/FixIt/fixit-vexing-parse.cpp
index 71d3eff532..0232f5dbb0 100644
--- a/test/FixIt/fixit-vexing-parse.cpp
+++ b/test/FixIt/fixit-vexing-parse.cpp
@@ -60,7 +60,7 @@ namespace N {
     VO m(int (*p)[4]);
 
     // Don't emit warning and fixit because direct initializer is not permitted here.
-    if (int n(int())){} // expected-error {{function type is not allowed here}}
+    if (int n(int())){} // expected-error {{function type is not allowed here}} expected-error {{condition must have an initializer}}
 
     // CHECK: fix-it:"{{.*}}":{66:8-66:10}:" = {}"
     U u(); // expected-warning {{function declaration}} expected-note {{replace parentheses with an initializer}}
diff --git a/test/Parser/cxx0x-condition.cpp b/test/Parser/cxx0x-condition.cpp
index 071e09e415..8b64bcf127 100644
--- a/test/Parser/cxx0x-condition.cpp
+++ b/test/Parser/cxx0x-condition.cpp
@@ -23,9 +23,9 @@ void f() {
 
   if (S b(a)) {} // expected-error {{variable declaration in condition cannot have a parenthesized initializer}}
 
-  if (S b(n)) {} // expected-error {{a function type is not allowed here}}
+  if (S b(n)) {} // expected-error {{a function type is not allowed here}} expected-error {{must have an initializer}}
   if (S b(n) = 0) {} // expected-error {{a function type is not allowed here}}
-  if (S b(n) == 0) {} // expected-error {{a function type is not allowed here}}
+  if (S b(n) == 0) {} // expected-error {{a function type is not allowed here}} expected-error {{did you mean '='?}}
 
   S s(a);
   if (S{s}) {} // ok
diff --git a/test/SemaCXX/crashes.cpp b/test/SemaCXX/crashes.cpp
index a80587d840..926d13ab45 100644
--- a/test/SemaCXX/crashes.cpp
+++ b/test/SemaCXX/crashes.cpp
@@ -105,7 +105,8 @@ namespace PR9026 {
 namespace PR10270 {
   template<typename T> class C;
   template<typename T> void f() {
-    if (C<T> == 1) // expected-error{{expected unqualified-id}}
+    if (C<T> == 1) // expected-error{{expected unqualified-id}} \
+                   // expected-error{{invalid '==' at end of declaration}}
       return;
   }
 }
diff --git a/test/SemaCXX/for-range-examples.cpp b/test/SemaCXX/for-range-examples.cpp
index 08a9982c63..83023e3110 100644
--- a/test/SemaCXX/for-range-examples.cpp
+++ b/test/SemaCXX/for-range-examples.cpp
@@ -176,9 +176,9 @@ namespace test4 {
 
     // Make sure these don't crash. Better diagnostics would be nice.
     for (: {1, 2, 3}) {} // expected-error {{expected expression}} expected-error {{expected ';'}}
-    for (1 : {1, 2, 3}) {} // expected-error {{must declare a variable}}
+    for (1 : {1, 2, 3}) {} // expected-error {{must declare a variable}} expected-warning {{result unused}}
     for (+x : {1, 2, 3}) {} // expected-error {{undeclared identifier}} expected-error {{expected ';'}}
-    for (+y : {1, 2, 3}) {} // expected-error {{must declare a variable}}
+    for (+y : {1, 2, 3}) {} // expected-error {{must declare a variable}} expected-warning {{result unused}}
   }
 }
 
diff --git a/test/SemaObjCXX/foreach.mm b/test/SemaObjCXX/foreach.mm
index 99f5d0ce55..d1302c19a5 100644
--- a/test/SemaObjCXX/foreach.mm
+++ b/test/SemaObjCXX/foreach.mm
@@ -6,8 +6,10 @@
 void f(NSArray *a) {
     id keys;
     for (int i : a); // expected-error{{selector element type 'int' is not a valid object}} 
-    for ((id)2 : a);  // expected-error {{for range declaration must declare a variable}}
-    for (2 : a); // expected-error {{for range declaration must declare a variable}}
+    for ((id)2 : a);  // expected-error {{for range declaration must declare a variable}} \
+                      // expected-warning {{expression result unused}}
+    for (2 : a); // expected-error {{for range declaration must declare a variable}} \
+                 // expected-warning {{expression result unused}}
   
   for (id thisKey : keys);
 
@@ -63,7 +65,8 @@ int main ()
 @end
 void test2(NSObject<NSFastEnumeration> *collection) {
   Test2 *obj;
-  for (obj.prop : collection) { // expected-error {{for range declaration must declare a variable}}
+  for (obj.prop : collection) { // expected-error {{for range declaration must declare a variable}} \
+                                // expected-warning {{property access result unused - getters should not be used for side effects}}
   }
 }