Rearrange condition handling so that semantic checks on a condition variable

are performed before the other substatements of the construct are parsed,
rather than deferring them until the end. This allows better error recovery
from semantic errors in the condition, improves diagnostic order, and is a
prerequisite for C++17 constexpr if.

llvm-svn: 273548

clang/include/clang/Parse/Parser.h

@@ -1588,8 +1588,8 @@ private:
 
   //===--------------------------------------------------------------------===//
   // C++ if/switch/while condition expression.
-  bool ParseCXXCondition(ExprResult &ExprResult, Decl *&DeclResult,
+  Sema::ConditionResult ParseCXXCondition(SourceLocation Loc,
-                         SourceLocation Loc, bool ConvertToBoolean);
+                                          Sema::ConditionKind CK);
 
   //===--------------------------------------------------------------------===//
   // C++ Coroutines
@@ -1680,10 +1680,9 @@ private:
                                     unsigned ScopeFlags);
   void ParseCompoundStatementLeadingPragmas();
   StmtResult ParseCompoundStatementBody(bool isStmtExpr = false);
-  bool ParseParenExprOrCondition(ExprResult &ExprResult,
+  bool ParseParenExprOrCondition(Sema::ConditionResult &CondResult,
-                                 Decl *&DeclResult,
                                  SourceLocation Loc,
-                                 bool ConvertToBoolean);
+                                 Sema::ConditionKind CK);
   StmtResult ParseIfStatement(SourceLocation *TrailingElseLoc);
   StmtResult ParseSwitchStatement(SourceLocation *TrailingElseLoc);
   StmtResult ParseWhileStatement(SourceLocation *TrailingElseLoc);

clang/include/clang/Sema/Sema.h

@@ -3298,6 +3298,7 @@ public:
 public:
   class FullExprArg {
   public:
+    FullExprArg() : E(nullptr) { }
     FullExprArg(Sema &actions) : E(nullptr) { }
 
     ExprResult release() {
@@ -3391,27 +3392,23 @@ public:
                                  ArrayRef<const Attr*> Attrs,
                                  Stmt *SubStmt);
 
-  StmtResult ActOnIfStmt(SourceLocation IfLoc,
+  class ConditionResult;
-                         FullExprArg CondVal, Decl *CondVar,
+  StmtResult ActOnIfStmt(SourceLocation IfLoc, ConditionResult Cond,
-                         Stmt *ThenVal,
+                         Stmt *ThenVal, SourceLocation ElseLoc, Stmt *ElseVal);
-                         SourceLocation ElseLoc, Stmt *ElseVal);
   StmtResult ActOnStartOfSwitchStmt(SourceLocation SwitchLoc,
-                                            Expr *Cond,
+                                    ConditionResult Cond);
-                                            Decl *CondVar);
   StmtResult ActOnFinishSwitchStmt(SourceLocation SwitchLoc,
                                            Stmt *Switch, Stmt *Body);
-  StmtResult ActOnWhileStmt(SourceLocation WhileLoc,
+  StmtResult ActOnWhileStmt(SourceLocation WhileLoc, ConditionResult Cond,
-                            FullExprArg Cond,
+                            Stmt *Body);
-                            Decl *CondVar, Stmt *Body);
   StmtResult ActOnDoStmt(SourceLocation DoLoc, Stmt *Body,
-                                 SourceLocation WhileLoc,
+                         SourceLocation WhileLoc, SourceLocation CondLParen,
-                                 SourceLocation CondLParen, Expr *Cond,
+                         Expr *Cond, SourceLocation CondRParen);
-                                 SourceLocation CondRParen);
 
   StmtResult ActOnForStmt(SourceLocation ForLoc,
                           SourceLocation LParenLoc,
-                          Stmt *First, FullExprArg Second,
+                          Stmt *First,
-                          Decl *SecondVar,
+                          ConditionResult Second,
                           FullExprArg Third,
                           SourceLocation RParenLoc,
                           Stmt *Body);
@@ -4801,11 +4798,6 @@ 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,
@@ -8923,6 +8915,46 @@ 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
@@ -8931,10 +8963,7 @@ public:
   /// \param Loc - A location associated with the condition, e.g. the
   /// 'if' keyword.
   /// \return true iff there were any errors
-  ExprResult CheckBooleanCondition(Expr *E, SourceLocation Loc);
+  ExprResult CheckBooleanCondition(SourceLocation Loc, Expr *E);
-
-  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.

clang/lib/Parse/ParseDeclCXX.cpp

@@ -3420,10 +3420,11 @@ Parser::tryParseExceptionSpecification(bool Delayed,
     NoexceptExpr = ParseConstantExpression();
     T.consumeClose();
     // The argument must be contextually convertible to bool. We use
-    // ActOnBooleanCondition for this purpose.
+    // CheckBooleanCondition for this purpose.
+    // FIXME: Add a proper Sema entry point for this.
     if (!NoexceptExpr.isInvalid()) {
-      NoexceptExpr = Actions.ActOnBooleanCondition(getCurScope(), KeywordLoc,
+      NoexceptExpr =
-                                                   NoexceptExpr.get());
+          Actions.CheckBooleanCondition(KeywordLoc, NoexceptExpr.get());
       NoexceptRange = SourceRange(KeywordLoc, T.getCloseLocation());
     } else {
       NoexceptType = EST_None;

clang/lib/Parse/ParseExprCXX.cpp

@@ -1726,27 +1726,19 @@ 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 true if there was a parsing, false otherwise.
+/// \returns The parsed condition.
-bool Parser::ParseCXXCondition(ExprResult &ExprOut,
+Sema::ConditionResult Parser::ParseCXXCondition(SourceLocation Loc,
-                               Decl *&DeclOut,
+                                                Sema::ConditionKind CK) {
-                               SourceLocation Loc,
-                               bool ConvertToBoolean) {
   if (Tok.is(tok::code_completion)) {
     Actions.CodeCompleteOrdinaryName(getCurScope(), Sema::PCC_Condition);
     cutOffParsing();
-    return true;
+    return Sema::ConditionError();
   }
 
   ParsedAttributesWithRange attrs(AttrFactory);
@@ -1756,16 +1748,11 @@ bool Parser::ParseCXXCondition(ExprResult &ExprOut,
     ProhibitAttributes(attrs);
 
     // Parse the expression.
-    ExprOut = ParseExpression(); // expression
+    ExprResult Expr = ParseExpression(); // expression
-    DeclOut = nullptr;
+    if (Expr.isInvalid())
-    if (ExprOut.isInvalid())
+      return Sema::ConditionError();
-      return true;
 
-    // If required, convert to a boolean value.
+    return Actions.ActOnCondition(getCurScope(), Loc, Expr.get(), CK);
-    if (ConvertToBoolean)
-      ExprOut
-        = Actions.ActOnBooleanCondition(getCurScope(), Loc, ExprOut.get());
-    return ExprOut.isInvalid();
   }
 
   // type-specifier-seq
@@ -1783,7 +1770,7 @@ bool Parser::ParseCXXCondition(ExprResult &ExprOut,
     ExprResult AsmLabel(ParseSimpleAsm(&Loc));
     if (AsmLabel.isInvalid()) {
       SkipUntil(tok::semi, StopAtSemi);
-      return true;
+      return Sema::ConditionError();
     }
     DeclaratorInfo.setAsmLabel(AsmLabel.get());
     DeclaratorInfo.SetRangeEnd(Loc);
@@ -1795,8 +1782,9 @@ bool Parser::ParseCXXCondition(ExprResult &ExprOut,
   // Type-check the declaration itself.
   DeclResult Dcl = Actions.ActOnCXXConditionDeclaration(getCurScope(), 
                                                         DeclaratorInfo);
-  DeclOut = Dcl.get();
+  if (Dcl.isInvalid())
-  ExprOut = ExprError();
+    return Sema::ConditionError();
+  Decl *DeclOut = Dcl.get();
 
   // '=' assignment-expression
   // If a '==' or '+=' is found, suggest a fixit to '='.
@@ -1816,12 +1804,11 @@ bool Parser::ParseCXXCondition(ExprResult &ExprOut,
     SourceLocation LParen = ConsumeParen(), RParen = LParen;
     if (SkipUntil(tok::r_paren, StopAtSemi | StopBeforeMatch))
       RParen = ConsumeParen();
-    Diag(DeclOut ? DeclOut->getLocation() : LParen,
+    Diag(DeclOut->getLocation(),
          diag::err_expected_init_in_condition_lparen)
       << SourceRange(LParen, RParen);
   } else {
-    Diag(DeclOut ? DeclOut->getLocation() : Tok.getLocation(),
+    Diag(DeclOut->getLocation(), diag::err_expected_init_in_condition);
-         diag::err_expected_init_in_condition);
   }
 
   if (!InitExpr.isInvalid())
@@ -1834,8 +1821,7 @@ bool Parser::ParseCXXCondition(ExprResult &ExprOut,
   // (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.

clang/lib/Parse/ParseStmt.cpp

@@ -1052,29 +1052,28 @@ 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(ExprResult &ExprResult,
+bool Parser::ParseParenExprOrCondition(Sema::ConditionResult &Cond,
-                                       Decl *&DeclResult,
                                        SourceLocation Loc,
-                                       bool ConvertToBoolean) {
+                                       Sema::ConditionKind CK) {
   BalancedDelimiterTracker T(*this, tok::l_paren);
   T.consumeOpen();
 
   if (getLangOpts().CPlusPlus)
-    ParseCXXCondition(ExprResult, DeclResult, Loc, ConvertToBoolean);
+    Cond = ParseCXXCondition(Loc, CK);
   else {
-    ExprResult = ParseExpression();
+    ExprResult CondExpr = ParseExpression();
-    DeclResult = nullptr;
 
     // If required, convert to a boolean value.
-    if (!ExprResult.isInvalid() && ConvertToBoolean)
+    if (CondExpr.isInvalid())
-      ExprResult
+      Cond = Sema::ConditionError();
-        = Actions.ActOnBooleanCondition(getCurScope(), Loc, ExprResult.get());
+    else
+      Cond = Actions.ActOnCondition(getCurScope(), Loc, CondExpr.get(), CK);
   }
 
   // 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 (ExprResult.isInvalid() && !DeclResult && Tok.isNot(tok::r_paren)) {
+  if (Cond.isInvalid() && 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.
@@ -1132,13 +1131,10 @@ StmtResult Parser::ParseIfStatement(SourceLocation *TrailingElseLoc) {
   ParseScope IfScope(this, Scope::DeclScope | Scope::ControlScope, C99orCXX);
 
   // Parse the condition.
-  ExprResult CondExp;
+  Sema::ConditionResult Cond;
-  Decl *CondVar = nullptr;
+  if (ParseParenExprOrCondition(Cond, IfLoc, Sema::ConditionKind::Boolean))
-  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.
@@ -1221,8 +1217,8 @@ StmtResult Parser::ParseIfStatement(SourceLocation *TrailingElseLoc) {
   if (ElseStmt.isInvalid())
     ElseStmt = Actions.ActOnNullStmt(ElseStmtLoc);
 
-  return Actions.ActOnIfStmt(IfLoc, FullCondExp, CondVar, ThenStmt.get(),
+  return Actions.ActOnIfStmt(IfLoc, Cond, ThenStmt.get(), ElseLoc,
-                             ElseLoc, ElseStmt.get());
+                             ElseStmt.get());
 }
 
 /// ParseSwitchStatement
@@ -1259,13 +1255,11 @@ StmtResult Parser::ParseSwitchStatement(SourceLocation *TrailingElseLoc) {
   ParseScope SwitchScope(this, ScopeFlags);
 
   // Parse the condition.
-  ExprResult Cond;
+  Sema::ConditionResult Cond;
-  Decl *CondVar = nullptr;
+  if (ParseParenExprOrCondition(Cond, SwitchLoc, Sema::ConditionKind::Switch))
-  if (ParseParenExprOrCondition(Cond, CondVar, SwitchLoc, false))
     return StmtError();
 
-  StmtResult Switch
+  StmtResult Switch = Actions.ActOnStartOfSwitchStmt(SwitchLoc, Cond);
-    = Actions.ActOnStartOfSwitchStmt(SwitchLoc, Cond.get(), CondVar);
 
   if (Switch.isInvalid()) {
     // Skip the switch body.
@@ -1347,13 +1341,10 @@ StmtResult Parser::ParseWhileStatement(SourceLocation *TrailingElseLoc) {
   ParseScope WhileScope(this, ScopeFlags);
 
   // Parse the condition.
-  ExprResult Cond;
+  Sema::ConditionResult Cond;
-  Decl *CondVar = nullptr;
+  if (ParseParenExprOrCondition(Cond, WhileLoc, Sema::ConditionKind::Boolean))
-  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.
@@ -1374,10 +1365,10 @@ StmtResult Parser::ParseWhileStatement(SourceLocation *TrailingElseLoc) {
   InnerScope.Exit();
   WhileScope.Exit();
 
-  if ((Cond.isInvalid() && !CondVar) || Body.isInvalid())
+  if (Cond.isInvalid() || Body.isInvalid())
     return StmtError();
 
-  return Actions.ActOnWhileStmt(WhileLoc, FullCond, CondVar, Body.get());
+  return Actions.ActOnWhileStmt(WhileLoc, Cond, Body.get());
 }
 
 /// ParseDoStatement
@@ -1535,12 +1526,10 @@ StmtResult Parser::ParseForStatement(SourceLocation *TrailingElseLoc) {
 
   bool ForEach = false, ForRange = false;
   StmtResult FirstPart;
-  bool SecondPartIsInvalid = false;
+  Sema::ConditionResult SecondPart;
-  FullExprArg SecondPart(Actions);
   ExprResult Collection;
   ForRangeInit ForRangeInit;
   FullExprArg ThirdPart(Actions);
-  Decl *SecondVar = nullptr;
 
   if (Tok.is(tok::code_completion)) {
     Actions.CodeCompleteOrdinaryName(getCurScope(),
@@ -1645,7 +1634,7 @@ StmtResult Parser::ParseForStatement(SourceLocation *TrailingElseLoc) {
       Diag(Tok, diag::err_for_range_expected_decl)
         << FirstPart.get()->getSourceRange();
       SkipUntil(tok::r_paren, StopBeforeMatch);
-      SecondPartIsInvalid = true;
+      SecondPart = Sema::ConditionError();
     } else {
       if (!Value.isInvalid()) {
         Diag(Tok, diag::err_expected_semi_for);
@@ -1660,29 +1649,28 @@ StmtResult Parser::ParseForStatement(SourceLocation *TrailingElseLoc) {
 
   // Parse the second part of the for specifier.
   getCurScope()->AddFlags(Scope::BreakScope | Scope::ContinueScope);
-  if (!ForEach && !ForRange) {
+  if (!ForEach && !ForRange && !SecondPart.isInvalid()) {
-    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)
-        ParseCXXCondition(Second, SecondVar, ForLoc, true);
+        SecondPart = ParseCXXCondition(ForLoc, Sema::ConditionKind::Boolean);
       else {
-        Second = ParseExpression();
+        ExprResult SecondExpr = ParseExpression();
-        if (!Second.isInvalid())
+        if (SecondExpr.isInvalid())
-          Second = Actions.ActOnBooleanCondition(getCurScope(), ForLoc,
+          SecondPart = Sema::ConditionError();
-                                                 Second.get());
+        else
+          SecondPart =
+              Actions.ActOnCondition(getCurScope(), ForLoc, SecondExpr.get(),
+                                     Sema::ConditionKind::Boolean);
       }
-      SecondPartIsInvalid = Second.isInvalid();
-      SecondPart = Actions.MakeFullExpr(Second.get(), ForLoc);
     }
 
     if (Tok.isNot(tok::semi)) {
-      if (!SecondPartIsInvalid || SecondVar)
+      if (!SecondPart.isInvalid())
         Diag(Tok, diag::err_expected_semi_for);
       else
         // Skip until semicolon or rparen, don't consume it.
@@ -1781,8 +1769,8 @@ StmtResult Parser::ParseForStatement(SourceLocation *TrailingElseLoc) {
     return Actions.FinishCXXForRangeStmt(ForRangeStmt.get(), Body.get());
 
   return Actions.ActOnForStmt(ForLoc, T.getOpenLocation(), FirstPart.get(),
-                              SecondPart, SecondVar, ThirdPart,
+                              SecondPart, ThirdPart, T.getCloseLocation(),
-                              T.getCloseLocation(), Body.get());
+                              Body.get());
 }
 
 /// ParseGotoStatement

clang/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, 
+  return S.ActOnForStmt(
-                        S.MakeFullExpr(Comparison),
+      Loc, Loc, InitStmt,
-                        nullptr, S.MakeFullDiscardedValueExpr(Increment),
+      S.ActOnCondition(nullptr, Loc, Comparison, Sema::ConditionKind::Boolean),
-                        Loc, Copy.get());
+      S.MakeFullDiscardedValueExpr(Increment), Loc, Copy.get());
 }
 
 static StmtResult

clang/lib/Sema/SemaExpr.cpp

@@ -14341,7 +14341,7 @@ void Sema::DiagnoseEqualityWithExtraParens(ParenExpr *ParenE) {
     }
 }
 
-ExprResult Sema::CheckBooleanCondition(Expr *E, SourceLocation Loc) {
+ExprResult Sema::CheckBooleanCondition(SourceLocation Loc, Expr *E) {
   DiagnoseAssignmentAsCondition(E);
   if (ParenExpr *parenE = dyn_cast<ParenExpr>(E))
     DiagnoseEqualityWithExtraParens(parenE);
@@ -14371,12 +14371,26 @@ ExprResult Sema::CheckBooleanCondition(Expr *E, SourceLocation Loc) {
   return E;
 }
 
-ExprResult Sema::ActOnBooleanCondition(Scope *S, SourceLocation Loc,
+Sema::ConditionResult Sema::ActOnCondition(Scope *S, SourceLocation Loc,
-                                       Expr *SubExpr) {
+                                           Expr *SubExpr, ConditionKind CK) {
+  // Empty conditions are valid in for-statements.
   if (!SubExpr)
-    return ExprError();
+    return ConditionResult();
 
-  return CheckBooleanCondition(SubExpr, Loc);
+  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 ConditionResult(nullptr, MakeFullExpr(Cond.get(), Loc));
 }
 
 namespace {

clang/lib/Sema/SemaExprCXX.cpp

@@ -3054,11 +3054,21 @@ 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,
-                                        bool ConvertToBoolean) {
+                                        ConditionKind CK) {
   if (ConditionVar->isInvalidDecl())
     return ExprError();
 
@@ -3082,13 +3092,15 @@ ExprResult Sema::CheckConditionVariable(VarDecl *ConditionVar,
 
   MarkDeclRefReferenced(cast<DeclRefExpr>(Condition.get()));
 
-  if (ConvertToBoolean) {
+  switch (CK) {
-    Condition = CheckBooleanCondition(Condition.get(), StmtLoc);
+  case ConditionKind::Boolean:
-    if (Condition.isInvalid())
+    return CheckBooleanCondition(StmtLoc, Condition.get());
-      return ExprError();
+
+  case ConditionKind::Switch:
+    return CheckSwitchCondition(StmtLoc, Condition.get());
   }
 
-  return Condition;
+  llvm_unreachable("unexpected condition kind");
 }
 
 /// CheckCXXBooleanCondition - Returns true if a conversion to bool is invalid.

clang/lib/Sema/SemaOpenMP.cpp

@@ -6826,12 +6826,11 @@ OMPClause *Sema::ActOnOpenMPIfClause(OpenMPDirectiveKind NameModifier,
   if (!Condition->isValueDependent() && !Condition->isTypeDependent() &&
       !Condition->isInstantiationDependent() &&
       !Condition->containsUnexpandedParameterPack()) {
-    ExprResult Val = ActOnBooleanCondition(DSAStack->getCurScope(),
+    ExprResult Val = CheckBooleanCondition(StartLoc, Condition);
-                                           Condition->getExprLoc(), Condition);
     if (Val.isInvalid())
       return nullptr;
 
-    ValExpr = Val.get();
+    ValExpr = MakeFullExpr(Val.get()).get();
   }
 
   return new (Context) OMPIfClause(NameModifier, ValExpr, StartLoc, LParenLoc,
@@ -6846,12 +6845,11 @@ OMPClause *Sema::ActOnOpenMPFinalClause(Expr *Condition,
   if (!Condition->isValueDependent() && !Condition->isTypeDependent() &&
       !Condition->isInstantiationDependent() &&
       !Condition->containsUnexpandedParameterPack()) {
-    ExprResult Val = ActOnBooleanCondition(DSAStack->getCurScope(),
+    ExprResult Val = CheckBooleanCondition(StartLoc, Condition);
-                                           Condition->getExprLoc(), Condition);
     if (Val.isInvalid())
       return nullptr;
 
-    ValExpr = Val.get();
+    ValExpr = MakeFullExpr(Val.get()).get();
   }
 
   return new (Context) OMPFinalClause(ValExpr, StartLoc, LParenLoc, EndLoc);

clang/lib/Sema/SemaStmt.cpp

@@ -504,39 +504,30 @@ public:
 }
 
 StmtResult
-Sema::ActOnIfStmt(SourceLocation IfLoc, FullExprArg CondVal, Decl *CondVar,
+Sema::ActOnIfStmt(SourceLocation IfLoc, ConditionResult Cond,
                   Stmt *thenStmt, SourceLocation ElseLoc,
                   Stmt *elseStmt) {
-  ExprResult CondResult(CondVal.release());
+  auto CondVal = Cond.get();
-
+  if (Cond.isInvalid()) {
-  VarDecl *ConditionVar = nullptr;
+    CondVal.first = nullptr;
-  if (CondVar) {
+    CondVal.second = new (Context)
-    ConditionVar = cast<VarDecl>(CondVar);
+        OpaqueValueExpr(SourceLocation(), Context.BoolTy, VK_RValue);
-    CondResult = CheckConditionVariable(ConditionVar, IfLoc, true);
-    CondResult = ActOnFinishFullExpr(CondResult.get(), IfLoc);
   }
-  Expr *ConditionExpr = CondResult.getAs<Expr>();
-  if (ConditionExpr) {
 
   if (!Diags.isIgnored(diag::warn_comma_operator,
-                         ConditionExpr->getExprLoc()))
+                       CondVal.second->getExprLoc()))
-      CommaVisitor(*this).Visit(ConditionExpr);
+    CommaVisitor(*this).Visit(CondVal.second);
 
   DiagnoseUnusedExprResult(thenStmt);
 
   if (!elseStmt) {
-      DiagnoseEmptyStmtBody(ConditionExpr->getLocEnd(), thenStmt,
+    DiagnoseEmptyStmtBody(CondVal.second->getLocEnd(), thenStmt,
                           diag::warn_empty_if_body);
   }
 
   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, ConditionVar, ConditionExpr,
+  return new (Context) IfStmt(Context, IfLoc, CondVal.first, CondVal.second,
                               thenStmt, ElseLoc, elseStmt);
 }
 
@@ -599,24 +590,7 @@ static QualType GetTypeBeforeIntegralPromotion(Expr *&expr) {
   return expr->getType();
 }
 
-StmtResult
+ExprResult Sema::CheckSwitchCondition(SourceLocation SwitchLoc, Expr *Cond) {
-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;
 
@@ -664,24 +638,24 @@ Sema::ActOnStartOfSwitchStmt(SourceLocation SwitchLoc, Expr *Cond,
     }
   } SwitchDiagnoser(Cond);
 
-  CondResult =
+  ExprResult CondResult =
       PerformContextualImplicitConversion(SwitchLoc, Cond, SwitchDiagnoser);
-  if (CondResult.isInvalid()) return StmtError();
+  if (CondResult.isInvalid())
-  Cond = CondResult.get();
+    return ExprError();
 
   // C99 6.8.4.2p5 - Integer promotions are performed on the controlling expr.
-  CondResult = UsualUnaryConversions(Cond);
+  return UsualUnaryConversions(CondResult.get());
-  if (CondResult.isInvalid()) return StmtError();
+}
-  Cond = CondResult.get();
 
-  CondResult = ActOnFinishFullExpr(Cond, SwitchLoc);
+StmtResult
-  if (CondResult.isInvalid())
+Sema::ActOnStartOfSwitchStmt(SourceLocation SwitchLoc, ConditionResult Cond) {
+  if (Cond.isInvalid())
     return StmtError();
-  Cond = CondResult.get();
 
   getCurFunction()->setHasBranchIntoScope();
 
-  SwitchStmt *SS = new (Context) SwitchStmt(Context, ConditionVar, Cond);
+  SwitchStmt *SS =
+      new (Context) SwitchStmt(Context, Cond.get().first, Cond.get().second);
   getCurFunction()->SwitchStack.push_back(SS);
   return SS;
 }
@@ -1242,27 +1216,17 @@ Sema::DiagnoseAssignmentEnum(QualType DstType, QualType SrcType,
     }
 }
 
-StmtResult
+StmtResult Sema::ActOnWhileStmt(SourceLocation WhileLoc, ConditionResult Cond,
-Sema::ActOnWhileStmt(SourceLocation WhileLoc, FullExprArg Cond,
+                                Stmt *Body) {
-                     Decl *CondVar, Stmt *Body) {
+  if (Cond.isInvalid())
-  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);
 
-  if (ConditionExpr &&
+  auto CondVal = Cond.get();
-      !Diags.isIgnored(diag::warn_comma_operator, ConditionExpr->getExprLoc()))
+  CheckBreakContinueBinding(CondVal.second);
-    CommaVisitor(*this).Visit(ConditionExpr);
+
+  if (CondVal.second &&
+      !Diags.isIgnored(diag::warn_comma_operator, CondVal.second->getExprLoc()))
+    CommaVisitor(*this).Visit(CondVal.second);
 
   DiagnoseUnusedExprResult(Body);
 
@@ -1270,7 +1234,7 @@ Sema::ActOnWhileStmt(SourceLocation WhileLoc, FullExprArg Cond,
     getCurCompoundScope().setHasEmptyLoopBodies();
 
   return new (Context)
-      WhileStmt(Context, ConditionVar, ConditionExpr, Body, WhileLoc);
+      WhileStmt(Context, CondVal.first, CondVal.second, Body, WhileLoc);
 }
 
 StmtResult
@@ -1280,7 +1244,7 @@ Sema::ActOnDoStmt(SourceLocation DoLoc, Stmt *Body,
   assert(Cond && "ActOnDoStmt(): missing expression");
 
   CheckBreakContinueBinding(Cond);
-  ExprResult CondResult = CheckBooleanCondition(Cond, DoLoc);
+  ExprResult CondResult = CheckBooleanCondition(DoLoc, Cond);
   if (CondResult.isInvalid())
     return StmtError();
   Cond = CondResult.get();
@@ -1644,11 +1608,13 @@ void Sema::CheckBreakContinueBinding(Expr *E) {
   }
 }
 
-StmtResult
+StmtResult Sema::ActOnForStmt(SourceLocation ForLoc, SourceLocation LParenLoc,
-Sema::ActOnForStmt(SourceLocation ForLoc, SourceLocation LParenLoc,
+                              Stmt *First, ConditionResult Second,
-                   Stmt *First, FullExprArg second, Decl *secondVar,
+                              FullExprArg third, SourceLocation RParenLoc,
-                   FullExprArg third,
+                              Stmt *Body) {
-                   SourceLocation RParenLoc, Stmt *Body) {
+  if (Second.isInvalid())
+    return StmtError();
+
   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
@@ -1666,26 +1632,17 @@ Sema::ActOnForStmt(SourceLocation ForLoc, SourceLocation LParenLoc,
     }
   }
 
-  CheckBreakContinueBinding(second.get());
+  CheckBreakContinueBinding(Second.get().second);
   CheckBreakContinueBinding(third.get());
 
-  CheckForLoopConditionalStatement(*this, second.get(), third.get(), Body);
+  CheckForLoopConditionalStatement(*this, Second.get().second, third.get(),
+                                   Body);
   CheckForRedundantIteration(*this, third.get(), Body);
 
-  ExprResult SecondResult(second.release());
+  if (Second.get().second &&
-  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,
-                       SecondResult.get()->getExprLoc()))
+                       Second.get().second->getExprLoc()))
-    CommaVisitor(*this).Visit(SecondResult.get());
+    CommaVisitor(*this).Visit(Second.get().second);
 
   Expr *Third  = third.release().getAs<Expr>();
 
@@ -1696,8 +1653,9 @@ Sema::ActOnForStmt(SourceLocation ForLoc, SourceLocation LParenLoc,
   if (isa<NullStmt>(Body))
     getCurCompoundScope().setHasEmptyLoopBodies();
 
-  return new (Context) ForStmt(Context, First, SecondResult.get(), ConditionVar,
+  return new (Context)
-                               Third, Body, ForLoc, LParenLoc, RParenLoc);
+      ForStmt(Context, First, Second.get().second, Second.get().first, Third,
+              Body, ForLoc, LParenLoc, RParenLoc);
 }
 
 /// In an Objective C collection iteration statement:
@@ -2384,7 +2342,9 @@ Sema::BuildCXXForRangeStmt(SourceLocation ForLoc, SourceLocation CoawaitLoc,
     // Build and check __begin != __end expression.
     NotEqExpr = ActOnBinOp(S, ColonLoc, tok::exclaimequal,
                            BeginRef.get(), EndRef.get());
-    NotEqExpr = ActOnBooleanCondition(S, ColonLoc, NotEqExpr.get());
+    if (!NotEqExpr.isInvalid())
+      NotEqExpr = CheckBooleanCondition(ColonLoc, NotEqExpr.get());
+    if (!NotEqExpr.isInvalid())
       NotEqExpr = ActOnFinishFullExpr(NotEqExpr.get());
     if (NotEqExpr.isInvalid()) {
       Diag(RangeLoc, diag::note_for_range_invalid_iterator)

clang/lib/Sema/TreeTransform.h

@@ -410,6 +410,14 @@ 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.
   ///
@@ -1166,10 +1174,9 @@ 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::FullExprArg Cond,
+  StmtResult RebuildIfStmt(SourceLocation IfLoc, Sema::ConditionResult Cond,
-                           VarDecl *CondVar, Stmt *Then,
+                           Stmt *Then, SourceLocation ElseLoc, Stmt *Else) {
-                           SourceLocation ElseLoc, Stmt *Else) {
+    return getSema().ActOnIfStmt(IfLoc, Cond, Then, ElseLoc, Else);
-    return getSema().ActOnIfStmt(IfLoc, Cond, CondVar, Then, ElseLoc, Else);
   }
 
   /// \brief Start building a new switch statement.
@@ -1177,9 +1184,8 @@ public:
   /// By default, performs semantic analysis to build the new statement.
   /// Subclasses may override this routine to provide different behavior.
   StmtResult RebuildSwitchStmtStart(SourceLocation SwitchLoc,
-                                    Expr *Cond, VarDecl *CondVar) {
+                                    Sema::ConditionResult Cond) {
-    return getSema().ActOnStartOfSwitchStmt(SwitchLoc, Cond,
+    return getSema().ActOnStartOfSwitchStmt(SwitchLoc, Cond);
-                                            CondVar);
   }
 
   /// \brief Attach the body to the switch statement.
@@ -1195,9 +1201,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::FullExprArg Cond,
+  StmtResult RebuildWhileStmt(SourceLocation WhileLoc,
-                              VarDecl *CondVar, Stmt *Body) {
+                              Sema::ConditionResult Cond, Stmt *Body) {
-    return getSema().ActOnWhileStmt(WhileLoc, Cond, CondVar, Body);
+    return getSema().ActOnWhileStmt(WhileLoc, Cond, Body);
   }
 
   /// \brief Build a new do-while statement.
@@ -1216,11 +1222,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::FullExprArg Cond,
+                            Stmt *Init, Sema::ConditionResult Cond,
-                            VarDecl *CondVar, Sema::FullExprArg Inc,
+                            Sema::FullExprArg Inc, SourceLocation RParenLoc,
-                            SourceLocation RParenLoc, Stmt *Body) {
+                            Stmt *Body) {
     return getSema().ActOnForStmt(ForLoc, LParenLoc, Init, Cond,
-                                  CondVar, Inc, RParenLoc, Body);
+                                  Inc, RParenLoc, Body);
   }
 
   /// \brief Build a new goto statement.
@@ -3357,6 +3363,31 @@ 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(
@@ -4962,8 +4993,8 @@ bool TreeTransform<Derived>::TransformExceptionSpec(
     if (NoexceptExpr.isInvalid())
       return true;
 
-    NoexceptExpr = getSema().CheckBooleanCondition(
+    // FIXME: This is bogus, a noexcept expression is not a condition.
-        NoexceptExpr.get(), NoexceptExpr.get()->getLocStart());
+    NoexceptExpr = getSema().CheckBooleanCondition(Loc, NoexceptExpr.get());
     if (NoexceptExpr.isInvalid())
       return true;
 
@@ -6195,37 +6226,12 @@ template<typename Derived>
 StmtResult
 TreeTransform<Derived>::TransformIfStmt(IfStmt *S) {
   // Transform the condition
-  ExprResult Cond;
+  Sema::ConditionResult Cond = getDerived().TransformCondition(
-  VarDecl *ConditionVar = nullptr;
+      S->getIfLoc(), S->getConditionVariable(), S->getCond(),
-  if (S->getConditionVariable()) {
+      Sema::ConditionKind::Boolean);
-    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.
   StmtResult Then = getDerived().TransformStmt(S->getThen());
   if (Then.isInvalid())
@@ -6237,14 +6243,12 @@ TreeTransform<Derived>::TransformIfStmt(IfStmt *S) {
     return StmtError();
 
   if (!getDerived().AlwaysRebuild() &&
-      FullCond.get() == S->getCond() &&
+      Cond.get() == std::make_pair(S->getConditionVariable(), S->getCond()) &&
-      ConditionVar == S->getConditionVariable() &&
       Then.get() == S->getThen() &&
       Else.get() == S->getElse())
     return S;
 
-  return getDerived().RebuildIfStmt(S->getIfLoc(), FullCond, ConditionVar,
+  return getDerived().RebuildIfStmt(S->getIfLoc(), Cond, Then.get(),
-                                    Then.get(),
                                     S->getElseLoc(), Else.get());
 }
 
@@ -6252,27 +6256,15 @@ template<typename Derived>
 StmtResult
 TreeTransform<Derived>::TransformSwitchStmt(SwitchStmt *S) {
   // Transform the condition.
-  ExprResult Cond;
+  Sema::ConditionResult Cond = getDerived().TransformCondition(
-  VarDecl *ConditionVar = nullptr;
+      S->getSwitchLoc(), S->getConditionVariable(), S->getCond(),
-  if (S->getConditionVariable()) {
+      Sema::ConditionKind::Switch);
-    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.get(),
+    = getDerived().RebuildSwitchStmtStart(S->getSwitchLoc(), Cond);
-                                          ConditionVar);
   if (Switch.isInvalid())
     return StmtError();
 
@@ -6290,51 +6282,23 @@ template<typename Derived>
 StmtResult
 TreeTransform<Derived>::TransformWhileStmt(WhileStmt *S) {
   // Transform the condition
-  ExprResult Cond;
+  Sema::ConditionResult Cond = getDerived().TransformCondition(
-  VarDecl *ConditionVar = nullptr;
+      S->getWhileLoc(), S->getConditionVariable(), S->getCond(),
-  if (S->getConditionVariable()) {
+      Sema::ConditionKind::Boolean);
-    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
   StmtResult Body = getDerived().TransformStmt(S->getBody());
   if (Body.isInvalid())
     return StmtError();
 
   if (!getDerived().AlwaysRebuild() &&
-      FullCond.get() == S->getCond() &&
+      Cond.get() == std::make_pair(S->getConditionVariable(), S->getCond()) &&
-      ConditionVar == S->getConditionVariable() &&
       Body.get() == S->getBody())
     return Owned(S);
 
-  return getDerived().RebuildWhileStmt(S->getWhileLoc(), FullCond,
+  return getDerived().RebuildWhileStmt(S->getWhileLoc(), Cond, Body.get());
-                                       ConditionVar, Body.get());
 }
 
 template<typename Derived>
@@ -6374,39 +6338,12 @@ TreeTransform<Derived>::TransformForStmt(ForStmt *S) {
     getSema().ActOnOpenMPLoopInitialization(S->getForLoc(), Init.get());
 
   // Transform the condition
-  ExprResult Cond;
+  Sema::ConditionResult Cond = getDerived().TransformCondition(
-  VarDecl *ConditionVar = nullptr;
+      S->getForLoc(), S->getConditionVariable(), S->getCond(),
-  if (S->getConditionVariable()) {
+      Sema::ConditionKind::Boolean);
-    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
   ExprResult Inc = getDerived().TransformExpr(S->getInc());
   if (Inc.isInvalid())
@@ -6423,14 +6360,14 @@ TreeTransform<Derived>::TransformForStmt(ForStmt *S) {
 
   if (!getDerived().AlwaysRebuild() &&
       Init.get() == S->getInit() &&
-      FullCond.get() == S->getCond() &&
+      Cond.get() == std::make_pair(S->getConditionVariable(), S->getCond()) &&
       Inc.get() == S->getInc() &&
       Body.get() == S->getBody())
     return S;
 
   return getDerived().RebuildForStmt(S->getForLoc(), S->getLParenLoc(),
-                                     Init.get(), FullCond, ConditionVar,
+                                     Init.get(), Cond, FullInc,
-                                     FullInc, S->getRParenLoc(), Body.get());
+                                     S->getRParenLoc(), Body.get());
 }
 
 template<typename Derived>
@@ -6924,7 +6861,7 @@ TreeTransform<Derived>::TransformCXXForRangeStmt(CXXForRangeStmt *S) {
   if (Cond.isInvalid())
     return StmtError();
   if (Cond.get())
-    Cond = SemaRef.CheckBooleanCondition(Cond.get(), S->getColonLoc());
+    Cond = SemaRef.CheckBooleanCondition(S->getColonLoc(), Cond.get());
   if (Cond.isInvalid())
     return StmtError();
   if (Cond.get())

clang/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}} expected-error {{condition must have an initializer}}
+    if (int n(int())){} // expected-error {{function type is not allowed here}}
 
     // CHECK: fix-it:"{{.*}}":{66:8-66:10}:" = {}"
     U u(); // expected-warning {{function declaration}} expected-note {{replace parentheses with an initializer}}

clang/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}} expected-error {{must have an initializer}}
+  if (S b(n)) {} // 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 '='?}}
+  if (S b(n) == 0) {} // expected-error {{a function type is not allowed here}}
 
   S s(a);
   if (S{s}) {} // ok

clang/test/SemaCXX/crashes.cpp

@@ -105,8 +105,7 @@ 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;
   }
 }

clang/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}} expected-warning {{result unused}}
+    for (1 : {1, 2, 3}) {} // expected-error {{must declare a variable}}
     for (+x : {1, 2, 3}) {} // expected-error {{undeclared identifier}} expected-error {{expected ';'}}
-    for (+y : {1, 2, 3}) {} // expected-error {{must declare a variable}} expected-warning {{result unused}}
+    for (+y : {1, 2, 3}) {} // expected-error {{must declare a variable}}
   }
 }
 

clang/test/SemaObjCXX/foreach.mm

@@ -6,10 +6,8 @@
 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 ((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}}
-    for (2 : a); // expected-error {{for range declaration must declare a variable}} \
-                 // expected-warning {{expression result unused}}
   
   for (id thisKey : keys);
 
@@ -65,8 +63,7 @@ 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}}
   }
 }