When we see a '<' operator, check whether it's a probable typo for a template-id.

The heuristic that we use here is:
 * the left-hand side must be a simple identifier or a class member access
 * the right-hand side must be '<' followed by either a '>' or by a type-id that
   cannot be an expression (in particular, not followed by '(' or '{')
 * there is a '>' token matching the '<' token

The second condition guarantees the expression would otherwise be ill-formed.

If we're confident that the user intended the name before the '<' to be
interpreted as a template, diagnose the fact that we didn't interpret it
that way, rather than diagnosing that the template arguments are not valid
expressions.

llvm-svn: 302615

Author: zygoloid

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -8191,6 +8191,15 @@ def err_undeclared_var_use_suggest : Error<
 def err_no_template_suggest : Error<"no template named %0; did you mean %1?">;
 def err_no_member_template_suggest : Error<
   "no template named %0 in %1; did you mean %select{|simply }2%3?">;
+def err_non_template_in_template_id : Error<
+  "%0 does not name a template but is followed by template arguments">;
+def err_non_template_in_template_id_suggest : Error<
+  "%0 does not name a template but is followed by template arguments; "
+  "did you mean %1?">;
+def err_non_template_in_member_template_id_suggest : Error<
+  "member %0 of %1 is not a template; did you mean %select{|simply }2%3?">;
+def note_non_template_in_template_id_found : Note<
+  "non-template declaration found by name lookup">;
 def err_mem_init_not_member_or_class_suggest : Error<
   "initializer %0 does not name a non-static data member or base "
   "class; did you mean the %select{base class|member}1 %2?">;

clang/include/clang/Parse/Parser.h

@@ -1488,6 +1488,8 @@ class Parser : public CodeCompletionHandler {
             K == tok::plusplus || K == tok::minusminus);
   }
 
+  bool diagnoseUnknownTemplateId(ExprResult TemplateName, SourceLocation Less);
+
   ExprResult ParsePostfixExpressionSuffix(ExprResult LHS);
   ExprResult ParseUnaryExprOrTypeTraitExpression();
   ExprResult ParseBuiltinPrimaryExpression();

clang/include/clang/Sema/Sema.h

@@ -1738,6 +1738,23 @@ class Sema {
   TemplateNameKindForDiagnostics
   getTemplateNameKindForDiagnostics(TemplateName Name);
 
+  /// Determine whether it's plausible that E was intended to be a
+  /// template-name.
+  bool mightBeIntendedToBeTemplateName(ExprResult E) {
+    if (!getLangOpts().CPlusPlus || E.isInvalid())
+      return false;
+    if (auto *DRE = dyn_cast<DeclRefExpr>(E.get()))
+      return !DRE->hasExplicitTemplateArgs();
+    if (auto *ME = dyn_cast<MemberExpr>(E.get()))
+      return !ME->hasExplicitTemplateArgs();
+    // Any additional cases recognized here should also be handled by
+    // diagnoseExprIntendedAsTemplateName.
+    return false;
+  }
+  void diagnoseExprIntendedAsTemplateName(Scope *S, ExprResult TemplateName,
+                                          SourceLocation Less,
+                                          SourceLocation Greater);
+
   Decl *ActOnDeclarator(Scope *S, Declarator &D);
 
   NamedDecl *HandleDeclarator(Scope *S, Declarator &D,

clang/lib/Parse/ParseExpr.cpp

@@ -235,6 +235,30 @@ 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;
+}
+
 static bool isFoldOperator(prec::Level Level) {
   return Level > prec::Unknown && Level != prec::Conditional;
 }
@@ -276,6 +300,16 @@ Parser::ParseRHSOfBinaryExpression(ExprResult LHS, prec::Level MinPrec) {
       return LHS;
     }
 
+    // If a '<' token 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 a template-id.
+    if (OpToken.is(tok::less) && Actions.mightBeIntendedToBeTemplateName(LHS) &&
+        (isKnownToBeDeclarationSpecifier() ||
+         Tok.isOneOf(tok::greater, tok::greatergreater,
+                     tok::greatergreatergreater)) &&
+        diagnoseUnknownTemplateId(LHS, OpToken.getLocation()))
+      return ExprError();
+
     // If the next token is an ellipsis, then this is a fold-expression. Leave
     // it alone so we can handle it in the paren expression.
     if (isFoldOperator(NextTokPrec) && Tok.is(tok::ellipsis)) {

clang/lib/Sema/SemaTemplate.cpp

@@ -455,6 +455,85 @@ void Sema::LookupTemplateName(LookupResult &Found,
   }
 }
 
+void Sema::diagnoseExprIntendedAsTemplateName(Scope *S, ExprResult TemplateName,
+                                              SourceLocation Less,
+                                              SourceLocation Greater) {
+  if (TemplateName.isInvalid())
+    return;
+
+  DeclarationNameInfo NameInfo;
+  CXXScopeSpec SS;
+  LookupNameKind LookupKind;
+
+  DeclContext *LookupCtx = nullptr;
+  NamedDecl *Found = nullptr;
+
+  // Figure out what name we looked up.
+  if (auto *ME = dyn_cast<MemberExpr>(TemplateName.get())) {
+    NameInfo = ME->getMemberNameInfo();
+    SS.Adopt(ME->getQualifierLoc());
+    LookupKind = LookupMemberName;
+    LookupCtx = ME->getBase()->getType()->getAsCXXRecordDecl();
+    Found = ME->getMemberDecl();
+  } else {
+    auto *DRE = cast<DeclRefExpr>(TemplateName.get());
+    NameInfo = DRE->getNameInfo();
+    SS.Adopt(DRE->getQualifierLoc());
+    LookupKind = LookupOrdinaryName;
+    Found = DRE->getFoundDecl();
+  }
+
+  // Try to correct the name by looking for templates and C++ named casts.
+  struct TemplateCandidateFilter : CorrectionCandidateCallback {
+    TemplateCandidateFilter() {
+      WantTypeSpecifiers = false;
+      WantExpressionKeywords = false;
+      WantRemainingKeywords = false;
+      WantCXXNamedCasts = true;
+    };
+    bool ValidateCandidate(const TypoCorrection &Candidate) override {
+      if (auto *ND = Candidate.getCorrectionDecl())
+        return isAcceptableTemplateName(ND->getASTContext(), ND, true);
+      return Candidate.isKeyword();
+    }
+  };
+
+  DeclarationName Name = NameInfo.getName();
+  if (TypoCorrection Corrected =
+          CorrectTypo(NameInfo, LookupKind, S, &SS,
+                      llvm::make_unique<TemplateCandidateFilter>(),
+                      CTK_ErrorRecovery, LookupCtx)) {
+    auto *ND = Corrected.getFoundDecl();
+    if (ND)
+      ND = isAcceptableTemplateName(Context, ND,
+                                    /*AllowFunctionTemplates*/ true);
+    if (ND || Corrected.isKeyword()) {
+      if (LookupCtx) {
+        std::string CorrectedStr(Corrected.getAsString(getLangOpts()));
+        bool DroppedSpecifier = Corrected.WillReplaceSpecifier() &&
+                                Name.getAsString() == CorrectedStr;
+        diagnoseTypo(Corrected,
+                     PDiag(diag::err_non_template_in_member_template_id_suggest)
+                         << Name << LookupCtx << DroppedSpecifier
+                         << SS.getRange());
+      } else {
+        diagnoseTypo(Corrected,
+                     PDiag(diag::err_non_template_in_template_id_suggest)
+                         << Name);
+      }
+      if (Found)
+        Diag(Found->getLocation(),
+             diag::note_non_template_in_template_id_found);
+      return;
+    }
+  }
+
+  Diag(NameInfo.getLoc(), diag::err_non_template_in_template_id)
+    << Name << SourceRange(Less, Greater);
+  if (Found)
+    Diag(Found->getLocation(), diag::note_non_template_in_template_id_found);
+}
+
 /// ActOnDependentIdExpression - Handle a dependent id-expression that
 /// was just parsed.  This is only possible with an explicit scope
 /// specifier naming a dependent type.

clang/test/SemaCXX/cxx1y-variable-templates_top_level.cpp

@@ -9,7 +9,7 @@
 #endif
 
 template<typename T> 
-T pi = T(3.1415926535897932385); // expected-note {{template is declared here}}
+T pi = T(3.1415926535897932385); // expected-note 2{{declared here}}
 
 template<typename T> 
 CONST T cpi = T(3.1415926535897932385); // expected-note {{template is declared here}}
@@ -58,10 +58,9 @@ namespace use_in_top_level_funcs {
 namespace shadow {
   void foo() {
     int ipi0 = pi<int>;
-    int pi;
+    int pi; // expected-note {{found}}
     int a = pi;
-    int ipi = pi<int>;  // expected-error {{expected '(' for function-style cast or type construction}} \
-                        // expected-error {{expected expression}}
+    int ipi = pi<int>;  // expected-error {{'pi' does not name a template but is followed by template arguments; did you mean '::pi'?}}
   }
 }
 

clang/test/SemaTemplate/typo-template-name.cpp

@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 -std=c++1z %s -verify -Wno-unused
+
+namespace InExpr {
+  namespace A {
+    void typo_first_a(); // expected-note {{found}}
+    template<typename T> void typo_first_b(); // expected-note 2{{declared here}}
+  }
+  void testA() { A::typo_first_a<int>(); } // expected-error {{'typo_first_a' does not name a template but is followed by template arguments; did you mean 'typo_first_b'?}}
+
+  namespace B {
+    void typo_first_b(); // expected-note {{found}}
+  }
+  void testB() { B::typo_first_b<int>(); } // expected-error {{'typo_first_b' does not name a template but is followed by template arguments; did you mean 'A::typo_first_b'?}}
+
+  struct Base {
+    template<typename T> static void foo(); // expected-note 4{{declared here}}
+    int n;
+  };
+  struct Derived : Base {
+    void foo(); // expected-note {{found}}
+  };
+  // We probably don't want to suggest correcting to .Base::foo<int>
+  void testMember() { Derived().foo<int>(); } // expected-error-re {{does not name a template but is followed by template arguments{{$}}}}
+
+  struct Derived2 : Base {
+    void goo(); // expected-note {{found}}
+  };
+  void testMember2() { Derived2().goo<int>(); } // expected-error {{member 'goo' of 'InExpr::Derived2' is not a template; did you mean 'foo'?}}
+
+  void no_correction() {
+    int foo; // expected-note 3{{found}}
+
+    foo<int>(); // expected-error {{'foo' does not name a template but is followed by template arguments; did you mean 'Base::foo'?}}
+    foo<>(); // expected-error {{'foo' does not name a template but is followed by template arguments; did you mean 'Base::foo'?}}
+    foo<Base *>(); // expected-error {{'foo' does not name a template but is followed by template arguments; did you mean 'Base::foo'?}}
+
+    // These are valid expressions.
+    foo<foo; // expected-warning {{self-comparison}}
+    foo<int()>(0);
+    foo<int(), true>(false);
+    foo<Base{}.n;
+  }
+}