Template class: emit better diagnostic in case of missing template argument list

[davide]

Summary

Richard, this implements what you proposed in https://llvm.org/bugs/show_bug.cgi?id=25223 , hopefully in the correct way.
This is what we emit now:

template.cpp:7:3: error: missing template argument list for class template 'X'
T X::foo(void)

^
X<T, Q>

template.cpp:2:7: note: 'X' declared here
class X {

^

1 error generated.

I will tackle the case where we emit a terrible diagnostic for ambigous lookup separately.

[davide]

Gentle ping.

[davide]

Gentle Monday morning ping :)

[rsmith]

lib/Sema/SemaCXXScopeSpec.cpp

775–787

This isn't quite the right thing to do; it's looking at the template parameters of the class template rather than the current template parameters, and they could be quite different. Instead, you should do something like:

1. Check that the Scope (S) is a TemplateParamScope
2. Walk that scope, extract all of its declarations, and check that they match in type and kind the template parameters of CTD
3. If so, produce a suggestion using the names of those template parameters, as found in the scope

789

A FixItHint should only be attached to an error or warning diagnostic if we recover as if the fix-it were applied. Maybe move this to a separate note diagnostic instead?

790

You don't need to do this; ND will always be the same as CTD here.

791

Just pass CTD in here instead of using &Identifier.

792–793

No newline between } and else, please.

796

Likewise.

include/clang/Basic/DiagnosticSemaKinds.td

 def err_typecheck_nonviable_condition_incomplete : Error<
   "no viable conversion%diff{ from $ to incomplete type $|}0,1">;
 def err_typecheck_deleted_function : Error<
   "conversion function %diff{from $ to $|between types}0,1 "
   "invokes a deleted function">;
   
 def err_expected_class_or_namespace : Error<"%0 is not a class"
   "%select{ or namespace|, namespace, or enumeration}1">;
+def err_missing_argument_list : Error<"missing template argument list for "
+  "class template %0">;
 def err_invalid_declarator_scope : Error<"cannot define or redeclare %0 here "
   "because namespace %1 does not enclose namespace %2">;
 def err_invalid_declarator_global_scope : Error<
   "definition or redeclaration of %0 cannot name the global scope">;
 def err_invalid_declarator_in_function : Error<
   "definition or redeclaration of %0 not allowed inside a function">;
 def err_invalid_declarator_in_block : Error<
   "definition or redeclaration of %0 not allowed inside a block">;

lib/Sema/SemaCXXScopeSpec.cpp

             << &Identifier << ContainingClass;
         SS.Extend(Context, &Identifier, IdentifierLoc, CCLoc);
         return false;
       }
     }
   }
 
   if (!Found.empty()) {
-    if (TypeDecl *TD = Found.getAsSingle<TypeDecl>())
+    if (ClassTemplateDecl *CTD = Found.getAsSingle<ClassTemplateDecl>()) {
+      TemplateParameterList *TPL = CTD->getTemplateParameters();
+      assert(TPL && "NULL template parameter list");
+      std::string FixString = Identifier.getName();
+      FixString += "<";
+      bool FirstArg = true;
+      for (NamedDecl *TemplArg : *TPL) {
+        if (FirstArg)
+          FirstArg = false;
+        else
+          FixString += ", ";
+        FixString += TemplArg->getName();
+      }
+      FixString += ">";
+      Diag(IdentifierLoc, diag::err_missing_argument_list) << &Identifier
+          << FixItHint::CreateReplacement(IdentifierLoc, FixString);
+      if (NamedDecl *ND = Found.getAsSingle<NamedDecl>())
+        Diag(ND->getLocation(), diag::note_entity_declared_at) << &Identifier;
+    }
+    else if (TypeDecl *TD = Found.getAsSingle<TypeDecl>()) {
       Diag(IdentifierLoc, diag::err_expected_class_or_namespace)
           << QualType(TD->getTypeForDecl(), 0) << getLangOpts().CPlusPlus;
+    }
     else {
       Diag(IdentifierLoc, diag::err_expected_class_or_namespace)
           << &Identifier << getLangOpts().CPlusPlus;
       if (NamedDecl *ND = Found.getAsSingle<NamedDecl>())
         Diag(ND->getLocation(), diag::note_entity_declared_at) << &Identifier;
     }
   } else if (SS.isSet())
     Diag(IdentifierLoc, diag::err_no_member) << &Identifier << LookupCtx

test/SemaTemplate/temp_arg_lookup.cpp

+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+template<class T, class Q>
+class X { // expected-note{{declared here}}
+  T foo(void);
+};
+
+template<class T, class Q>
+T X::foo(void)  // expected-error{{missing template argument list for class template}}}
+{
+  return 0;
+}