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

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

llvm-svn: 273589

3 files changed, 78 insertions, 53 deletions

diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index 5718e1e50e9..d55b2d9442c 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/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/clang/lib/Parse/ParseExprCXX.cpp b/clang/lib/Parse/ParseExprCXX.cpp
index 00f20e5e7b7..b0740970d88 100644
--- a/clang/lib/Parse/ParseExprCXX.cpp
+++ b/clang/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/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp
index c849554238e..8173d09cbbe 100644
--- a/clang/lib/Parse/ParseStmt.cpp
+++ b/clang/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