From 2f412f2b9817c1b159472cb3c18438ffdb4a5407 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Wed, 27 Jun 2018 01:32:04 +0000 Subject: [PATCH] Diagnose missing 'template' keywords in contexts where a comma is not a binary operator. Factor out the checking for a comma within potential angle brackets and also call it from contexts where we parse a comma-separated list of arguments or initializers. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@335699 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Parse/Parser.h | 8 ++ include/clang/Sema/Sema.h | 18 ++- lib/Parse/ParseExpr.cpp | 91 +++------------ lib/Parse/ParseExprCXX.cpp | 11 +- lib/Parse/ParseTemplate.cpp | 108 ++++++++++++++++++ .../dependent-template-recover.cpp | 10 ++ 6 files changed, 157 insertions(+), 89 deletions(-) diff --git a/include/clang/Parse/Parser.h b/include/clang/Parse/Parser.h index df803c52f1..29cd9e82d7 100644 --- a/include/clang/Parse/Parser.h +++ b/include/clang/Parse/Parser.h @@ -1607,6 +1607,14 @@ private: } bool diagnoseUnknownTemplateId(ExprResult TemplateName, SourceLocation Less); + void checkPotentialAngleBracket(ExprResult &PotentialTemplateName); + bool checkPotentialAngleBracketDelimiter(const AngleBracketTracker::Loc &, + const Token &OpToken); + bool checkPotentialAngleBracketDelimiter(const Token &OpToken) { + if (auto *Info = AngleBrackets.getCurrent(*this)) + return checkPotentialAngleBracketDelimiter(*Info, OpToken); + return false; + } ExprResult ParsePostfixExpressionSuffix(ExprResult LHS); ExprResult ParseUnaryExprOrTypeTraitExpression(); diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index 93b24297d3..4897a2613e 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -1830,23 +1830,19 @@ public: getTemplateNameKindForDiagnostics(TemplateName Name); /// Determine whether it's plausible that E was intended to be a - /// template-name. Updates E to denote the template name expression. - bool mightBeIntendedToBeTemplateName(ExprResult &ER, bool &Dependent) { - if (!getLangOpts().CPlusPlus || ER.isInvalid()) + /// template-name. + bool mightBeIntendedToBeTemplateName(ExprResult E, bool &Dependent) { + if (!getLangOpts().CPlusPlus || E.isInvalid()) return false; - Expr *E = ER.get()->IgnoreImplicit(); - while (auto *BO = dyn_cast(E)) - E = BO->getRHS()->IgnoreImplicit(); - ER = E; Dependent = false; - if (auto *DRE = dyn_cast(E)) + if (auto *DRE = dyn_cast(E.get())) return !DRE->hasExplicitTemplateArgs(); - if (auto *ME = dyn_cast(E)) + if (auto *ME = dyn_cast(E.get())) return !ME->hasExplicitTemplateArgs(); Dependent = true; - if (auto *DSDRE = dyn_cast(E)) + if (auto *DSDRE = dyn_cast(E.get())) return !DSDRE->hasExplicitTemplateArgs(); - if (auto *DSME = dyn_cast(E)) + if (auto *DSME = dyn_cast(E.get())) return !DSME->hasExplicitTemplateArgs(); // Any additional cases recognized here should also be handled by // diagnoseExprIntendedAsTemplateName. diff --git a/lib/Parse/ParseExpr.cpp b/lib/Parse/ParseExpr.cpp index 94e56e9d8a..619fd222fd 100644 --- a/lib/Parse/ParseExpr.cpp +++ b/lib/Parse/ParseExpr.cpp @@ -246,30 +246,6 @@ bool Parser::isNotExpressionStart() { return isKnownToBeDeclarationSpecifier(); } -/// We've parsed something that could plausibly be intended to be a template -/// name (\p LHS) followed by a '<' token, and the following code can't possibly -/// be an expression. Determine if this is likely to be a template-id and if so, -/// diagnose it. -bool Parser::diagnoseUnknownTemplateId(ExprResult LHS, SourceLocation Less) { - TentativeParsingAction TPA(*this); - // FIXME: We could look at the token sequence in a lot more detail here. - if (SkipUntil(tok::greater, tok::greatergreater, tok::greatergreatergreater, - StopAtSemi | StopBeforeMatch)) { - TPA.Commit(); - - SourceLocation Greater; - ParseGreaterThanInTemplateList(Greater, true, false); - Actions.diagnoseExprIntendedAsTemplateName(getCurScope(), LHS, - Less, Greater); - return true; - } - - // There's no matching '>' token, this probably isn't supposed to be - // interpreted as a template-id. Parse it as an (ill-formed) comparison. - TPA.Revert(); - return false; -} - bool Parser::isFoldOperator(prec::Level Level) const { return Level > prec::Unknown && Level != prec::Conditional && Level != prec::Spaceship; @@ -303,57 +279,12 @@ Parser::ParseRHSOfBinaryExpression(ExprResult LHS, prec::Level MinPrec) { return ExprError(Diag(Tok, diag::err_opencl_logical_exclusive_or)); } - // If we have a name-shaped expression followed by '<', track it in case we - // later find we're probably supposed to be in a template-id. - ExprResult TemplateName = LHS; - bool DependentTemplateName = false; - if (OpToken.is(tok::less) && Actions.mightBeIntendedToBeTemplateName( - TemplateName, DependentTemplateName)) { - AngleBracketTracker::Priority Priority = - (DependentTemplateName ? AngleBracketTracker::DependentName - : AngleBracketTracker::PotentialTypo) | - (OpToken.hasLeadingSpace() ? AngleBracketTracker::SpaceBeforeLess - : AngleBracketTracker::NoSpaceBeforeLess); - AngleBrackets.add(*this, TemplateName.get(), OpToken.getLocation(), - Priority); - } - // If we're potentially in a template-id, we may now be able to determine // whether we're actually in one or not. - if (auto *Info = AngleBrackets.getCurrent(*this)) { - // If an operator is followed by a type that can be a template argument - // and cannot be an expression, then this is ill-formed, but might be - // intended to be part of a template-id. Likewise if this is <>. - if ((OpToken.isOneOf(tok::less, tok::comma) && - isKnownToBeDeclarationSpecifier()) || - (OpToken.is(tok::less) && - Tok.isOneOf(tok::greater, tok::greatergreater, - tok::greatergreatergreater))) { - if (diagnoseUnknownTemplateId(Info->TemplateName, Info->LessLoc)) { - AngleBrackets.clear(*this); - return ExprError(); - } - } - - // If a context that looks like a template-id is followed by '()', then - // this is ill-formed, but might be intended to be a template-id followed - // by '()'. - if (OpToken.is(tok::greater) && Tok.is(tok::l_paren) && - NextToken().is(tok::r_paren)) { - Actions.diagnoseExprIntendedAsTemplateName( - getCurScope(), Info->TemplateName, Info->LessLoc, - OpToken.getLocation()); - AngleBrackets.clear(*this); - return ExprError(); - } - } - - // After a '>' (etc), we're no longer potentially in a construct that's - // intended to be treated as a template-id. - if (OpToken.is(tok::greater) || - (getLangOpts().CPlusPlus11 && - OpToken.isOneOf(tok::greatergreater, tok::greatergreatergreater))) - AngleBrackets.clear(*this); + if (OpToken.isOneOf(tok::comma, tok::greater, tok::greatergreater, + tok::greatergreatergreater) && + checkPotentialAngleBracketDelimiter(OpToken)) + return ExprError(); // Bail out when encountering a comma followed by a token which can't // possibly be the start of an expression. For instance: @@ -879,6 +810,8 @@ ExprResult Parser::ParseCastExpression(bool isUnaryExpression, assert(Res.get() == nullptr && "Stray primary-expression annotation?"); Res = getExprAnnotation(Tok); ConsumeAnnotationToken(); + if (!Res.isInvalid() && Tok.is(tok::less)) + checkPotentialAngleBracket(Res); break; case tok::kw___super: @@ -1098,11 +1031,13 @@ ExprResult Parser::ParseCastExpression(bool isUnaryExpression, isAddressOfOperand, std::move(Validator), /*IsInlineAsmIdentifier=*/false, Tok.is(tok::r_paren) ? nullptr : &Replacement); - if (!Res.isInvalid() && !Res.get()) { + if (!Res.isInvalid() && Res.isUnset()) { UnconsumeToken(Replacement); return ParseCastExpression(isUnaryExpression, isAddressOfOperand, NotCastExpr, isTypeCast); } + if (!Res.isInvalid() && Tok.is(tok::less)) + checkPotentialAngleBracket(Res); break; } case tok::char_constant: // constant: character-constant @@ -1841,6 +1776,8 @@ Parser::ParsePostfixExpressionSuffix(ExprResult LHS) { OpKind, SS, TemplateKWLoc, Name, CurParsedObjCImpl ? CurParsedObjCImpl->Dcl : nullptr); + if (!LHS.isInvalid() && Tok.is(tok::less)) + checkPotentialAngleBracket(LHS); break; } case tok::plusplus: // postfix-expression: postfix-expression '++' @@ -2878,7 +2815,10 @@ bool Parser::ParseExpressionList(SmallVectorImpl &Exprs, if (Tok.isNot(tok::comma)) break; // Move to the next argument, remember where the comma was. + Token Comma = Tok; CommaLocs.push_back(ConsumeToken()); + + checkPotentialAngleBracketDelimiter(Comma); } if (SawError) { // Ensure typos get diagnosed when errors were encountered while parsing the @@ -2913,7 +2853,10 @@ Parser::ParseSimpleExpressionList(SmallVectorImpl &Exprs, return false; // Move to the next argument, remember where the comma was. + Token Comma = Tok; CommaLocs.push_back(ConsumeToken()); + + checkPotentialAngleBracketDelimiter(Comma); } } diff --git a/lib/Parse/ParseExprCXX.cpp b/lib/Parse/ParseExprCXX.cpp index a5a340ad22..77f758b159 100644 --- a/lib/Parse/ParseExprCXX.cpp +++ b/lib/Parse/ParseExprCXX.cpp @@ -561,10 +561,13 @@ ExprResult Parser::tryParseCXXIdExpression(CXXScopeSpec &SS, bool isAddressOfOpe if (isAddressOfOperand && isPostfixExpressionSuffixStart()) isAddressOfOperand = false; - return Actions.ActOnIdExpression(getCurScope(), SS, TemplateKWLoc, Name, - Tok.is(tok::l_paren), isAddressOfOperand, - nullptr, /*IsInlineAsmIdentifier=*/false, - &Replacement); + ExprResult E = Actions.ActOnIdExpression( + getCurScope(), SS, TemplateKWLoc, Name, Tok.is(tok::l_paren), + isAddressOfOperand, nullptr, /*IsInlineAsmIdentifier=*/false, + &Replacement); + if (!E.isInvalid() && !E.isUnset() && Tok.is(tok::less)) + checkPotentialAngleBracket(E); + return E; } /// ParseCXXIdExpression - Handle id-expression. diff --git a/lib/Parse/ParseTemplate.cpp b/lib/Parse/ParseTemplate.cpp index d1d53b07e0..5d47161ff5 100644 --- a/lib/Parse/ParseTemplate.cpp +++ b/lib/Parse/ParseTemplate.cpp @@ -1474,3 +1474,111 @@ void Parser::LexTemplateFunctionForLateParsing(CachedTokens &Toks) { } } } + +/// We've parsed something that could plausibly be intended to be a template +/// name (\p LHS) followed by a '<' token, and the following code can't possibly +/// be an expression. Determine if this is likely to be a template-id and if so, +/// diagnose it. +bool Parser::diagnoseUnknownTemplateId(ExprResult LHS, SourceLocation Less) { + TentativeParsingAction TPA(*this); + // FIXME: We could look at the token sequence in a lot more detail here. + if (SkipUntil(tok::greater, tok::greatergreater, tok::greatergreatergreater, + StopAtSemi | StopBeforeMatch)) { + TPA.Commit(); + + SourceLocation Greater; + ParseGreaterThanInTemplateList(Greater, true, false); + Actions.diagnoseExprIntendedAsTemplateName(getCurScope(), LHS, + Less, Greater); + return true; + } + + // There's no matching '>' token, this probably isn't supposed to be + // interpreted as a template-id. Parse it as an (ill-formed) comparison. + TPA.Revert(); + return false; +} + +void Parser::checkPotentialAngleBracket(ExprResult &PotentialTemplateName) { + assert(Tok.is(tok::less) && "not at a potential angle bracket"); + + bool DependentTemplateName = false; + if (!Actions.mightBeIntendedToBeTemplateName(PotentialTemplateName, + DependentTemplateName)) + return; + + // OK, this might be a name that the user intended to be parsed as a + // template-name, followed by a '<' token. Check for some easy cases. + + // If we have potential_template<>, then it's supposed to be a template-name. + if (NextToken().is(tok::greater) || + (getLangOpts().CPlusPlus11 && + NextToken().isOneOf(tok::greatergreater, tok::greatergreatergreater))) { + SourceLocation Less = ConsumeToken(); + SourceLocation Greater; + ParseGreaterThanInTemplateList(Greater, true, false); + Actions.diagnoseExprIntendedAsTemplateName( + getCurScope(), PotentialTemplateName, Less, Greater); + // FIXME: Perform error recovery. + PotentialTemplateName = ExprError(); + return; + } + + // If we have 'potential_template' later on. + { + // FIXME: Avoid the tentative parse when NextToken() can't begin a type. + TentativeParsingAction TPA(*this); + SourceLocation Less = ConsumeToken(); + if (isTypeIdUnambiguously() && + diagnoseUnknownTemplateId(PotentialTemplateName, Less)) { + TPA.Commit(); + // FIXME: Perform error recovery. + PotentialTemplateName = ExprError(); + return; + } + TPA.Revert(); + } + + // Otherwise, remember that we saw this in case we see a potentially-matching + // '>' token later on. + AngleBracketTracker::Priority Priority = + (DependentTemplateName ? AngleBracketTracker::DependentName + : AngleBracketTracker::PotentialTypo) | + (Tok.hasLeadingSpace() ? AngleBracketTracker::SpaceBeforeLess + : AngleBracketTracker::NoSpaceBeforeLess); + AngleBrackets.add(*this, PotentialTemplateName.get(), Tok.getLocation(), + Priority); +} + +bool Parser::checkPotentialAngleBracketDelimiter( + const AngleBracketTracker::Loc &LAngle, const Token &OpToken) { + // If a comma in an expression context is followed by a type that can be a + // template argument and cannot be an expression, then this is ill-formed, + // but might be intended to be part of a template-id. + if (OpToken.is(tok::comma) && isTypeIdUnambiguously() && + diagnoseUnknownTemplateId(LAngle.TemplateName, LAngle.LessLoc)) { + AngleBrackets.clear(*this); + return true; + } + + // If a context that looks like a template-id is followed by '()', then + // this is ill-formed, but might be intended to be a template-id + // followed by '()'. + if (OpToken.is(tok::greater) && Tok.is(tok::l_paren) && + NextToken().is(tok::r_paren)) { + Actions.diagnoseExprIntendedAsTemplateName( + getCurScope(), LAngle.TemplateName, LAngle.LessLoc, + OpToken.getLocation()); + AngleBrackets.clear(*this); + return true; + } + + // After a '>' (etc), we're no longer potentially in a construct that's + // intended to be treated as a template-id. + if (OpToken.is(tok::greater) || + (getLangOpts().CPlusPlus11 && + OpToken.isOneOf(tok::greatergreater, tok::greatergreatergreater))) + AngleBrackets.clear(*this); + return false; +} diff --git a/test/SemaTemplate/dependent-template-recover.cpp b/test/SemaTemplate/dependent-template-recover.cpp index 617950a4da..37a8faa705 100644 --- a/test/SemaTemplate/dependent-template-recover.cpp +++ b/test/SemaTemplate/dependent-template-recover.cpp @@ -32,6 +32,16 @@ struct X { // FIXME: Is this the right heuristic? xyz(); // expected-error{{missing 'template' keyword prior to dependent template name 'foo'}} T::foo < xyz<1>(); // expected-error{{missing 'template' keyword prior to dependent template name 'foo'}} + + sizeof T::foo < 123 > (); // expected-error{{missing 'template' keyword prior to dependent template name 'foo'}} + f(t->foo<1, 2>(), // expected-error{{missing 'template' keyword prior to dependent template name 'foo'}} + t->bar<3, 4>()); // expected-error{{missing 'template' keyword prior to dependent template name 'bar'}} + + int arr[] = { + t->baz<1, 2>(1 + 1), // ok, two comparisons + t->foo<1, 2>(), // expected-error{{missing 'template' keyword prior to dependent template name 'foo'}} + t->bar<3, 4>() // FIXME: we don't recover from the previous error so don't diagnose this + }; } int xyz; -- 2.40.0