From 26e65cec83b81d60dd4bc1977f01280905f00edc Mon Sep 17 00:00:00 2001 From: Alp Toker Date: Tue, 17 Dec 2013 14:12:37 +0000 Subject: [PATCH] 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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@197497 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Parse/Parser.h | 35 ++++++++++--- lib/Parse/ParseCXXInlineMethods.cpp | 28 ++++------- lib/Parse/ParseDecl.cpp | 76 ++++++++++++----------------- lib/Parse/ParseExpr.cpp | 18 ++----- lib/Parse/ParseExprCXX.cpp | 25 ++++------ lib/Parse/ParseTemplate.cpp | 67 ++++++++----------------- 6 files changed, 102 insertions(+), 147 deletions(-) diff --git a/include/clang/Parse/Parser.h b/include/clang/Parse/Parser.h index ddef9dfe84..bc33a7428f 100644 --- a/include/clang/Parse/Parser.h +++ b/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) { - assert(!isTokenStringLiteral() && !isTokenParen() && !isTokenBracket() && - !isTokenBrace() && + SourceLocation ConsumeToken() { + assert(!isTokenSpecial() && "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. diff --git a/lib/Parse/ParseCXXInlineMethods.cpp b/lib/Parse/ParseCXXInlineMethods.cpp index 9a69bec167..594baa1eda 100644 --- a/lib/Parse/ParseCXXInlineMethods.cpp +++ b/lib/Parse/ParseCXXInlineMethods.cpp @@ -69,9 +69,7 @@ NamedDecl *Parser::ParseCXXInlineMethodDef(AccessSpecifier AS, D.complete(FnD); - if (Tok.is(tok::equal)) { - ConsumeToken(); - + if (TryConsumeToken(tok::equal)) { 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)) { - Diag(Tok, getLangOpts().CPlusPlus11 ? - diag::warn_cxx98_compat_deleted_function : - diag::ext_deleted_function); - - KWLoc = ConsumeToken(); + if (TryConsumeToken(tok::kw_delete, KWLoc)) { + Diag(KWLoc, getLangOpts().CPlusPlus11 + ? diag::warn_cxx98_compat_deleted_function + : diag::ext_deleted_function); Actions.SetDeclDeleted(FnD, KWLoc); Delete = true; - } else if (Tok.is(tok::kw_default)) { - Diag(Tok, getLangOpts().CPlusPlus11 ? - diag::warn_cxx98_compat_defaulted_function : - diag::ext_defaulted_function); - - KWLoc = ConsumeToken(); + } else if (TryConsumeToken(tok::kw_default, KWLoc)) { + Diag(KWLoc, getLangOpts().CPlusPlus11 + ? diag::warn_cxx98_compat_defaulted_function + : diag::ext_defaulted_function); 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)) - ConsumeToken(); - else { + if (!TryConsumeToken(tok::cxx_defaultarg_end)) { // 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. diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp index 9d8af30b5b..6425725997 100644 --- a/lib/Parse/ParseDecl.cpp +++ b/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)) { - ConsumeToken(); - } else if (Tok.is(tok::r_brace)) { + if (TryConsumeToken(tok::semi)) + continue; + + 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)) { - EqualLoc = ConsumeToken(); + if (TryConsumeToken(tok::equal, EqualLoc)) { 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 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)) - break; - ConsumeToken(); - } + } while (TryConsumeToken(tok::comma)); } /// ParseParameterDeclarationClause - Parse a (possibly empty) parameter-list @@ -5481,13 +5477,11 @@ void Parser::ParseParameterDeclarationClause( ParsedAttributes &FirstArgAttrs, SmallVectorImpl &ParamInfo, SourceLocation &EllipsisLoc) { - while (1) { - if (Tok.is(tok::ellipsis)) { - // FIXME: Issue a diagnostic if we parsed an attribute-specifier-seq - // before deciding this was a parameter-declaration-clause. - EllipsisLoc = ConsumeToken(); // Consume the ellipsis. + do { + // FIXME: Issue a diagnostic if we parsed an attribute-specifier-seq + // before deciding this was a parameter-declaration-clause. + if (TryConsumeToken(tok::ellipsis, EllipsisLoc)) 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 (Tok.isNot(tok::comma)) { - if (Tok.is(tok::ellipsis)) { - EllipsisLoc = ConsumeToken(); // Consume the ellipsis. - - if (!getLangOpts().CPlusPlus) { - // 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, ", "); - } - } - + if (TryConsumeToken(tok::ellipsis, EllipsisLoc) && + !getLangOpts().CPlusPlus) { + // 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. - ConsumeToken(); - } - + // If the next token is a comma, consume it and keep reading arguments. + } 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)) - StaticLoc = ConsumeToken(); + TryConsumeToken(tok::kw_static, StaticLoc); // 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)) - StaticLoc = ConsumeToken(); + if (!StaticLoc.isValid()) + TryConsumeToken(tok::kw_static, StaticLoc); // Handle "direct-declarator [ type-qual-list[opt] * ]". bool isStar = false; diff --git a/lib/Parse/ParseExpr.cpp b/lib/Parse/ParseExpr.cpp index 5c6ff65841..7ace1f63d9 100644 --- a/lib/Parse/ParseExpr.cpp +++ b/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)) { - // Eat the colon. - ColonLoc = ConsumeToken(); - } else { + if (!TryConsumeToken(tok::colon, ColonLoc)) { // 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(); - if (Tok.is(tok::greatergreatergreater)) { - ConsumeToken(); + SourceLocation CloseLoc; + if (TryConsumeToken(tok::greatergreatergreater, CloseLoc)) { } 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()); - - if (Tok.isNot(tok::comma)) - break; - ConsumeToken(); - } + } while (TryConsumeToken(tok::comma)); T.consumeClose(); if (T.getCloseLocation().isInvalid()) diff --git a/lib/Parse/ParseExprCXX.cpp b/lib/Parse/ParseExprCXX.cpp index 4aba5f950a..38bf76ae21 100644 --- a/lib/Parse/ParseExprCXX.cpp +++ b/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 Parser::ParseLambdaIntroducer(LambdaIntroducer &Intro, // to save the necessary state, and restore it later. EnterExpressionEvaluationContext EC(Actions, Sema::PotentiallyEvaluated); - if (Tok.is(tok::equal)) - ConsumeToken(); + TryConsumeToken(tok::equal); if (!SkippedInits) Init = ParseInitializer(); @@ -868,8 +868,8 @@ Optional Parser::ParseLambdaIntroducer(LambdaIntroducer &Intro, ConsumeToken(); } } - } else if (Tok.is(tok::ellipsis)) - EllipsisLoc = ConsumeToken(); + } else + 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()); - - if (Tok.is(tok::comma)) { - ConsumeToken(); - continue; - } - - break; - } while (true); - + } while (TryConsumeToken(tok::comma)); + if (Parens.consumeClose()) return ExprError(); diff --git a/lib/Parse/ParseTemplate.cpp b/lib/Parse/ParseTemplate.cpp index 8275922a6e..67004cefaf 100644 --- a/lib/Parse/ParseTemplate.cpp +++ b/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)) { - ExportLoc = ConsumeToken(); - } + TryConsumeToken(tok::kw_export, ExportLoc); // Consume the 'template', which should be here. SourceLocation TemplateLoc; - if (Tok.is(tok::kw_template)) { - TemplateLoc = ConsumeToken(); - } else { + if (!TryConsumeToken(tok::kw_template, TemplateLoc)) { 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)) - ConsumeToken(); + TryConsumeToken(tok::semi); 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)) { - EqualLoc = ConsumeToken(); + if (TryConsumeToken(tok::equal, EqualLoc)) 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)) { - EllipsisLoc = ConsumeToken(); - + if (TryConsumeToken(tok::ellipsis, EllipsisLoc)) 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)) { - EqualLoc = ConsumeToken(); + if (TryConsumeToken(tok::equal, EqualLoc)) { 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)) { - EqualLoc = ConsumeToken(); - + if (TryConsumeToken(tok::equal, EqualLoc)) { // 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)) - ConsumeToken(); + TryConsumeToken(tok::greater); 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)) - ConsumeToken(); + TryConsumeToken(tok::greater); return true; } @@ -1067,10 +1048,8 @@ ParsedTemplateArgument Parser::ParseTemplateTemplateArgument() { UnqualifiedId Name; Name.setIdentifier(Tok.getIdentifierInfo(), Tok.getLocation()); ConsumeToken(); // the identifier - - // Parse the ellipsis. - if (Tok.is(tok::ellipsis)) - EllipsisLoc = ConsumeToken(); + + TryConsumeToken(tok::ellipsis, EllipsisLoc); 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 = ConsumeToken(); + SourceLocation EllipsisLoc; + 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; - - // Consume the comma. - ConsumeToken(); - } + } while (TryConsumeToken(tok::comma)); return false; } -- 2.50.1