Some parsing improvements

Author: Sirraide

clang/include/clang/Basic/DiagnosticParseKinds.td

@@ -457,6 +457,10 @@ def err_expansion_stmt_requires_range : Error<
   "expansion statement must be a range-based for loop">;
 def err_expansion_stmt_requires_cxx2c : Error<
   "expansion statements are only supported in C++2c">;
+def err_for_template : Error<
+  "'for template' is invalid; use 'template for' instead">;
+def err_expansion_stmt_invalid_kw : Error<
+  "'template %0' is invalid">;
 
 def err_expected_case_before_expression: Error<
   "expected 'case' keyword before expression">;

clang/include/clang/Parse/Parser.h

@@ -4189,7 +4189,7 @@ class Parser : public CodeCompletionHandler {
                            llvm::function_ref<void()> ExpressionStarts =
                                llvm::function_ref<void()>(),
                            bool FailImmediatelyOnInvalidExpr = false,
-                           bool StopAtRBraceAfterComma = false);
+                           bool ParsingExpansionInitList = false);
 
   /// ParseSimpleExpressionList - A simple comma-separated list of expressions,
   /// used for misc language extensions.
@@ -7531,7 +7531,8 @@ class Parser : public CodeCompletionHandler {
   ///       expansion-init-list
   /// \endverbatim
   StmtResult ParseExpansionStatement(SourceLocation *TrailingElseLoc,
-                                     LabelDecl *PrecedingLabel);
+                                     LabelDecl *PrecedingLabel,
+                                     SourceLocation TemplateLoc);
 
   StmtResult ParsePragmaLoopHint(StmtVector &Stmts, ParsedStmtContext StmtCtx,
                                  SourceLocation *TrailingElseLoc,

clang/lib/Parse/ParseExpr.cpp

@@ -3167,7 +3167,7 @@ void Parser::injectEmbedTokens() {
 bool Parser::ParseExpressionList(SmallVectorImpl<Expr *> &Exprs,
                                  llvm::function_ref<void()> ExpressionStarts,
                                  bool FailImmediatelyOnInvalidExpr,
-                                 bool StopAtRBraceAfterComma) {
+                                 bool ParsingExpansionInitList) {
   bool SawError = false;
   while (true) {
     if (ExpressionStarts)
@@ -3197,10 +3197,10 @@ bool Parser::ParseExpressionList(SmallVectorImpl<Expr *> &Exprs,
       if (FailImmediatelyOnInvalidExpr)
         break;
 
-      if (StopAtRBraceAfterComma)
-        SkipUntil(tok::comma, tok::r_brace, StopAtSemi | StopBeforeMatch);
-      else
-        SkipUntil(tok::comma, tok::r_paren, StopAtSemi | StopBeforeMatch);
+      // We expect '}' rather than ')' at the end of an expansion-init-list.
+      SkipUntil(tok::comma,
+                ParsingExpansionInitList ? tok::r_brace : tok::r_paren,
+                StopAtSemi | StopBeforeMatch);
     } else {
       Exprs.push_back(Expr.get());
     }
@@ -3211,7 +3211,8 @@ bool Parser::ParseExpressionList(SmallVectorImpl<Expr *> &Exprs,
     Token Comma = Tok;
     ConsumeToken();
 
-    if (StopAtRBraceAfterComma && Tok.is(tok::r_brace))
+    // CWG 3061: Trailing commas are allowed in expansion-init-lists.
+    if (ParsingExpansionInitList && Tok.is(tok::r_brace))
       break;
 
     checkPotentialAngleBracketDelimiter(Comma);

clang/lib/Parse/ParseInit.cpp

@@ -522,11 +522,10 @@ ExprResult Parser::ParseExpansionInitList() {
 
   ExprVector InitExprs;
 
-  // CWG 3061: Accept a trailing comma here.
   if (!Tok.is(tok::r_brace) &&
       ParseExpressionList(InitExprs, /*ExpressionStarts=*/{},
                           /*FailImmediatelyOnInvalidExpr=*/false,
-                          /*StopAtRBraceAfterComma=*/true)) {
+                          /*ParsingExpansionInitList=*/true)) {
     T.consumeClose();
     return ExprError();
   }

clang/lib/Parse/ParseStmt.cpp

@@ -271,7 +271,23 @@ StmtResult Parser::ParseStatementOrDeclarationAfterAttributes(
         return ParseForStatement(TrailingElseLoc, PrecedingLabel);
       }
 
-      return ParseExpansionStatement(TrailingElseLoc, PrecedingLabel);
+      SourceLocation TemplateLoc = ConsumeToken();
+      return ParseExpansionStatement(TrailingElseLoc, PrecedingLabel,
+                                     TemplateLoc);
+    }
+
+    // Since 'template for' is a thing, users might reasonably try to write
+    // 'template while' or something like that; diagnose that here for better
+    // QOI (and basically just ignore the 'template' token for error recovery).
+    if (NextToken().isOneOf(tok::kw_while, tok::kw_do)) {
+      bool IsWhile = NextToken().is(tok::kw_while);
+      Diag(Tok.getLocation(), diag::err_expansion_stmt_invalid_kw)
+          << (IsWhile ? "while" : "do");
+
+      ConsumeToken();
+      if (IsWhile)
+        return ParseWhileStatement(TrailingElseLoc, PrecedingLabel);
+      return ParseDoStatement(PrecedingLabel);
     }
 
     SourceLocation DeclEnd;
@@ -304,6 +320,19 @@ StmtResult Parser::ParseStatementOrDeclarationAfterAttributes(
     SemiError = "do/while";
     break;
   case tok::kw_for:                 // C99 6.8.5.3: for-statement
+    // Correct 'for template' to 'template for'.
+    if (NextToken().is(tok::kw_template)) {
+      Diag(Tok.getLocation(), diag::err_for_template)
+          << FixItHint::CreateReplacement(
+                 SourceRange(Tok.getLocation(), NextToken().getEndLoc()),
+                 "template for");
+      Tok.setKind(tok::kw_template);
+      SourceLocation TemplateLoc = ConsumeToken();
+      Tok.setKind(tok::kw_for);
+      return ParseExpansionStatement(TrailingElseLoc, PrecedingLabel,
+                                     TemplateLoc);
+    }
+
     return ParseForStatement(TrailingElseLoc, PrecedingLabel);
 
   case tok::kw_goto:                // C99 6.8.6.1: goto-statement
@@ -2706,9 +2735,9 @@ StmtResult Parser::ParseCXXCatchBlock(bool FnCatch) {
 }
 
 StmtResult Parser::ParseExpansionStatement(SourceLocation *TrailingElseLoc,
-                                           LabelDecl *PrecedingLabel) {
-  assert(Tok.is(tok::kw_template) && NextToken().is(tok::kw_for));
-  SourceLocation TemplateLoc = ConsumeToken();
+                                           LabelDecl *PrecedingLabel,
+                                           SourceLocation TemplateLoc) {
+  assert(Tok.is(tok::kw_for));
   DiagCompat(TemplateLoc, diag_compat::expansion_statements);
 
   CXXExpansionStmtDecl *ExpansionDecl =

clang/test/Parser/cxx2c-expansion-statements.cpp

@@ -40,7 +40,9 @@ void bad() {
   template for (3 : "error") // expected-error {{expansion statement declaration must declare a variable}} \
                                 expected-error {{expansion statement must be a range-based for loop}} expected-error {{TODO (expansion statements)}}
     ;
-  template while (true) {} // expected-error {{expected '<' after 'template'}}
+  template while (true) {} // expected-error {{'template while' is invalid}}
+  template do {} while (true); // expected-error {{'template do' is invalid}}
+  for template (int x : {}) {} // expected-error {{'for template' is invalid; use 'template for' instead}} expected-error {{TODO (expansion statements)}}
 }
 
 void good() {