[Sema] Emit a diagnostic for an invalid dependent function template specialization

[erik.pilkington]

Summary

Previously, clang marked a decl as invalid without emitting a diagnostic. This lead to an assert in CodeGen for the attached test case.

rdar://41806724

Thanks for taking a look!
Erik

[rjmccall]

This is the right basic approach. I think it would be better if the diagnostic text was more like err_function_template_spec_no_match, maybe "no candidate function template was found for dependent friend function template specialization". And it would be good to emit notes on any declarations we found but discarded.

[erik.pilkington]

Improve the diagnostics. Thanks!

[rjmccall]

clang/include/clang/Basic/DiagnosticSemaKinds.td

4111

Your first explanation has a subject, but the second doesn't. And I think it would be nice to suggest adding explicit scope qualification in the second case.

I assume non-templates have previously been filtered out?

[erik.pilkington]

Improve the diagnostics further.

[erik.pilkington]

clang/include/clang/Basic/DiagnosticSemaKinds.td

4111

Oh, no, good point. A non-template can still get into here:

namespace ns1 { template <class T> struct foo {}; }
namespace ns2 { int foo() {} } // bad diag: template is not a function template
using ns1::foo;
using ns2::foo;

template <class T> class A {
    friend void foo<T>() {}
};

I added this to the testcases.

[rjmccall]

LGTM.

clang/include/clang/Basic/DiagnosticSemaKinds.td

4111

Thanks, these look great.

clang/include/clang/Basic/DiagnosticSemaKinds.td

 def err_not_class_template_specialization : Error<
   "cannot specialize a %select{dependent template|template template "
   "parameter}0">;
 def ext_explicit_specialization_storage_class : ExtWarn<
   "explicit specialization cannot have a storage class">;
 def err_explicit_specialization_inconsistent_storage_class : Error<
   "explicit specialization has extraneous, inconsistent storage class "
   "'%select{none|extern|static|__private_extern__|auto|register}0'">;
+def err_dependent_function_template_spec_no_match : Error<
+  "no candidate function template was found for dependent"
+  " friend function template specialization">;
+def note_dependent_function_template_spec_discard_reason : Note<
+  "candidate ignored: %select{not a function template"
+  "|not a member of the enclosing namespace; did you mean to explicitly qualify the specialization?}0">;
 
 // C++ class template specializations and out-of-line definitions
 def err_template_spec_needs_header : Error<
   "template specialization requires 'template<>'">;
 def err_template_spec_needs_template_parameters : Error<
   "template specialization or definition requires a template parameter list "
   "corresponding to the nested type %0">;
 def err_template_param_list_matches_nontemplate : Error<

clang/lib/Sema/SemaTemplate.cpp

 bool
 Sema::CheckDependentFunctionTemplateSpecialization(FunctionDecl *FD,
                    const TemplateArgumentListInfo &ExplicitTemplateArgs,
                                                    LookupResult &Previous) {
   // Remove anything from Previous that isn't a function template in
   // the correct context.
   DeclContext *FDLookupContext = FD->getDeclContext()->getRedeclContext();
   LookupResult::Filter F = Previous.makeFilter();
+  enum DiscardReason { NotAFunctionTemplate, NotAMemberOfEnclosing };
+  SmallVector<std::pair<DiscardReason, Decl *>, 8> DiscardedCandidates;
   while (F.hasNext()) {
     NamedDecl *D = F.next()->getUnderlyingDecl();
-    if (!isa<FunctionTemplateDecl>(D) ||
-        !FDLookupContext->InEnclosingNamespaceSetOf(
-                              D->getDeclContext()->getRedeclContext()))
+    if (!isa<FunctionTemplateDecl>(D)) {
       F.erase();
+      DiscardedCandidates.push_back(std::make_pair(NotAFunctionTemplate, D));
+      continue;
+    }
+
+    if (!FDLookupContext->InEnclosingNamespaceSetOf(
+            D->getDeclContext()->getRedeclContext())) {
+      F.erase();
+      DiscardedCandidates.push_back(std::make_pair(NotAMemberOfEnclosing, D));
+      continue;
+    }
   }
   F.done();
 
-  // Should this be diagnosed here?
-  if (Previous.empty()) return true;
+  if (Previous.empty()) {
+    Diag(FD->getLocation(),
+         diag::err_dependent_function_template_spec_no_match);
+    for (auto &P : DiscardedCandidates)
+      Diag(P.second->getLocation(),
+           diag::note_dependent_function_template_spec_discard_reason)
+          << P.first;
+    return true;
+  }
 
   FD->setDependentTemplateSpecialization(Context, Previous.asUnresolvedSet(),
                                          ExplicitTemplateArgs);
   return false;
 }
 
 /// Perform semantic analysis for the given function template
 /// specialization.

clang/test/CXX/temp/temp.decls/temp.friend/p1.cpp

namespace PR10913 {
   protected:
     int member; // expected-note{{declared protected here}}
   };
 
   template void f(X<int> *);
   template void f2<int>(X<int> *);
   template void f2<float>(X<int> *); // expected-note{{in instantiation of function template specialization 'PR10913::f2<float, int>' requested here}}
 }
+
+namespace test16 {
+template <class T> struct foo {}; // expected-note{{candidate ignored: not a function template}}
+template <class T> class A {
+  friend void foo<T>(); // expected-error{{no candidate function template was found for dependent friend function template specialization}}
+};
+}
+
+namespace test17 {
+namespace ns {
+template <class T> void foo() {} // expected-note{{candidate ignored: not a member of the enclosing namespace; did you mean to explicitly qualify the specialization?}}
+}
+using ns::foo;
+template <class T> struct A {
+  friend void foo<T>() {} // expected-error{{no candidate function template was found for dependent friend function template specialization}}
+};
+}
+
+namespace test18 {
+namespace ns1 { template <class T> struct foo {}; } // expected-note{{candidate ignored: not a function template}}
+namespace ns2 { void foo() {} } // expected-note{{candidate ignored: not a function template}}
+using ns1::foo;
+using ns2::foo;
+
+template <class T> class A {
+  friend void foo<T>() {} // expected-error{{no candidate function template was found for dependent friend function template specialization}}
+};
+}