[Sema] Fix assertion when constructor is disabled with partially specialized template.

[vsapsai]

Summary

The added test case was triggering assertion

Assertion failed: (!SpecializedTemplate.is<SpecializedPartialSpecialization*>() && "Already set to a class template partial specialization!"), function setInstantiationOf, file clang/include/clang/AST/DeclTemplate.h, line 1825.

It was happening with ClassTemplateSpecializationDecl
enable_if_not_same<int, int>. Because this template is specialized for
equal types not to have a definition, it wasn't instantiated and its
specialization kind remained TSK_Undeclared. And because it was implicit
instantiation, we didn't mark the decl as invalid. So when we try to
find the best matching partial specialization the second time, we hit
the assertion as partial specialization is already set.

Fix by reusing stored partial specialization when available, instead of
looking for the best match every time.

rdar://problem/39524996

[vsapsai]

clang/lib/Sema/SemaTemplate.cpp

3873–3881 ↗

(On Diff #146943)

Code of interest (will refer to it later).

[vsapsai]

Potentially, VarTemplateDecl is susceptible to the same bug as
ClassTemplateSpecializationDecl. But I wasn't able to trigger it. And based on
code I've convinced myself that the mentioned early exit in
Sema::CheckVarTemplateId is equivalent to reusing available partial
specialization. So seems like no changes for VarTemplateDecl are required.

[rsmith]

Looks good. It might be beneficial to do the lookup twice in this case and then issue a diagnostic if the results differ, but given how rarely this is happening, it doesn't really seem worthwhile.

[vsapsai]

Thanks for the quick review, Richard. I'll keep in mind the idea with comparing results of multiple lookups when I work on other partial specialization-related bugs.

cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp

 getPatternForClassTemplateSpecialization(
     Sema &S, SourceLocation PointOfInstantiation,
     ClassTemplateSpecializationDecl *ClassTemplateSpec,
     TemplateSpecializationKind TSK, bool Complain) {
   Sema::InstantiatingTemplate Inst(S, PointOfInstantiation, ClassTemplateSpec);
   if (Inst.isInvalid() || Inst.isAlreadyInstantiating())
     return nullptr;
 
+  llvm::PointerUnion<ClassTemplateDecl *,
+                     ClassTemplatePartialSpecializationDecl *>
+      Specialized = ClassTemplateSpec->getSpecializedTemplateOrPartial();
+  if (!Specialized.is<ClassTemplatePartialSpecializationDecl *>()) {
+    // Find best matching specialization.
     ClassTemplateDecl *Template = ClassTemplateSpec->getSpecializedTemplate();
-  CXXRecordDecl *Pattern = nullptr;
 
     // C++ [temp.class.spec.match]p1:
     //   When a class template is used in a context that requires an
     //   instantiation of the class, it is necessary to determine
     //   whether the instantiation is to be generated using the primary
     //   template or one of the partial specializations. This is done by
     //   matching the template arguments of the class template
     //   specialization with the template argument lists of the partial
     //   specializations.
     typedef PartialSpecMatchResult MatchResult;
     SmallVector<MatchResult, 4> Matched;
     SmallVector<ClassTemplatePartialSpecializationDecl *, 4> PartialSpecs;
     Template->getPartialSpecializations(PartialSpecs);
     TemplateSpecCandidateSet FailedCandidates(PointOfInstantiation);
     for (unsigned I = 0, N = PartialSpecs.size(); I != N; ++I) {
       ClassTemplatePartialSpecializationDecl *Partial = PartialSpecs[I];
       TemplateDeductionInfo Info(FailedCandidates.getLocation());
       if (Sema::TemplateDeductionResult Result = S.DeduceTemplateArguments(
               Partial, ClassTemplateSpec->getTemplateArgs(), Info)) {
         // Store the failed-deduction information for use in diagnostics, later.
         // TODO: Actually use the failed-deduction info?
         FailedCandidates.addCandidate().set(
             DeclAccessPair::make(Template, AS_public), Partial,
             MakeDeductionFailureInfo(S.Context, Result, Info));
         (void)Result;
       } else {
         Matched.push_back(PartialSpecMatchResult());
         Matched.back().Partial = Partial;
         Matched.back().Args = Info.take();
       }
     }
 
     // If we're dealing with a member template where the template parameters
     // have been instantiated, this provides the original template parameters
     // from which the member template's parameters were instantiated.
 
     if (Matched.size() >= 1) {
       SmallVectorImpl<MatchResult>::iterator Best = Matched.begin();
       if (Matched.size() == 1) {
         //   -- If exactly one matching specialization is found, the
         //      instantiation is generated from that specialization.
         // We don't need to do anything for this.
       } else {
         //   -- If more than one matching specialization is found, the
         //      partial order rules (14.5.4.2) are used to determine
         //      whether one of the specializations is more specialized
         //      than the others. If none of the specializations is more
         //      specialized than all of the other matching
         //      specializations, then the use of the class template is
         //      ambiguous and the program is ill-formed.
         for (SmallVectorImpl<MatchResult>::iterator P = Best + 1,
                                                  PEnd = Matched.end();
              P != PEnd; ++P) {
           if (S.getMoreSpecializedPartialSpecialization(
-                P->Partial, Best->Partial, PointOfInstantiation) == P->Partial)
+                  P->Partial, Best->Partial, PointOfInstantiation) ==
+              P->Partial)
             Best = P;
         }
-      
         // Determine if the best partial specialization is more specialized than
         // the others.
         bool Ambiguous = false;
         for (SmallVectorImpl<MatchResult>::iterator P = Matched.begin(),
                                                  PEnd = Matched.end();
              P != PEnd; ++P) {
-        if (P != Best &&
-            S.getMoreSpecializedPartialSpecialization(P->Partial, Best->Partial,
-                                                      PointOfInstantiation) !=
-                Best->Partial) {
+          if (P != Best && S.getMoreSpecializedPartialSpecialization(
+                               P->Partial, Best->Partial,
+                               PointOfInstantiation) != Best->Partial) {
             Ambiguous = true;
             break;
           }
         }
-       
         if (Ambiguous) {
           // Partial ordering did not produce a clear winner. Complain.
           Inst.Clear();
           ClassTemplateSpec->setInvalidDecl();
-        S.Diag(PointOfInstantiation, diag::err_partial_spec_ordering_ambiguous)
+          S.Diag(PointOfInstantiation,
+                 diag::err_partial_spec_ordering_ambiguous)
               << ClassTemplateSpec;
-        
           // Print the matching partial specializations.
           for (SmallVectorImpl<MatchResult>::iterator P = Matched.begin(),
                                                    PEnd = Matched.end();
                P != PEnd; ++P)
             S.Diag(P->Partial->getLocation(), diag::note_partial_spec_match)
                 << S.getTemplateArgumentBindingsText(
                        P->Partial->getTemplateParameters(), *P->Args);
 
           return nullptr;
         }
       }
-    
+      ClassTemplateSpec->setInstantiationOf(Best->Partial, Best->Args);
+    } else {
+      //   -- If no matches are found, the instantiation is generated
+      //      from the primary template.
+    }
+  }
+
+  CXXRecordDecl *Pattern = nullptr;
+  Specialized = ClassTemplateSpec->getSpecializedTemplateOrPartial();
+  if (auto *PartialSpec =
+          Specialized.dyn_cast<ClassTemplatePartialSpecializationDecl *>()) {
     // Instantiate using the best class template partial specialization.
-    ClassTemplatePartialSpecializationDecl *OrigPartialSpec = Best->Partial;
-    while (OrigPartialSpec->getInstantiatedFromMember()) {
+    while (PartialSpec->getInstantiatedFromMember()) {
       // If we've found an explicit specialization of this class template,
       // stop here and use that as the pattern.
-      if (OrigPartialSpec->isMemberSpecialization())
+      if (PartialSpec->isMemberSpecialization())
         break;
-      
-      OrigPartialSpec = OrigPartialSpec->getInstantiatedFromMember();
+      PartialSpec = PartialSpec->getInstantiatedFromMember();
     }
-    
-    Pattern = OrigPartialSpec;
-    ClassTemplateSpec->setInstantiationOf(Best->Partial, Best->Args);
+    Pattern = PartialSpec;
   } else {
-    //   -- If no matches are found, the instantiation is generated
-    //      from the primary template.
-    ClassTemplateDecl *OrigTemplate = Template;
-    while (OrigTemplate->getInstantiatedFromMemberTemplate()) {
+    ClassTemplateDecl *Template = ClassTemplateSpec->getSpecializedTemplate();
+    while (Template->getInstantiatedFromMemberTemplate()) {
       // If we've found an explicit specialization of this class template,
       // stop here and use that as the pattern.
-      if (OrigTemplate->isMemberSpecialization())
+      if (Template->isMemberSpecialization())
         break;
-      
-      OrigTemplate = OrigTemplate->getInstantiatedFromMemberTemplate();
+      Template = Template->getInstantiatedFromMemberTemplate();
     }
-    
-    Pattern = OrigTemplate->getTemplatedDecl();
+    Pattern = Template->getTemplatedDecl();
   }
 
   return Pattern;
 }
 
 bool Sema::InstantiateClassTemplateSpecialization(
     SourceLocation PointOfInstantiation,
     ClassTemplateSpecializationDecl *ClassTemplateSpec,

cfe/trunk/test/SemaTemplate/partial-spec-instantiate.cpp

namespace rdar9169404 {
 
   X<bool, -1>::type value;
 #if __cplusplus >= 201103L
   // expected-error@-2 {{non-type template argument evaluates to -1, which cannot be narrowed to type 'bool'}}
 #else
   // expected-no-diagnostics
 #endif
 }
+
+// rdar://problem/39524996
+namespace rdar39524996 {
+  template <typename T, typename U>
+  struct enable_if_not_same
+  {
+    typedef void type;
+  };
+  template <typename T>
+  struct enable_if_not_same<T, T>;
+
+  template <typename T>
+  struct Wrapper {
+    // Assertion triggered on trying to set twice the same partial specialization
+    // enable_if_not_same<int, int>
+    template <class U>
+    Wrapper(const Wrapper<U>& other,
+            typename enable_if_not_same<U, T>::type* = 0) {}
+
+    explicit Wrapper(int i) {}
+  };
+
+  template <class T>
+  struct Container {
+    // It is important that the struct has implicit copy and move constructors.
+    Container() : x() {}
+
+    template <class U>
+    Container(const Container<U>& other) : x(static_cast<T>(other.x)) {}
+
+    // Implicit constructors are member-wise, so the field triggers instantiation
+    // of T constructors and we instantiate all of them for overloading purposes.
+    T x;
+  };
+
+  void takesWrapperInContainer(const Container< Wrapper<int> >& c);
+  void test() {
+    // Type mismatch triggers initialization with conversion which requires
+    // implicit constructors to be instantiated.
+    Container<int> c;
+    takesWrapperInContainer(c);
+  }
+}