[Sema] Fix PR14211 Crash for explicit instantiation of overloaded template function within class template

[hintonda]

Summary

Fix PR14211

Only add FunctionDecl's that are actual template methods to the set of
matches passed to getMostSpecializied() -- otherwise it will
assert/crash. If a match to a non-template method is found, save the
pointer and don't call getMostSpecialized().

[rjmccall]

lib/Sema/SemaTemplate.cpp

8959

Please name this variable NonTemplateMatch instead of Specialization, then initialize the later Specialization variable to it if it's non-null. That's more self-documenting, and it avoids readability problems with the fact that this loop actually declares its own Specialization variable that would shadow this one.

Please also rename Matches to TemplateMatches.

8970–8976

This cast shouldn't be necessary. CXXMethodDecl is a subclass of FunctionDecl.

[hintonda]

• Renamed variables to make code self-documenting

[hintonda]

Btw, I don't have commit access, so if accepted, could you please commit for me.

[rjmccall]

lib/Sema/SemaTemplate.cpp

8977

Could you add an assertion here that NonTemplateMatch is still null? That should definitely never trip.

Hmm, actually, it might trip in invalid code; you should include a test case like

template <class T, class U> class A { void foo(T) {} void foo(U) {} };
template void A<int, int>::foo(int);

[hintonda]

lib/Sema/SemaTemplate.cpp

8977

I'll add the new test, but we do check for null below, i.e., we assign NonTemplateMatch to the original Specialization pointer, and then check the matches. If we don't find one, we true on line 7883.

[rjmccall]

lib/Sema/SemaTemplate.cpp

8977

No, I mean an assertion that we're not re-assigning NonTemplateMatch, i.e. that we didn't find two different non-template functions like this.

[hintonda]

• Added assert and additional test

[hintonda]

lib/Sema/SemaTemplate.cpp

8977

Got it...

[hintonda]

Fixes PR14211.

Since getMostSpecialized only handles FunctionTemplateDecl matches,
keep track of non-FunctionTemplateDecl matches and only call
getMostSpecialized if no non-FunctionTemplateDecl matches are found.

Added tests.

[rjmccall]

lib/Sema/SemaTemplate.cpp

8979

These continues that you're adding are already at the end of the loop body, so this code is really confusing.

I think the template-specialization check here is trying to do the same thing you're doing: treat non-template matches specially. But your way of handling it is better, so I would just remove the check and all the breaks and continues so that we add the match to either NonTemplateMatch or TemplateMatches. (Please do rename Matches to TemplateMatches, though.)

[hintonda]

• Addressed comments.

[rjmccall]

Thanks, looks great. Do you still not have commit access?

[hintonda]

Great, thanks John.

Yes, that's correct -- I do not have commit access.

[hintonda]

Since I don't have commit access, could you commit this for me?

thanks...
don

[rjmccall]

Sure thing, r304951.

lib/Sema/SemaTemplate.cpp

if (D.getName().getKind() == UnqualifiedId::IK_TemplateId) {
     HasExplicitTemplateArgs = true;
   }
 
   // C++ [temp.explicit]p1:
   //   A [...] function [...] can be explicitly instantiated from its template.
   //   A member function [...] of a class template can be explicitly
   //  instantiated from the member definition associated with its class
   //  template.
-  UnresolvedSet<8> Matches;
+  UnresolvedSet<8> TemplateMatches;
+  FunctionDecl *NonTemplateMatch = nullptr;
   AttributeList *Attr = D.getDeclSpec().getAttributes().getList();
   TemplateSpecCandidateSet FailedCandidates(D.getIdentifierLoc());
   for (LookupResult::iterator P = Previous.begin(), PEnd = Previous.end();
        P != PEnd; ++P) {
     NamedDecl *Prev = *P;
     if (!HasExplicitTemplateArgs) {
       if (CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(Prev)) {
         QualType Adjusted = adjustCCAndNoReturn(R, Method->getType(),
                                                 /*AdjustExceptionSpec*/true);
         if (Context.hasSameUnqualifiedType(Method->getType(), Adjusted)) {
-          Matches.clear();
-
-          Matches.addDecl(Method, P.getAccess());
-          if (Method->getTemplateSpecializationKind() == TSK_Undeclared)
-            break;
+          if (Method->getPrimaryTemplate()) {
+            TemplateMatches.addDecl(Method, P.getAccess());
+          } else {
+            // FIXME: Can this assert ever happen?  Needs a test.
+            assert(!NonTemplateMatch && "Multiple NonTemplateMatches");
+            NonTemplateMatch = Method;
+          }
         }
       }
     }
 
     FunctionTemplateDecl *FunTmpl = dyn_cast<FunctionTemplateDecl>(Prev);
     if (!FunTmpl)
       continue;
 
     TemplateDeductionInfo Info(FailedCandidates.getLocation());
     FunctionDecl *Specialization = nullptr;
     if (TemplateDeductionResult TDK
if (LangOpts.CUDA &&
                            /* IgnoreImplicitHDAttributes = */ true) !=
             IdentifyCUDATarget(Attr)) {
       FailedCandidates.addCandidate().set(
           P.getPair(), FunTmpl->getTemplatedDecl(),
           MakeDeductionFailureInfo(Context, TDK_CUDATargetMismatch, Info));
       continue;
     }
 
-    Matches.addDecl(Specialization, P.getAccess());
+    TemplateMatches.addDecl(Specialization, P.getAccess());
   }
 
+  FunctionDecl *Specialization = NonTemplateMatch;
+  if (!Specialization) {
     // Find the most specialized function template specialization.
     UnresolvedSetIterator Result = getMostSpecialized(
-      Matches.begin(), Matches.end(), FailedCandidates,
+        TemplateMatches.begin(), TemplateMatches.end(), FailedCandidates,
         D.getIdentifierLoc(),
         PDiag(diag::err_explicit_instantiation_not_known) << Name,
         PDiag(diag::err_explicit_instantiation_ambiguous) << Name,
         PDiag(diag::note_explicit_instantiation_candidate));
 
-  if (Result == Matches.end())
+    if (Result == TemplateMatches.end())
       return true;
 
     // Ignore access control bits, we don't need them for redeclaration checking.
-  FunctionDecl *Specialization = cast<FunctionDecl>(*Result);
+    Specialization = cast<FunctionDecl>(*Result);
+  }
 
   // C++11 [except.spec]p4
   // In an explicit instantiation an exception-specification may be specified,
   // but is not required.
   // If an exception-specification is specified in an explicit instantiation
   // directive, it shall be compatible with the exception-specifications of
   // other declarations of that function.
   if (auto *FPT = R->getAs<FunctionProtoType>())

test/CXX/temp/temp.decls/temp.mem/p5.cpp

 template X0::operator const int*(); // expected-note{{'X0::operator const int *<const int>' requested here}}
 template X0::operator float*() const; // expected-error{{explicit instantiation of undefined function template}}
 
 void test_X0(X0 x0, const X0 &x0c) {
   x0.operator const int*(); // expected-note{{in instantiation of function template specialization}}
   x0.operator float *();
   x0c.operator const char*();
 }
+
+namespace PR14211 {
+template <class U> struct X {
+  void foo(U){}
+  template <class T> void foo(T){}
+
+  template <class T> void bar(T){}
+  void bar(U){}
+};
+
+template void X<int>::foo(int);
+template void X<int>::bar(int);
+}