[Parse] Use empty RecoveryExpr when if/while/do/switch conditions fail to parse

[sammccall]

Summary

This allows the body to be parsed.
An special-case that would replace a missing if condition with OpaqueValueExpr
was removed as it's now redundant (unless recovery-expr is disabled).

For loops are not handled at this point, as the parsing is more complicated.

[sammccall]

More conservative in changes to ActOnIfStmt

[hokein]

this looks great, some nits and questions.

clang/lib/Parse/ParseStmt.cpp

1194

what's the purpose of introducing MissingOK? It seems unclear to me, my understanding is

• before the patch, ActionOnCondition always returned a valid ConditionResult for an empty ConditionResult. I assume this was conceptually a "MissingOK-true" case.
• now in the patch, the MissingOK is propagated to ActOnCondition, which determines the returning result; ​The MissingOK is set to false in ParseSwitchStatement, ParseDoStatement below.

The only case I can think of is that we might fail to create a recoveryExpr (when the recovery-ast flag is off) for the condition.

1811

we should keep the Cond.isInvalid() branch, because the above typo-correction code path (Line1800) can hit it.

clang/lib/Sema/SemaExpr.cpp

19215

while here, we always return a valid condition (conceptually MissingOK is true), but in the patch, the default value of MissingOK parameter is false, this seems we are changing the default behavior for all call sides of ActOnCondition.

19217

nit: return MissingOK? ConditionResult() : ConditionError(), is easier to understand.

clang/test/AST/loop-recovery.cpp

2

file name: loop-recovery.cpp => ast-dump-loop-recovery.cpp

[sammccall]

clang/lib/Parse/ParseStmt.cpp

1194

Your understanding is right: previously we were returning a "valid but empty" ConditionResult for a while loop with no condition.

We want functions like ParseParenExprOrCondition to be able to recover in this case, without having them "recover" legitimately missing for loop conditions.

It would be possible to have ParseParenExprOrCondition conditionally recover based on MissingOK, but since we're already using the tristate ConditionResult, using its error state seems cleaner.

The only case I can think of is that we might fail to create a recoveryExpr (when the recovery-ast flag is off) for the condition.

I'm not sure what you're saying here - what is that a case of?
Is there something you'd like me to do here?

clang/lib/Sema/SemaExpr.cpp

19215

That's right, MissingOK=false is a better (safer) default.

I've checked all callsites: in most we're passing it explicitly, in one the default is correct (if statement), and in the remaining cases the condition is synthetic and should never be null.

clang/test/AST/loop-recovery.cpp

2

Why? This directory is all tests of the AST, usually that's tested by building and then dumping it.

An ast-dump- prefix makes sense if what we're testing is the AST dumping functionality (e.g. its formatting) rather than *using* that functionality to test the AST itself.

[sammccall]

address comments

[hokein]

since we're now preserving more invalid code, we should check whether const-evaluator is cable of handling these newly-added invalid case.

I have played around it, it seems that if, do/while, while cases are already handled well. switch case need some work:

• the ASTs among different cases (switch (;), switch(;;), switch(!!;)) are subtle
• the follow case will result an value-dependent violation, the fix would be to handle value-dependent condition in EvaluateSwitch (clang/lib/AST/ExprConstant.cpp)

constexpr int s() {
  switch(!!) {
  }
  return 0;
}
void a() {
  constexpr int k = s();
}

clang/lib/Parse/ParseExprCXX.cpp

1965–1966

nit: update the doc comment, though the comment is already stale (missing CK).

clang/lib/Parse/ParseStmt.cpp

1194

I'm not sure what you're saying here - what is that a case of?
Is there something you'd like me to do here?

sorry, no action needed.

clang/test/AST/loop-recovery.cpp

41

can you add an init-statement switch case? e.g. switch (;), switch(;;), switch(!!;)

clang/test/Parser/cxx0x-attributes.cpp

155

This looks like a bogus diagnostic, but I think it is ok, as this is a poor-recovery case for clang already -- IIUC, the do-while statement range claims to the } on Line56.

this is a case which can be improved by our bracket-matching repair :)

[sammccall]

In D113752#3188486, @hokein wrote:

since we're now preserving more invalid code, we should check whether const-evaluator is cable of handling these newly-added invalid case.

• the follow case will result an value-dependent violation, the fix would be to handle value-dependent condition in EvaluateSwitch (clang/lib/AST/ExprConstant.cpp)

Great catch, thanks!
I added constant-evaluation to ensure we don't crash for all the varieties of broken loops I could think of.
They're in SemaCXX/constexpr-function-recovery-crash.cpp which seems to be very similar. Maybe slightly misplaced as really we're testing AST/ExprConstant.

clang/test/Parser/cxx0x-attributes.cpp

155

Right. My understanding is the diagnostic is bogus if you put both missing )s before the semicolon, but clang is noticing them/implicitly inserting them before the }.

The code is horribly mangled here in any case, I'm not sure what diagnostic I'd want as a user really.

[sammccall]

Add constant-evaluation for broken switches, and a bunch of evaluation tests.
Address other comments.

[sammccall]

Anything left to do here?
I think it would be nice to get this into 14 release.

[hokein]

thanks, this looks good (sorry, I missed this.)

clang/include/clang/Parse/Parser.h

ExprResult ParseCXXDeleteExpression(bool UseGlobal,
                                             SourceLocation Start);
 
   //===--------------------------------------------------------------------===//
   // C++ if/switch/while/for condition expression.
   struct ForRangeInfo;
   Sema::ConditionResult ParseCXXCondition(StmtResult *InitStmt,
                                           SourceLocation Loc,
                                           Sema::ConditionKind CK,
+                                          bool MissingOK,
                                           ForRangeInfo *FRI = nullptr,
                                           bool EnterForConditionScope = false);
   DeclGroupPtrTy
   ParseAliasDeclarationInInitStatement(DeclaratorContext Context,
                                        ParsedAttributesWithRange &Attrs);
 
   //===--------------------------------------------------------------------===//
   // C++ Coroutines
private:
   StmtResult ParseCompoundStatement(bool isStmtExpr,
                                     unsigned ScopeFlags);
   void ParseCompoundStatementLeadingPragmas();
   bool ConsumeNullStmt(StmtVector &Stmts);
   StmtResult ParseCompoundStatementBody(bool isStmtExpr = false);
   bool ParseParenExprOrCondition(StmtResult *InitStmt,
                                  Sema::ConditionResult &CondResult,
                                  SourceLocation Loc, Sema::ConditionKind CK,
-                                 SourceLocation *LParenLoc = nullptr,
-                                 SourceLocation *RParenLoc = nullptr);
+                                 bool MissingOK, SourceLocation *LParenLoc,
+                                 SourceLocation *RParenLoc);
   StmtResult ParseIfStatement(SourceLocation *TrailingElseLoc);
   StmtResult ParseSwitchStatement(SourceLocation *TrailingElseLoc);
   StmtResult ParseWhileStatement(SourceLocation *TrailingElseLoc);
   StmtResult ParseDoStatement();
   StmtResult ParseForStatement(SourceLocation *TrailingElseLoc);
   StmtResult ParseGotoStatement();
   StmtResult ParseContinueStatement();
   StmtResult ParseBreakStatement();

clang/include/clang/Sema/Sema.h

public:
   };
   static ConditionResult ConditionError() { return ConditionResult(true); }
 
   enum class ConditionKind {
     Boolean,     ///< A boolean condition, from 'if', 'while', 'for', or 'do'.
     ConstexprIf, ///< A constant boolean condition from 'if constexpr'.
     Switch       ///< An integral condition for a 'switch' statement.
   };
+  QualType PreferredConditionType(ConditionKind K) const {
+    return K == ConditionKind::Switch ? Context.IntTy : Context.BoolTy;
+  }
 
-  ConditionResult ActOnCondition(Scope *S, SourceLocation Loc,
-                                 Expr *SubExpr, ConditionKind CK);
+  ConditionResult ActOnCondition(Scope *S, SourceLocation Loc, Expr *SubExpr,
+                                 ConditionKind CK, bool MissingOK = false);
 
   ConditionResult ActOnConditionVariable(Decl *ConditionVar,
                                          SourceLocation StmtLoc,
                                          ConditionKind CK);
 
   DeclResult ActOnCXXConditionDeclaration(Scope *S, Declarator &D);
 
   ExprResult CheckConditionVariable(VarDecl *ConditionVar,

clang/lib/AST/ExprConstant.cpp

if (const Stmt *Init = SS->getInit()) {
         return ESR;
       }
     }
 
     FullExpressionRAII CondScope(Info);
     if (SS->getConditionVariable() &&
         !EvaluateDecl(Info, SS->getConditionVariable()))
       return ESR_Failed;
+    if (SS->getCond()->isValueDependent()) {
+      if (!EvaluateDependentExpr(SS->getCond(), Info))
+        return ESR_Failed;
+    } else {
       if (!EvaluateInteger(SS->getCond(), Value, Info))
         return ESR_Failed;
+    }
     if (!CondScope.destroy())
       return ESR_Failed;
   }
 
   // Find the switch case corresponding to the value of the condition.
   // FIXME: Cache this lookup.
   const SwitchCase *Found = nullptr;
   for (const SwitchCase *SC = SS->getSwitchCaseList(); SC;

clang/lib/Parse/ParseExprCXX.cpp

 /// optional init-statement. This function will parse that too.
 ///
 /// \param InitStmt If non-null, an init-statement is permitted, and if present
 /// will be parsed and stored here.
 ///
 /// \param Loc The location of the start of the statement that requires this
 /// condition, e.g., the "for" in a for loop.
 ///
+/// \param MissingOK Whether an empty condition is acceptable here. Otherwise
+/// it is considered an error to be recovered from.
+///
 /// \param FRI If non-null, a for range declaration is permitted, and if
 /// present will be parsed and stored here, and a null result will be returned.
 ///
 /// \param EnterForConditionScope If true, enter a continue/break scope at the
 /// appropriate moment for a 'for' loop.
 ///
 /// \returns The parsed condition.
-Sema::ConditionResult Parser::ParseCXXCondition(StmtResult *InitStmt,
-                                                SourceLocation Loc,
-                                                Sema::ConditionKind CK,
-                                                ForRangeInfo *FRI,
-                                                bool EnterForConditionScope) {
+Sema::ConditionResult
+Parser::ParseCXXCondition(StmtResult *InitStmt, SourceLocation Loc,
+                          Sema::ConditionKind CK, bool MissingOK,
+                          ForRangeInfo *FRI, bool EnterForConditionScope) {
   // Helper to ensure we always enter a continue/break scope if requested.
   struct ForConditionScopeRAII {
     Scope *S;
     void enter(bool IsConditionVariable) {
       if (S) {
         S->AddFlags(Scope::BreakScope | Scope::ContinueScope);
         S->setIsConditionVarScope(IsConditionVariable);
       }
if (InitStmt && Tok.is(tok::semi)) {
       SourceLocation SemiLoc = Tok.getLocation();
       if (!Tok.hasLeadingEmptyMacro() && !SemiLoc.isMacroID()) {
         Diag(SemiLoc, diag::warn_empty_init_statement)
             << (CK == Sema::ConditionKind::Switch)
             << FixItHint::CreateRemoval(SemiLoc);
       }
       ConsumeToken();
       *InitStmt = Actions.ActOnNullStmt(SemiLoc);
-      return ParseCXXCondition(nullptr, Loc, CK);
+      return ParseCXXCondition(nullptr, Loc, CK, MissingOK);
     }
 
     // Parse the expression.
     ExprResult Expr = ParseExpression(); // expression
     if (Expr.isInvalid())
       return Sema::ConditionError();
 
     if (InitStmt && Tok.is(tok::semi)) {
       WarnOnInit();
       *InitStmt = Actions.ActOnExprStmt(Expr.get());
       ConsumeToken();
-      return ParseCXXCondition(nullptr, Loc, CK);
+      return ParseCXXCondition(nullptr, Loc, CK, MissingOK);
     }
 
-    return Actions.ActOnCondition(getCurScope(), Loc, Expr.get(), CK);
+    return Actions.ActOnCondition(getCurScope(), Loc, Expr.get(), CK,
+                                  MissingOK);
   }
 
   case ConditionOrInitStatement::InitStmtDecl: {
     WarnOnInit();
     DeclGroupPtrTy DG;
     SourceLocation DeclStart = Tok.getLocation(), DeclEnd;
     if (Tok.is(tok::kw_using))
       DG = ParseAliasDeclarationInInitStatement(
           DeclaratorContext::SelectionInit, attrs);
     else
       DG = ParseSimpleDeclaration(DeclaratorContext::SelectionInit, DeclEnd,
                                   attrs, /*RequireSemi=*/true);
     *InitStmt = Actions.ActOnDeclStmt(DG, DeclStart, DeclEnd);
-    return ParseCXXCondition(nullptr, Loc, CK);
+    return ParseCXXCondition(nullptr, Loc, CK, MissingOK);
   }
 
   case ConditionOrInitStatement::ForRangeDecl: {
     // This is 'for (init-stmt; for-range-decl : range-expr)'.
     // We're not actually in a for loop yet, so 'break' and 'continue' aren't
     // permitted here.
     assert(FRI && "should not parse a for range declaration here");
     SourceLocation DeclStart = Tok.getLocation(), DeclEnd;

clang/lib/Parse/ParseStmt.cpp

 /// 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.
 /// Additionally, if LParenLoc and RParenLoc are non-null, it will assign
 /// the location of the outer-most '(' and ')', respectively, to them.
 bool Parser::ParseParenExprOrCondition(StmtResult *InitStmt,
                                        Sema::ConditionResult &Cond,
                                        SourceLocation Loc,
-                                       Sema::ConditionKind CK,
+                                       Sema::ConditionKind CK, bool MissingOK,
                                        SourceLocation *LParenLoc,
                                        SourceLocation *RParenLoc) {
   BalancedDelimiterTracker T(*this, tok::l_paren);
   T.consumeOpen();
+  SourceLocation Start = Tok.getLocation();
 
-  if (getLangOpts().CPlusPlus)
-    Cond = ParseCXXCondition(InitStmt, Loc, CK);
-  else {
+  if (getLangOpts().CPlusPlus) {
+    Cond = ParseCXXCondition(InitStmt, Loc, CK, MissingOK);
+  } else {
     ExprResult CondExpr = ParseExpression();
 
     // If required, convert to a boolean value.
     if (CondExpr.isInvalid())
       Cond = Sema::ConditionError();
     else
-      Cond = Actions.ActOnCondition(getCurScope(), Loc, CondExpr.get(), CK);
+      Cond = Actions.ActOnCondition(getCurScope(), Loc, CondExpr.get(), CK,
+                                    MissingOK);
   }
 
   // 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)) {
     SkipUntil(tok::semi);
     // Skipping may have stopped if it found the containing ')'.  If so, we can
     // continue parsing the if statement.
     if (Tok.isNot(tok::r_paren))
       return true;
   }
 
-  // Otherwise the condition is valid or the rparen is present.
+  if (Cond.isInvalid()) {
+    ExprResult CondExpr = Actions.CreateRecoveryExpr(
+        Start, Tok.getLocation() == Start ? Start : PrevTokLocation, {},
+        Actions.PreferredConditionType(CK));
+    if (!CondExpr.isInvalid())
+      Cond = Actions.ActOnCondition(getCurScope(), Loc, CondExpr.get(), CK,
+                                    MissingOK);
+  }
+
+  // Either the condition is valid or the rparen is present.
   T.consumeClose();
 
   if (LParenLoc != nullptr) {
     *LParenLoc = T.getOpenLocation();
   }
   if (RParenLoc != nullptr) {
     *RParenLoc = T.getCloseLocation();
   }
StmtResult Parser::ParseIfStatement(SourceLocation *TrailingElseLoc) {
   SourceLocation LParen;
   SourceLocation RParen;
   llvm::Optional<bool> ConstexprCondition;
   if (!IsConsteval) {
 
     if (ParseParenExprOrCondition(&InitStmt, Cond, IfLoc,
                                   IsConstexpr ? Sema::ConditionKind::ConstexprIf
                                               : Sema::ConditionKind::Boolean,
-                                  &LParen, &RParen))
+                                  /*MissingOK=*/false, &LParen, &RParen))
       return StmtError();
 
     if (IsConstexpr)
       ConstexprCondition = Cond.getKnownValue();
   }
 
   bool IsBracedThen = Tok.is(tok::l_brace);
 
StmtResult Parser::ParseSwitchStatement(SourceLocation *TrailingElseLoc) {
   ParseScope SwitchScope(this, ScopeFlags);
 
   // Parse the condition.
   StmtResult InitStmt;
   Sema::ConditionResult Cond;
   SourceLocation LParen;
   SourceLocation RParen;
   if (ParseParenExprOrCondition(&InitStmt, Cond, SwitchLoc,
-                                Sema::ConditionKind::Switch, &LParen, &RParen))
+                                Sema::ConditionKind::Switch,
+                                /*MissingOK=*/false, &LParen, &RParen))
     return StmtError();
 
   StmtResult Switch = Actions.ActOnStartOfSwitchStmt(
       SwitchLoc, LParen, InitStmt.get(), Cond, RParen);
 
   if (Switch.isInvalid()) {
     // Skip the switch body.
     // FIXME: This is not optimal recovery, but parsing the body is more
else
     ScopeFlags = Scope::BreakScope | Scope::ContinueScope;
   ParseScope WhileScope(this, ScopeFlags);
 
   // Parse the condition.
   Sema::ConditionResult Cond;
   SourceLocation LParen;
   SourceLocation RParen;
   if (ParseParenExprOrCondition(nullptr, Cond, WhileLoc,
-                                Sema::ConditionKind::Boolean, &LParen, &RParen))
+                                Sema::ConditionKind::Boolean,
+                                /*MissingOK=*/false, &LParen, &RParen))
     return StmtError();
 
   // 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.
   //
   // C++ 6.5p2:
   // The substatement in an iteration-statement implicitly defines a local scope
StmtResult Parser::ParseDoStatement() {
 
   // Parse the parenthesized expression.
   BalancedDelimiterTracker T(*this, tok::l_paren);
   T.consumeOpen();
 
   // A do-while expression is not a condition, so can't have attributes.
   DiagnoseAndSkipCXX11Attributes();
 
+  SourceLocation Start = Tok.getLocation();
   ExprResult Cond = ParseExpression();
   // Correct the typos in condition before closing the scope.
   if (Cond.isUsable())
     Cond = Actions.CorrectDelayedTyposInExpr(Cond);
+  else {
+    if (!Tok.isOneOf(tok::r_paren, tok::r_square, tok::r_brace))
+      SkipUntil(tok::semi);
+    Cond = Actions.CreateRecoveryExpr(
+        Start, Start == Tok.getLocation() ? Start : PrevTokLocation, {},
+        Actions.getASTContext().BoolTy);
+  }
   T.consumeClose();
   DoScope.Exit();
 
   if (Cond.isInvalid() || Body.isInvalid())
     return StmtError();
 
   return Actions.ActOnDoStmt(DoLoc, Body.get(), WhileLoc, T.getOpenLocation(),
                              Cond.get(), T.getCloseLocation());
 }
 
 bool Parser::isForRangeIdentifier() {
   assert(Tok.is(tok::identifier));
if (!ForEach && !ForRangeInfo.ParsedForRangeDecl() &&
     } else if (Tok.is(tok::r_paren)) {
       // missing both semicolons.
     } else {
       if (getLangOpts().CPlusPlus) {
         // C++2a: We've parsed an init-statement; we might have a
         // for-range-declaration next.
         bool MightBeForRangeStmt = !ForRangeInfo.ParsedForRangeDecl();
         ColonProtectionRAIIObject ColonProtection(*this, MightBeForRangeStmt);
-        SecondPart =
-            ParseCXXCondition(nullptr, ForLoc, Sema::ConditionKind::Boolean,
-                              MightBeForRangeStmt ? &ForRangeInfo : nullptr,
-                              /*EnterForConditionScope*/ true);
+        SecondPart = ParseCXXCondition(
+            nullptr, ForLoc, Sema::ConditionKind::Boolean,
+            // FIXME: recovery if we don't see another semi!
+            /*MissingOK=*/true, MightBeForRangeStmt ? &ForRangeInfo : nullptr,
 
         if (ForRangeInfo.ParsedForRangeDecl()) {
           Diag(FirstPart.get() ? FirstPart.get()->getBeginLoc()
                                : ForRangeInfo.ColonLoc,
                getLangOpts().CPlusPlus20
                    ? diag::warn_cxx17_compat_for_range_init_stmt
                    : diag::ext_for_range_init_stmt)
               << (FirstPart.get() ? FirstPart.get()->getSourceRange()
                                   : SourceRange());
           if (EmptyInitStmtSemiLoc.isValid()) {
             Diag(EmptyInitStmtSemiLoc, diag::warn_empty_init_statement)
                 << /*for-loop*/ 2
                 << FixItHint::CreateRemoval(EmptyInitStmtSemiLoc);
           }
         }
       } else {
         // We permit 'continue' and 'break' in the condition of a for loop.
         getCurScope()->AddFlags(Scope::BreakScope | Scope::ContinueScope);
 
         ExprResult SecondExpr = ParseExpression();
         if (SecondExpr.isInvalid())
           SecondPart = Sema::ConditionError();
         else
-          SecondPart =
-              Actions.ActOnCondition(getCurScope(), ForLoc, SecondExpr.get(),
-                                     Sema::ConditionKind::Boolean);
+          SecondPart = Actions.ActOnCondition(
+              getCurScope(), ForLoc, SecondExpr.get(),
+              Sema::ConditionKind::Boolean, /*MissingOK=*/true);
       }
     }
   }
 
   // Enter a break / continue scope, if we didn't already enter one while
   // parsing the second part.
   if (!(getCurScope()->getFlags() & Scope::ContinueScope))
     getCurScope()->AddFlags(Scope::BreakScope | Scope::ContinueScope);

clang/lib/Sema/SemaExpr.cpp

if (!E->isTypeDependent()) {
     }
     CheckBoolLikeConversion(E, Loc);
   }
 
   return E;
 }
 
 Sema::ConditionResult Sema::ActOnCondition(Scope *S, SourceLocation Loc,
-                                           Expr *SubExpr, ConditionKind CK) {
-  // Empty conditions are valid in for-statements.
+                                           Expr *SubExpr, ConditionKind CK,
+                                           bool MissingOK) {
+  // MissingOK indicates whether having no condition expression is valid
+  // (for loop) or invalid (e.g. while loop).
   if (!SubExpr)
-    return ConditionResult();
+    return MissingOK ? ConditionResult() : ConditionError();
 
   ExprResult Cond;
   switch (CK) {
   case ConditionKind::Boolean:
     Cond = CheckBooleanCondition(Loc, SubExpr);
     break;
 
   case ConditionKind::ConstexprIf:
     Cond = CheckBooleanCondition(Loc, SubExpr, true);
     break;
 
   case ConditionKind::Switch:
     Cond = CheckSwitchCondition(Loc, SubExpr);
     break;
   }
   if (Cond.isInvalid()) {
     Cond = CreateRecoveryExpr(SubExpr->getBeginLoc(), SubExpr->getEndLoc(),
-                              {SubExpr});
+                              {SubExpr}, PreferredConditionType(CK));
     if (!Cond.get())
       return ConditionError();
   }
   // FIXME: FullExprArg doesn't have an invalid bit, so check nullness instead.
   FullExprArg FullExpr = MakeFullExpr(Cond.get(), Loc);
   if (!FullExpr.get())
     return ConditionError();
 

clang/lib/Sema/SemaStmt.cpp

 
 StmtResult Sema::ActOnIfStmt(SourceLocation IfLoc,
                              IfStatementKind StatementKind,
                              SourceLocation LParenLoc, Stmt *InitStmt,
                              ConditionResult Cond, SourceLocation RParenLoc,
                              Stmt *thenStmt, SourceLocation ElseLoc,
                              Stmt *elseStmt) {
   if (Cond.isInvalid())
-    Cond = ConditionResult(
-        *this, nullptr,
-        MakeFullExpr(new (Context) OpaqueValueExpr(SourceLocation(),
-                                                   Context.BoolTy, VK_PRValue),
-                     IfLoc),
-        false);
+    return StmtError();
 
   bool ConstevalOrNegatedConsteval =
       StatementKind == IfStatementKind::ConstevalNonNegated ||
       StatementKind == IfStatementKind::ConstevalNegated;
 
   Expr *CondExpr = Cond.get().second;
   assert((CondExpr || ConstevalOrNegatedConsteval) &&
          "If statement: missing condition");

 ///
 /// The body of the loop is not available yet, since it cannot be analysed until
 /// we have determined the type of the for-range-declaration.
 StmtResult Sema::ActOnCXXForRangeStmt(Scope *S, SourceLocation ForLoc,
                                       SourceLocation CoawaitLoc, Stmt *InitStmt,
                                       Stmt *First, SourceLocation ColonLoc,
                                       Expr *Range, SourceLocation RParenLoc,
                                       BuildForRangeKind Kind) {
+  // FIXME: recover in order to allow the body to be parsed.
   if (!First)
     return StmtError();
 
   if (Range && ObjCEnumerationCollection(Range)) {
     // FIXME: Support init-statements in Objective-C++20 ranged for statement.
     if (InitStmt)
       return Diag(InitStmt->getBeginLoc(), diag::err_objc_for_range_init_stmt)
                  << InitStmt->getSourceRange();

clang/lib/Sema/TreeTransform.h

Sema::ConditionResult TreeTransform<Derived>::TransformCondition(
   }
 
   if (Expr) {
     ExprResult CondExpr = getDerived().TransformExpr(Expr);
 
     if (CondExpr.isInvalid())
       return Sema::ConditionError();
 
-    return getSema().ActOnCondition(nullptr, Loc, CondExpr.get(), Kind);
+    return getSema().ActOnCondition(nullptr, Loc, CondExpr.get(), Kind,
+                                    /*MissingOK=*/true);
   }
 
   return Sema::ConditionResult();
 }
 
 template <typename Derived>
 NestedNameSpecifierLoc TreeTransform<Derived>::TransformNestedNameSpecifierLoc(
     NestedNameSpecifierLoc NNS, QualType ObjectType,

clang/test/AST/ast-dump-invalid.cpp

else
     return i;
 }
 }
 // CHECK: NamespaceDecl {{.*}} <{{.*}}> {{.*}} TestInvalidIf
 // CHECK-NEXT: `-FunctionDecl
 // CHECK-NEXT:   |-ParmVarDecl
 // CHECK-NEXT:   `-CompoundStmt
 // CHECK-NEXT:     `-IfStmt {{.*}} <line:25:3, line:28:12>
-// CHECK-NEXT:       |-OpaqueValueExpr {{.*}} <<invalid sloc>> 'bool'
+// CHECK-NEXT:       |-RecoveryExpr {{.*}} <line:25:7> 'bool'
 // CHECK-NEXT:       |-ReturnStmt {{.*}} <line:26:5, col:12>
 // CHECK-NEXT:       | `-IntegerLiteral {{.*}} <col:12> 'int' 4
 // CHECK-NEXT:       `-ReturnStmt {{.*}} <line:28:5, col:12>
 // CHECK-NEXT:         `-ImplicitCastExpr {{.*}} <col:12> 'int' <LValueToRValue>
 // CHECK-NEXT:           `-DeclRefExpr {{.*}} <col:12> 'int' lvalue ParmVar {{.*}} 'i' 'int'
 
 namespace TestInvalidFunctionDecl {
 struct Str {

clang/test/AST/loop-recovery.cpp

+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 %s
+// RUN: not %clang_cc1 -fsyntax-only -ast-dump %s -std=c++17 | FileCheck %s
+
+void test() {
+  while(!!!) // expected-error {{expected expression}}
+    int whileBody;
+  // CHECK: WhileStmt
+  // CHECK: RecoveryExpr {{.*}} <line:{{.*}}:9, col:11> 'bool'
+  // CHECK: whileBody 'int'
+
+  for(!!!) // expected-error {{expected expression}} expected-error {{expected ';'}}
+    int forBody;
+  // CHECK: ForStmt
+  // FIXME: the AST should have a RecoveryExpr to distinguish from for(;;)
+  // CHECK-NOT: RecoveryExpr
+  // CHECK: forBody 'int'
+
+  for(auto c : !!!) // expected-error {{expected expression}}
+    int forEachBody;
+  // FIXME: parse the foreach body
+  // CHECK-NOT: CXXForRangeStmt
+  // CHECK-NOT: forEachBody 'int'
+
+  do
+    int doBody;
+  while(!!!); // expected-error {{expected expression}}
+  // CHECK: DoStmt
+  // CHECK: doBody 'int'
+  // CHECK: RecoveryExpr {{.*}} <line:{{.*}}:9, col:11> 'bool'
+
+  if(!!!) // expected-error {{expected expression}}
+    int ifBody;
+  else
+    int elseBody;
+  // CHECK: IfStmt
+  // CHECK: RecoveryExpr {{.*}} <line:{{.*}}:6, col:8> 'bool'
+  // CHECK: ifBody 'int'
+  // CHECK: elseBody 'int'
+
+  switch(!!!) // expected-error {{expected expression}}
+    int switchBody;
+  // CHECK: SwitchStmt
+  // CHECK: RecoveryExpr {{.*}} <line:{{.*}}:10, col:12> 'int'
+  // CHECK: switchBody 'int'
+
+  switch (;) // expected-error {{expected expression}}
+    int switchBody;
+  // CHECK: SwitchStmt
+  // CHECK: NullStmt
+  // CHECK: RecoveryExpr {{.*}} <col:11> 'int'
+  // CHECK: switchBody 'int'
+
+  switch (;;) // expected-error {{expected expression}}
+    int switchBody;
+  // CHECK: SwitchStmt
+  // CHECK: NullStmt
+  // CHECK: RecoveryExpr {{.*}} <col:11, col:12> 'int'
+  // CHECK: switchBody 'int'
+
+  switch (!!!;) // expected-error {{expected expression}}
+    int switchBody;
+  // CHECK: SwitchStmt
+  // CHECK: RecoveryExpr {{.*}} <line:{{.*}}:11, col:14> 'int'
+  // CHECK: switchBody 'int'
+}

clang/test/Parser/cxx0x-attributes.cpp

void bad_attributes_in_do_while() {
   do {} while (
       [[a]b ns::i); // expected-error {{expected ']'}} \
                     // expected-note {{to match this '['}} \
                     // expected-error {{expected expression}}
   do {} while (
       [[ab]ab] ns::i); // expected-error {{an attribute list cannot appear here}}
   do {} while ( // expected-note {{to match this '('}}
       alignas(4 ns::i; // expected-note {{to match this '('}}
+                       // expected-error@-1 {{expected ';' after do/while}}
 } // expected-error 2{{expected ')'}} expected-error {{expected expression}}
 
 [[]] using T = int; // expected-error {{an attribute list cannot appear here}}
 using T [[]] = int; // ok
 template<typename T> using U [[]] = T;
 using ns::i [[]];
 using ns::i [[]], ns::i [[]]; // expected-warning {{use of multiple declarators in a single using declaration is a C++17 extension}}
 struct using_in_struct_base {

clang/test/Sema/complex-int.c

 
 result = arr*ii;
 result = ii*brr;
 
 result = arr*brr;
 result = xx*yy;
 
 switch (arr) { // expected-error{{statement requires expression of integer type ('_Complex int' invalid)}}
-  case brr: ;
-  case xx: ;
+  case brr: ; // expected-error{{integer constant expression must have integer type}}
+  case xx: ; // expected-error{{integer constant expression must have integer type}}
 }
 switch (ii) {
   case brr: ; // expected-error{{integer constant expression must have integer type}}
   case xx: ; // expected-error{{integer constant expression must have integer type}}
 }
 }
 
 void Tester() {

clang/test/SemaCXX/condition.cpp

void test() {
   do ; while (s); // expected-error {{value of type 'struct S' is not contextually convertible to 'bool'}}
   for (;s;) ; // expected-error {{value of type 'struct S' is not contextually convertible to 'bool'}}
   switch (s) {} // expected-error {{statement requires expression of integer type ('struct S' invalid)}}
 
   while (struct NewS *x=0) ;
   while (struct S {} *x=0) ; // expected-error {{'S' cannot be defined in a condition}}
   while (struct {} *x=0) ; // expected-error-re {{'(unnamed struct at {{.*}})' cannot be defined in a condition}}
   switch (enum {E} x=0) ; // expected-error-re {{'(unnamed enum at {{.*}})' cannot be defined in a condition}}
+                          // expected-warning@-1 {{switch statement has empty body}}
+                          // expected-note@-2 {{put the semicolon on a separate line}}
 
   if (int x=0) { // expected-note 2 {{previous definition is here}}
     int x;  // expected-error {{redefinition of 'x'}}
   }
   else
     int x;  // expected-error {{redefinition of 'x'}}
   while (int x=0) int x; // expected-error {{redefinition of 'x'}} expected-note {{previous definition is here}}
   while (int x=0) { int x; } // expected-error {{redefinition of 'x'}} expected-note {{previous definition is here}}

clang/test/SemaCXX/constexpr-function-recovery-crash.cpp

 
 struct X {} array[] = {undef()}; // expected-error {{use of undeclared identifier 'undef'}}
 constexpr void test11() {
   for (X& e : array) {}
 }
 
 constexpr int test12() { return "wrong"; } // expected-error {{cannot initialize return object of type 'int'}}
 constexpr int force12 = test12();          // expected-error {{must be initialized by a constant}}
+
+#define TEST_EVALUATE(Name, X)         \
+  constexpr int testEvaluate##Name() { \
+    X return 0;                        \
+  }                                    \
+  constexpr int forceEvaluate##Name = testEvaluate##Name()
+// Check that a variety of broken loops don't crash constant evaluation.
+// We're not checking specific recovery here so don't assert diagnostics.
+TEST_EVALUATE(Switch, switch (!!){});              // expected-error + {{}}
+TEST_EVALUATE(SwitchInit, switch (auto x = !!){}); // expected-error + {{}}
+TEST_EVALUATE(For, for (!!){}); // expected-error + {{}}
+                                // FIXME: should bail out instead of looping.
+                                // expected-note@-2 + {{infinite loop}}
+                                // expected-note@-3 {{in call}}
+TEST_EVALUATE(ForRange, for (auto x : !!){}); // expected-error + {{}}
+TEST_EVALUATE(While, while (!!){});           // expected-error + {{}}
+TEST_EVALUATE(DoWhile, do {} while (!!););    // expected-error + {{}}
+TEST_EVALUATE(If, if (!!){};);                // expected-error + {{}}
+TEST_EVALUATE(IfInit, if (auto x = !!; 1){};);// expected-error + {{}}
+TEST_EVALUATE(ForInit, if (!!;;){};);         // expected-error + {{}}
+TEST_EVALUATE(ForCond, if (; !!;){};);        // expected-error + {{}}
+TEST_EVALUATE(ForInc, if (;; !!){};);         // expected-error + {{}}