Refactor and micro-optimize ConsumeToken()

1) Introduce TryConsumeToken() to handle the common test-and-consume pattern.
   This brings about readability improvements in the parser and optimizes to avoid
   redundant checks in the common case.

2) Eliminate the ConsumeCodeCompletionTok special case from ConsumeToken(). This
   was used by only one caller which has been switched over to the more
   appropriate ConsumeCodeCompletionToken() function.

llvm-svn: 197497

clang/include/clang/Parse/Parser.h

@@ -276,12 +276,11 @@ public:
   /// This does not work with all kinds of tokens: strings and specific other
   /// tokens must be consumed with custom methods below.  This returns the
   /// location of the consumed token.
-  SourceLocation ConsumeToken(bool ConsumeCodeCompletionTok = false) {
+  SourceLocation ConsumeToken() {
-    assert(!isTokenStringLiteral() && !isTokenParen() && !isTokenBracket() &&
+    assert(!isTokenSpecial() &&
-           !isTokenBrace() &&
            "Should consume special tokens with Consume*Token");
 
-    if (!ConsumeCodeCompletionTok && Tok.is(tok::code_completion))
+    if (LLVM_UNLIKELY(Tok.is(tok::code_completion)))
       return handleUnexpectedCodeCompletionToken();
 
     PrevTokLocation = Tok.getLocation();
@@ -289,6 +288,23 @@ public:
     return PrevTokLocation;
   }
 
+  bool TryConsumeToken(tok::TokenKind Expected) {
+    if (Tok.isNot(Expected))
+      return false;
+    assert(!isTokenSpecial() &&
+           "Should consume special tokens with Consume*Token");
+    PrevTokLocation = Tok.getLocation();
+    PP.Lex(Tok);
+    return true;
+  }
+
+  bool TryConsumeToken(tok::TokenKind Expected, SourceLocation &Loc) {
+    if (!TryConsumeToken(Expected))
+      return false;
+    Loc = PrevTokLocation;
+    return true;
+  }
+
 private:
   //===--------------------------------------------------------------------===//
   // Low-Level token peeking and consumption methods.
@@ -306,12 +322,15 @@ private:
   bool isTokenBrace() const {
     return Tok.getKind() == tok::l_brace || Tok.getKind() == tok::r_brace;
   }
-
   /// isTokenStringLiteral - True if this token is a string-literal.
-  ///
   bool isTokenStringLiteral() const {
     return tok::isStringLiteral(Tok.getKind());
   }
+  /// isTokenSpecial - True if this token requires special consumption methods.
+  bool isTokenSpecial() const {
+    return isTokenStringLiteral() || isTokenParen() || isTokenBracket() ||
+           isTokenBrace();
+  }
 
   /// \brief Returns true if the current token is '=' or is a type of '='.
   /// For typos, give a fixit to '='
@@ -329,8 +348,10 @@ private:
       return ConsumeBrace();
     else if (isTokenStringLiteral())
       return ConsumeStringToken();
+    else if (ConsumeCodeCompletionTok && Tok.is(tok::code_completion))
+      return ConsumeCodeCompletionToken();
     else
-      return ConsumeToken(ConsumeCodeCompletionTok);
+      return ConsumeToken();
   }
 
   /// ConsumeParen - This consume method keeps the paren count up-to-date.

clang/lib/Parse/ParseCXXInlineMethods.cpp

@@ -69,9 +69,7 @@ NamedDecl *Parser::ParseCXXInlineMethodDef(AccessSpecifier AS,
 
   D.complete(FnD);
 
-  if (Tok.is(tok::equal)) {
+  if (TryConsumeToken(tok::equal)) {
-    ConsumeToken();
-
     if (!FnD) {
       SkipUntil(tok::semi);
       return 0;
@@ -79,20 +77,16 @@ NamedDecl *Parser::ParseCXXInlineMethodDef(AccessSpecifier AS,
 
     bool Delete = false;
     SourceLocation KWLoc;
-    if (Tok.is(tok::kw_delete)) {
+    if (TryConsumeToken(tok::kw_delete, KWLoc)) {
-      Diag(Tok, getLangOpts().CPlusPlus11 ?
+      Diag(KWLoc, getLangOpts().CPlusPlus11
-           diag::warn_cxx98_compat_deleted_function :
+                      ? diag::warn_cxx98_compat_deleted_function
-           diag::ext_deleted_function);
+                      : diag::ext_deleted_function);
-
-      KWLoc = ConsumeToken();
       Actions.SetDeclDeleted(FnD, KWLoc);
       Delete = true;
-    } else if (Tok.is(tok::kw_default)) {
+    } else if (TryConsumeToken(tok::kw_default, KWLoc)) {
-      Diag(Tok, getLangOpts().CPlusPlus11 ?
+      Diag(KWLoc, getLangOpts().CPlusPlus11
-           diag::warn_cxx98_compat_defaulted_function :
+                      ? diag::warn_cxx98_compat_defaulted_function
-           diag::ext_defaulted_function);
+                      : diag::ext_defaulted_function);
-
-      KWLoc = ConsumeToken();
       Actions.SetDeclDefaulted(FnD, KWLoc);
     } else {
       llvm_unreachable("function definition after = not 'delete' or 'default'");
@@ -348,9 +342,7 @@ void Parser::ParseLexedMethodDeclaration(LateParsedMethodDeclaration &LM) {
       if (DefArgResult.isInvalid())
         Actions.ActOnParamDefaultArgumentError(LM.DefaultArgs[I].Param);
       else {
-        if (Tok.is(tok::cxx_defaultarg_end))
+        if (!TryConsumeToken(tok::cxx_defaultarg_end)) {
-          ConsumeToken();
-        else {
           // The last two tokens are the terminator and the saved value of
           // Tok; the last token in the default argument is the one before
           // those.

clang/lib/Parse/ParseDecl.cpp

@@ -3535,18 +3535,19 @@ void Parser::ParseStructUnionBody(SourceLocation RecordLoc,
       ExpectAndConsume(tok::r_paren, diag::err_expected_rparen);
     }
 
-    if (Tok.is(tok::semi)) {
+    if (TryConsumeToken(tok::semi))
-      ConsumeToken();
+      continue;
-    } else if (Tok.is(tok::r_brace)) {
+
+    if (Tok.is(tok::r_brace)) {
       ExpectAndConsume(tok::semi, diag::ext_expected_semi_decl_list);
       break;
-    } else {
-      ExpectAndConsume(tok::semi, diag::err_expected_semi_decl_list);
-      // Skip to end of block or statement to avoid ext-warning on extra ';'.
-      SkipUntil(tok::r_brace, StopAtSemi | StopBeforeMatch);
-      // If we stopped at a ';', eat it.
-      if (Tok.is(tok::semi)) ConsumeToken();
     }
+
+    ExpectAndConsume(tok::semi, diag::err_expected_semi_decl_list);
+    // Skip to end of block or statement to avoid ext-warning on extra ';'.
+    SkipUntil(tok::r_brace, StopAtSemi | StopBeforeMatch);
+    // If we stopped at a ';', eat it.
+    TryConsumeToken(tok::semi);
   }
 
   T.consumeClose();
@@ -3954,8 +3955,7 @@ void Parser::ParseEnumBody(SourceLocation StartLoc, Decl *EnumDecl) {
     ExprResult AssignedVal;
     ParsingDeclRAIIObject PD(*this, ParsingDeclRAIIObject::NoParent);
 
-    if (Tok.is(tok::equal)) {
+    if (TryConsumeToken(tok::equal, EqualLoc)) {
-      EqualLoc = ConsumeToken();
       AssignedVal = ParseConstantExpression();
       if (AssignedVal.isInvalid())
         SkipUntil(tok::comma, tok::r_brace, StopAtSemi | StopBeforeMatch);
@@ -3980,9 +3980,9 @@ void Parser::ParseEnumBody(SourceLocation StartLoc, Decl *EnumDecl) {
       continue;
     }
 
-    if (Tok.isNot(tok::comma))
+    SourceLocation CommaLoc;
+    if (!TryConsumeToken(tok::comma, CommaLoc))
       break;
-    SourceLocation CommaLoc = ConsumeToken();
 
     if (Tok.isNot(tok::identifier)) {
       if (!getLangOpts().C99 && !getLangOpts().CPlusPlus11)
@@ -5409,7 +5409,7 @@ void Parser::ParseFunctionDeclaratorIdentifierList(
   // Maintain an efficient lookup of params we have seen so far.
   llvm::SmallSet<const IdentifierInfo*, 16> ParamsSoFar;
 
-  while (1) {
+  do {
     // If this isn't an identifier, report the error and skip until ')'.
     if (Tok.isNot(tok::identifier)) {
       Diag(Tok, diag::err_expected_ident);
@@ -5437,12 +5437,8 @@ void Parser::ParseFunctionDeclaratorIdentifierList(
 
     // Eat the identifier.
     ConsumeToken();
-
     // The list continues if we see a comma.
-    if (Tok.isNot(tok::comma))
+  } while (TryConsumeToken(tok::comma));
-      break;
-    ConsumeToken();
-  }
 }
 
 /// ParseParameterDeclarationClause - Parse a (possibly empty) parameter-list
@@ -5481,13 +5477,11 @@ void Parser::ParseParameterDeclarationClause(
        ParsedAttributes &FirstArgAttrs,
        SmallVectorImpl<DeclaratorChunk::ParamInfo> &ParamInfo,
        SourceLocation &EllipsisLoc) {
-  while (1) {
+  do {
-    if (Tok.is(tok::ellipsis)) {
+    // FIXME: Issue a diagnostic if we parsed an attribute-specifier-seq
-      // FIXME: Issue a diagnostic if we parsed an attribute-specifier-seq
+    // before deciding this was a parameter-declaration-clause.
-      // before deciding this was a parameter-declaration-clause.
+    if (TryConsumeToken(tok::ellipsis, EllipsisLoc))
-      EllipsisLoc = ConsumeToken();     // Consume the ellipsis.
       break;
-    }
 
     // Parse the declaration-specifiers.
     // Just use the ParsingDeclaration "scope" of the declarator.
@@ -5606,26 +5600,17 @@ void Parser::ParseParameterDeclarationClause(
                                           Param, DefArgToks));
     }
 
-    // If the next token is a comma, consume it and keep reading arguments.
+    if (TryConsumeToken(tok::ellipsis, EllipsisLoc) &&
-    if (Tok.isNot(tok::comma)) {
+        !getLangOpts().CPlusPlus) {
-      if (Tok.is(tok::ellipsis)) {
+      // We have ellipsis without a preceding ',', which is ill-formed
-        EllipsisLoc = ConsumeToken();     // Consume the ellipsis.
+      // in C. Complain and provide the fix.
-
+      Diag(EllipsisLoc, diag::err_missing_comma_before_ellipsis)
-        if (!getLangOpts().CPlusPlus) {
+          << FixItHint::CreateInsertion(EllipsisLoc, ", ");
-          // We have ellipsis without a preceding ',', which is ill-formed
-          // in C. Complain and provide the fix.
-          Diag(EllipsisLoc, diag::err_missing_comma_before_ellipsis)
-            << FixItHint::CreateInsertion(EllipsisLoc, ", ");
-        }
-      }
-
       break;
     }
 
-    // Consume the comma.
+    // If the next token is a comma, consume it and keep reading arguments.
-    ConsumeToken();
+  } while (TryConsumeToken(tok::comma));
-  }
-
 }
 
 /// [C90]   direct-declarator '[' constant-expression[opt] ']'
@@ -5676,8 +5661,7 @@ void Parser::ParseBracketDeclarator(Declarator &D) {
 
   // If valid, this location is the position where we read the 'static' keyword.
   SourceLocation StaticLoc;
-  if (Tok.is(tok::kw_static))
+  TryConsumeToken(tok::kw_static, StaticLoc);
-    StaticLoc = ConsumeToken();
 
   // If there is a type-qualifier-list, read it now.
   // Type qualifiers in an array subscript are a C99 feature.
@@ -5686,8 +5670,8 @@ void Parser::ParseBracketDeclarator(Declarator &D) {
 
   // If we haven't already read 'static', check to see if there is one after the
   // type-qualifier-list.
-  if (!StaticLoc.isValid() && Tok.is(tok::kw_static))
+  if (!StaticLoc.isValid())
-    StaticLoc = ConsumeToken();
+    TryConsumeToken(tok::kw_static, StaticLoc);
 
   // Handle "direct-declarator [ type-qual-list[opt] * ]".
   bool isStar = false;

clang/lib/Parse/ParseExpr.cpp

@@ -269,10 +269,7 @@ Parser::ParseRHSOfBinaryExpression(ExprResult LHS, prec::Level MinPrec) {
         Diag(Tok, diag::ext_gnu_conditional_expr);
       }
 
-      if (Tok.is(tok::colon)) {
+      if (!TryConsumeToken(tok::colon, ColonLoc)) {
-        // Eat the colon.
-        ColonLoc = ConsumeToken();
-      } else {
         // Otherwise, we're missing a ':'.  Assume that this was a typo that
         // the user forgot. If we're not in a macro expansion, we can suggest
         // a fixit hint. If there were two spaces before the current token,
@@ -1294,9 +1291,8 @@ Parser::ParsePostfixExpressionSuffix(ExprResult LHS) {
           LHS = ExprError();
         }
 
-        SourceLocation CloseLoc = Tok.getLocation();
+        SourceLocation CloseLoc;
-        if (Tok.is(tok::greatergreatergreater)) {
+        if (TryConsumeToken(tok::greatergreatergreater, CloseLoc)) {
-          ConsumeToken();
         } else if (LHS.isInvalid()) {
           SkipUntil(tok::greatergreatergreater, StopAtSemi);
         } else {
@@ -2229,7 +2225,7 @@ ExprResult Parser::ParseGenericSelectionExpression() {
   SourceLocation DefaultLoc;
   TypeVector Types;
   ExprVector Exprs;
-  while (1) {
+  do {
     ParsedType Ty;
     if (Tok.is(tok::kw_default)) {
       // C11 6.5.1.1p2 "A generic selection shall have no more than one default
@@ -2266,11 +2262,7 @@ ExprResult Parser::ParseGenericSelectionExpression() {
       return ExprError();
     }
     Exprs.push_back(ER.release());
-
+  } while (TryConsumeToken(tok::comma));
-    if (Tok.isNot(tok::comma))
-      break;
-    ConsumeToken();
-  }
 
   T.consumeClose();
   if (T.getCloseLocation().isInvalid())

clang/lib/Parse/ParseExprCXX.cpp

@@ -233,12 +233,13 @@ bool Parser::ParseOptionalCXXScopeSpecifier(CXXScopeSpec &SS,
     DeclSpec DS(AttrFactory);
     SourceLocation DeclLoc = Tok.getLocation();
     SourceLocation EndLoc  = ParseDecltypeSpecifier(DS);
-    if (Tok.isNot(tok::coloncolon)) {
+
+    SourceLocation CCLoc;
+    if (!TryConsumeToken(tok::coloncolon, CCLoc)) {
       AnnotateExistingDecltypeSpecifier(DS, DeclLoc, EndLoc);
       return false;
     }
-    
+
-    SourceLocation CCLoc = ConsumeToken();
     if (Actions.ActOnCXXNestedNameSpecifierDecltype(SS, DS, CCLoc))
       SS.SetInvalid(SourceRange(DeclLoc, CCLoc));
 
@@ -818,8 +819,7 @@ Optional<unsigned> Parser::ParseLambdaIntroducer(LambdaIntroducer &Intro,
         // to save the necessary state, and restore it later.
         EnterExpressionEvaluationContext EC(Actions,
                                             Sema::PotentiallyEvaluated);
-        if (Tok.is(tok::equal))
+        TryConsumeToken(tok::equal);
-          ConsumeToken();
 
         if (!SkippedInits)
           Init = ParseInitializer();
@@ -868,8 +868,8 @@ Optional<unsigned> Parser::ParseLambdaIntroducer(LambdaIntroducer &Intro,
             ConsumeToken();
           }
         }
-      } else if (Tok.is(tok::ellipsis))
+      } else
-        EllipsisLoc = ConsumeToken();
+        TryConsumeToken(tok::ellipsis, EllipsisLoc);
     }
     // If this is an init capture, process the initialization expression
     // right away.  For lambda init-captures such as the following:
@@ -2770,15 +2770,8 @@ ExprResult Parser::ParseTypeTrait() {
     
     // Add this type to the list of arguments.
     Args.push_back(Ty.get());
-    
+  } while (TryConsumeToken(tok::comma));
-    if (Tok.is(tok::comma)) {
+
-      ConsumeToken();
-      continue;
-    }
-    
-    break;
-  } while (true);
-  
   if (Parens.consumeClose())
     return ExprError();
 

clang/lib/Parse/ParseTemplate.cpp

@@ -101,15 +101,11 @@ Parser::ParseTemplateDeclarationOrSpecialization(unsigned Context,
   do {
     // Consume the 'export', if any.
     SourceLocation ExportLoc;
-    if (Tok.is(tok::kw_export)) {
+    TryConsumeToken(tok::kw_export, ExportLoc);
-      ExportLoc = ConsumeToken();
-    }
 
     // Consume the 'template', which should be here.
     SourceLocation TemplateLoc;
-    if (Tok.is(tok::kw_template)) {
+    if (!TryConsumeToken(tok::kw_template, TemplateLoc)) {
-      TemplateLoc = ConsumeToken();
-    } else {
       Diag(Tok.getLocation(), diag::err_expected_template);
       return 0;
     }
@@ -121,8 +117,7 @@ Parser::ParseTemplateDeclarationOrSpecialization(unsigned Context,
                                 TemplateParams, LAngleLoc, RAngleLoc)) {
       // Skip until the semi-colon or a }.
       SkipUntil(tok::r_brace, StopAtSemi | StopBeforeMatch);
-      if (Tok.is(tok::semi))
+      TryConsumeToken(tok::semi);
-        ConsumeToken();
       return 0;
     }
 
@@ -302,11 +297,10 @@ bool Parser::ParseTemplateParameters(unsigned Depth,
                                      SourceLocation &LAngleLoc,
                                      SourceLocation &RAngleLoc) {
   // Get the template parameter list.
-  if (!Tok.is(tok::less)) {
+  if (!TryConsumeToken(tok::less, LAngleLoc)) {
     Diag(Tok.getLocation(), diag::err_expected_less_after) << "template";
     return true;
   }
-  LAngleLoc = ConsumeToken();
 
   // Try to parse the template parameter list.
   bool Failed = false;
@@ -483,10 +477,8 @@ Decl *Parser::ParseTypeParameter(unsigned Depth, unsigned Position) {
   // Grab the ellipsis (if given).
   bool Ellipsis = false;
   SourceLocation EllipsisLoc;
-  if (Tok.is(tok::ellipsis)) {
+  if (TryConsumeToken(tok::ellipsis, EllipsisLoc)) {
     Ellipsis = true;
-    EllipsisLoc = ConsumeToken();
-
     Diag(EllipsisLoc,
          getLangOpts().CPlusPlus11
            ? diag::warn_cxx98_compat_variadic_templates
@@ -513,11 +505,9 @@ Decl *Parser::ParseTypeParameter(unsigned Depth, unsigned Position) {
   // we introduce the type parameter into the local scope.
   SourceLocation EqualLoc;
   ParsedType DefaultArg;
-  if (Tok.is(tok::equal)) {
+  if (TryConsumeToken(tok::equal, EqualLoc))
-    EqualLoc = ConsumeToken();
     DefaultArg = ParseTypeName(/*Range=*/0,
                                Declarator::TemplateTypeArgContext).get();
-  }
 
   return Actions.ActOnTypeParameter(getCurScope(), TypenameKeyword, Ellipsis, 
                                     EllipsisLoc, KeyLoc, ParamName, NameLoc,
@@ -552,7 +542,7 @@ Parser::ParseTemplateTemplateParameter(unsigned Depth, unsigned Position) {
   // identifier, comma, or greater. Provide a fixit if the identifier, comma,
   // or greater appear immediately or after 'typename' or 'struct'. In the
   // latter case, replace the keyword with 'class'.
-  if (!Tok.is(tok::kw_class)) {
+  if (!TryConsumeToken(tok::kw_class)) {
     bool Replace = Tok.is(tok::kw_typename) || Tok.is(tok::kw_struct);
     const Token& Next = Replace ? NextToken() : Tok;
     if (Next.is(tok::identifier) || Next.is(tok::comma) ||
@@ -566,19 +556,15 @@ Parser::ParseTemplateTemplateParameter(unsigned Depth, unsigned Position) {
 
     if (Replace)
       ConsumeToken();
-  } else
+  }
-    ConsumeToken();
 
   // Parse the ellipsis, if given.
   SourceLocation EllipsisLoc;
-  if (Tok.is(tok::ellipsis)) {
+  if (TryConsumeToken(tok::ellipsis, EllipsisLoc))
-    EllipsisLoc = ConsumeToken();
-    
     Diag(EllipsisLoc,
          getLangOpts().CPlusPlus11
            ? diag::warn_cxx98_compat_variadic_templates
            : diag::ext_variadic_templates);
-  }
       
   // Get the identifier, if given.
   SourceLocation NameLoc;
@@ -607,8 +593,7 @@ Parser::ParseTemplateTemplateParameter(unsigned Depth, unsigned Position) {
   // we introduce the template parameter into the local scope.
   SourceLocation EqualLoc;
   ParsedTemplateArgument DefaultArg;
-  if (Tok.is(tok::equal)) {
+  if (TryConsumeToken(tok::equal, EqualLoc)) {
-    EqualLoc = ConsumeToken();
     DefaultArg = ParseTemplateTemplateArgument();
     if (DefaultArg.isInvalid()) {
       Diag(Tok.getLocation(), 
@@ -651,9 +636,7 @@ Parser::ParseNonTypeTemplateParameter(unsigned Depth, unsigned Position) {
   // we introduce the template parameter into the local scope.
   SourceLocation EqualLoc;
   ExprResult DefaultArg;
-  if (Tok.is(tok::equal)) {
+  if (TryConsumeToken(tok::equal, EqualLoc)) {
-    EqualLoc = ConsumeToken();
-
     // C++ [temp.param]p15:
     //   When parsing a default template-argument for a non-type
     //   template-parameter, the first non-nested > is taken as the
@@ -900,8 +883,7 @@ bool Parser::AnnotateTemplateIdToken(TemplateTy Template, TemplateNameKind TNK,
   if (Invalid) {
     // If we failed to parse the template ID but skipped ahead to a >, we're not
     // going to be able to form a token annotation.  Eat the '>' if present.
-    if (Tok.is(tok::greater))
+    TryConsumeToken(tok::greater);
-      ConsumeToken();
     return true;
   }
 
@@ -916,8 +898,7 @@ bool Parser::AnnotateTemplateIdToken(TemplateTy Template, TemplateNameKind TNK,
     if (Type.isInvalid()) {
       // If we failed to parse the template ID but skipped ahead to a >, we're not
       // going to be able to form a token annotation.  Eat the '>' if present.
-      if (Tok.is(tok::greater))
+      TryConsumeToken(tok::greater);
-        ConsumeToken();
       return true;
     }
 
@@ -1067,10 +1048,8 @@ ParsedTemplateArgument Parser::ParseTemplateTemplateArgument() {
     UnqualifiedId Name;
     Name.setIdentifier(Tok.getIdentifierInfo(), Tok.getLocation());
     ConsumeToken(); // the identifier
-    
+
-    // Parse the ellipsis.
+    TryConsumeToken(tok::ellipsis, EllipsisLoc);
-    if (Tok.is(tok::ellipsis))
-      EllipsisLoc = ConsumeToken();
 
     if (isEndOfTemplateArgument(Tok)) {
       bool MemberOfUnknownSpecialization;
@@ -1161,9 +1140,8 @@ bool Parser::IsTemplateArgumentList(unsigned Skip) {
   }
   
   // '<'
-  if (!Tok.is(tok::less))
+  if (!TryConsumeToken(tok::less))
     return false;
-  ConsumeToken();
 
   // An empty template argument list.
   if (Tok.is(tok::greater))
@@ -1188,12 +1166,11 @@ Parser::ParseTemplateArgumentList(TemplateArgList &TemplateArgs) {
   // Template argument lists are constant-evaluation contexts.
   EnterExpressionEvaluationContext EvalContext(Actions,Sema::ConstantEvaluated);
 
-  while (true) {
+  do {
     ParsedTemplateArgument Arg = ParseTemplateArgument();
-    if (Tok.is(tok::ellipsis)) {
+    SourceLocation EllipsisLoc;
-      SourceLocation EllipsisLoc  = ConsumeToken();
+    if (TryConsumeToken(tok::ellipsis, EllipsisLoc))
       Arg = Actions.ActOnPackExpansion(Arg, EllipsisLoc);
-    }
 
     if (Arg.isInvalid()) {
       SkipUntil(tok::comma, tok::greater, StopAtSemi | StopBeforeMatch);
@@ -1205,11 +1182,7 @@ Parser::ParseTemplateArgumentList(TemplateArgList &TemplateArgs) {
       
     // If the next token is a comma, consume it and keep reading
     // arguments.
-    if (Tok.isNot(tok::comma)) break;
+  } while (TryConsumeToken(tok::comma));
-
-    // Consume the comma.
-    ConsumeToken();
-  }
 
   return false;
 }