[Sema] DR727: Ensure we pick up enclosing template instantiation arguments for a class-scope explicit specialization.

[erik.pilkington]

Summary

Previously, we stopped early on a class-scope explicit specialization, leading us to lose enclosing levels of template arguments. I'm not entirely convinced that this is the right fix, so any insight here would be greatly appreciated!

llvm.org/PR39031

Thanks!
Erik

[erik.pilkington]

clang/lib/Sema/SemaTemplateInstantiate.cpp

143

It looks like theres a bug here too:

template <class T> struct S {
  template <class T> void f() {}
  template <> void f<int>() {}
};

Will lead to a depth-2 MultiLevelTemplateArgumentList for f<int>, but I believe there should only be 1 level. I don't think theres any way to observe this though.

clang/test/SemaCXX/member-spec-dr727.cpp

35

This fails on TOT because clang substitutes int for Ap!

[rsmith]

clang/include/clang/AST/DeclTemplate.h

1771–1779

I think this name is confusing, given that we also have isMemberSpecialization, which is an entirely different thing.

Maybe isInstantiatedSpecialization would capture the essence here? (That is, this is a partial or explicit specialization that we instantiated from its enclosing template context rather than one that was declared in a non-template context.)

That also makes me wonder if we need to store additional state for this at all, or if we can determine this by checking whether the (first) declaration isOutOfLine().

[erik.pilkington]

clang/include/clang/AST/DeclTemplate.h

1771–1779

Maybe isInstantiatedSpecialization would capture the essence here? (That is, this is a partial or explicit specialization that we instantiated from its enclosing template context rather than one that was declared in a non-template context.)

But we should still probably return true for a specialization in a non-template class even though it wasn't instantiated (it doesn't really matter here, but for consistency). The new patch calls this isClassScopeSpecialization, what do you think of that name?

That also makes me wonder if we need to store additional state for this at all, or if we can determine this by checking whether the (first) declaration isOutOfLine().

Oh, good point! I think that would work too.

[erik.pilkington]

Address @rsmith's review comments.

[erik.pilkington]

Looks like @rsmith fixed this in r359266.

[rsmith]

In D52521#1479907, @erik.pilkington wrote:

Looks like @rsmith fixed this in r359266.

Sorry, I completely forgot about the existence of this when working on that patch! :(

[erik.pilkington]

No worries! It happens, I probably should have pinged this more aggressively.

clang/include/clang/AST/DeclTemplate.h

 
   /// The point where this template was instantiated (if any)
   SourceLocation PointOfInstantiation;
 
   /// The kind of specialization this declaration refers to.
   /// Really a value of type TemplateSpecializationKind.
   unsigned SpecializationKind : 3;
 
+  /// If this class template specialization was declared at class scope (DR727).
+  unsigned IsSpecializedMember : 1;
+
 protected:
   ClassTemplateSpecializationDecl(ASTContext &Context, Kind DK, TagKind TK,
                                   DeclContext *DC, SourceLocation StartLoc,
                                   SourceLocation IdLoc,
                                   ClassTemplateDecl *SpecializedTemplate,
                                   ArrayRef<TemplateArgument> Args,
                                   ClassTemplateSpecializationDecl *PrevDecl);
 

     return PointOfInstantiation;
   }
 
   void setPointOfInstantiation(SourceLocation Loc) {
     assert(Loc.isValid() && "point of instantiation must be valid!");
     PointOfInstantiation = Loc;
   }
 
+  void setIsSpecializedMember(bool V = true) { IsSpecializedMember = V; }
+
+  /// If this class template specialization was declared at class scope (DR727).
+  /// For example, the specialization G<int> below:
+  ///   struct S {
+  ///     template <class> struct G {};
+  ///     template <> struct G<int> {};
+  ///   };
+  bool isSpecializedMember() const { return IsSpecializedMember; }
+
   /// If this class template specialization is an instantiation of
   /// a template (rather than an explicit specialization), return the
   /// class template or class template partial specialization from which it
   /// was instantiated.
   llvm::PointerUnion<ClassTemplateDecl *,
                      ClassTemplatePartialSpecializationDecl *>
   getInstantiatedFrom() const {
     if (!isTemplateInstantiation(getSpecializationKind()))

   unsigned SpecializationKind : 3;
 
   /// Whether this declaration is a complete definition of the
   /// variable template specialization. We can't otherwise tell apart
   /// an instantiated declaration from an instantiated definition with
   /// no initializer.
   unsigned IsCompleteDefinition : 1;
 
+  /// If this variable template specialization was declared at class scope
+  /// (DR727).
+  unsigned IsSpecializedMember : 1;
+
 protected:
   VarTemplateSpecializationDecl(Kind DK, ASTContext &Context, DeclContext *DC,
                                 SourceLocation StartLoc, SourceLocation IdLoc,
                                 VarTemplateDecl *SpecializedTemplate,
                                 QualType T, TypeSourceInfo *TInfo,
                                 StorageClass S,
                                 ArrayRef<TemplateArgument> Args);
 

     ExplicitInfo->TemplateKeywordLoc = Loc;
   }
 
   /// Gets the location of the template keyword, if present.
   SourceLocation getTemplateKeywordLoc() const {
     return ExplicitInfo ? ExplicitInfo->TemplateKeywordLoc : SourceLocation();
   }
 
+  void setIsSpecializedMember(bool V = true) { IsSpecializedMember = V; }
+
+  /// If this variable template specialization was declared at class scope
+  /// (DR727). For example, the specialization v<int> below:
+  ///   struct S {
+  ///     template <class> static int v;
+  ///     template <> static int v<int>;
+  ///   };
+  bool isSpecializedMember() const { return IsSpecializedMember; }
+
+
   void Profile(llvm::FoldingSetNodeID &ID) const {
     Profile(ID, TemplateArgs->asArray(), getASTContext());
   }
 
   static void Profile(llvm::FoldingSetNodeID &ID,
                       ArrayRef<TemplateArgument> TemplateArgs,
                       ASTContext &Context) {
     ID.AddInteger(TemplateArgs.size());

clang/lib/AST/DeclTemplate.cpp

                                 SourceLocation IdLoc,
                                 ClassTemplateDecl *SpecializedTemplate,
                                 ArrayRef<TemplateArgument> Args,
                                 ClassTemplateSpecializationDecl *PrevDecl)
     : CXXRecordDecl(DK, TK, Context, DC, StartLoc, IdLoc,
                     SpecializedTemplate->getIdentifier(), PrevDecl),
     SpecializedTemplate(SpecializedTemplate),
     TemplateArgs(TemplateArgumentList::CreateCopy(Context, Args)),
-    SpecializationKind(TSK_Undeclared) {
+    SpecializationKind(TSK_Undeclared), IsSpecializedMember(0) {
 }
 
 ClassTemplateSpecializationDecl::ClassTemplateSpecializationDecl(ASTContext &C,
                                                                  Kind DK)
     : CXXRecordDecl(DK, TTK_Struct, C, nullptr, SourceLocation(),
                     SourceLocation(), nullptr, nullptr),
-      SpecializationKind(TSK_Undeclared) {}
+      SpecializationKind(TSK_Undeclared), IsSpecializedMember(0) {}
 
 ClassTemplateSpecializationDecl *
 ClassTemplateSpecializationDecl::Create(ASTContext &Context, TagKind TK,
                                         DeclContext *DC,
                                         SourceLocation StartLoc,
                                         SourceLocation IdLoc,
                                         ClassTemplateDecl *SpecializedTemplate,
                                         ArrayRef<TemplateArgument> Args,

 VarTemplateSpecializationDecl::VarTemplateSpecializationDecl(
     Kind DK, ASTContext &Context, DeclContext *DC, SourceLocation StartLoc,
     SourceLocation IdLoc, VarTemplateDecl *SpecializedTemplate, QualType T,
     TypeSourceInfo *TInfo, StorageClass S, ArrayRef<TemplateArgument> Args)
     : VarDecl(DK, Context, DC, StartLoc, IdLoc,
               SpecializedTemplate->getIdentifier(), T, TInfo, S),
       SpecializedTemplate(SpecializedTemplate),
       TemplateArgs(TemplateArgumentList::CreateCopy(Context, Args)),
-      SpecializationKind(TSK_Undeclared), IsCompleteDefinition(false) {}
+      SpecializationKind(TSK_Undeclared), IsCompleteDefinition(false),
+      IsSpecializedMember(0) {}
 
 VarTemplateSpecializationDecl::VarTemplateSpecializationDecl(Kind DK,
                                                              ASTContext &C)
     : VarDecl(DK, C, nullptr, SourceLocation(), SourceLocation(), nullptr,
               QualType(), nullptr, SC_None),
-      SpecializationKind(TSK_Undeclared), IsCompleteDefinition(false) {}
+      SpecializationKind(TSK_Undeclared), IsCompleteDefinition(false),
+      IsSpecializedMember(0) {}
 
 VarTemplateSpecializationDecl *VarTemplateSpecializationDecl::Create(
     ASTContext &Context, DeclContext *DC, SourceLocation StartLoc,
     SourceLocation IdLoc, VarTemplateDecl *SpecializedTemplate, QualType T,
     TypeSourceInfo *TInfo, StorageClass S, ArrayRef<TemplateArgument> Args) {
   return new (Context, DC) VarTemplateSpecializationDecl(
       VarTemplateSpecialization, Context, DC, StartLoc, IdLoc,
       SpecializedTemplate, T, TInfo, S, Args);

clang/lib/Sema/SemaTemplate.cpp

         Context, VarTemplate->getDeclContext(), TemplateKWLoc, TemplateNameLoc,
         VarTemplate, DI->getType(), DI, SC, Converted);
     Specialization->setTemplateArgsInfo(TemplateArgs);
 
     if (!PrevDecl)
       VarTemplate->AddSpecialization(Specialization, InsertPos);
   }
 
+  // Record that this specialization was declared at class scope.
+  if (!CurContext->getRedeclContext()->isFileContext())
+    Specialization->setIsSpecializedMember();
+
   // C++ [temp.expl.spec]p6:
   //   If a template, a member template or the member of a class template is
   //   explicitly specialized then that specialization shall be declared
   //   before the first use of that specialization that would cause an implicit
   //   instantiation to take place, in every translation unit in which such a
   //   use occurs; no diagnostic is required.
   if (PrevDecl && PrevDecl->getPointOfInstantiation().isValid()) {
     bool Okay = false;

 
     if (CurContext->isDependentContext()) {
       TemplateName CanonTemplate = Context.getCanonicalTemplateName(Name);
       CanonType = Context.getTemplateSpecializationType(
           CanonTemplate, Converted);
     } else {
       CanonType = Context.getTypeDeclType(Specialization);
     }
+
   }
 
+  // Record that this specialization was declared at class scope.
+  if (!CurContext->getRedeclContext()->isFileContext())
+    Specialization->setIsSpecializedMember();
+
   // C++ [temp.expl.spec]p6:
   //   If a template, a member template or the member of a class template is
   //   explicitly specialized then that specialization shall be declared
   //   before the first use of that specialization that would cause an implicit
   //   instantiation to take place, in every translation unit in which such a
   //   use occurs; no diagnostic is required.
   if (PrevDecl && PrevDecl->getPointOfInstantiation().isValid()) {
     bool Okay = false;

clang/lib/Sema/SemaTemplateInstantiate.cpp

Sema::getTemplateInstantiationArgs(NamedDecl *D,
   DeclContext *Ctx = dyn_cast<DeclContext>(D);
   if (!Ctx) {
     Ctx = D->getDeclContext();
 
     // Add template arguments from a variable template instantiation.
     if (VarTemplateSpecializationDecl *Spec =
             dyn_cast<VarTemplateSpecializationDecl>(D)) {
       // We're done when we hit an explicit specialization.
-      if (Spec->getSpecializationKind() == TSK_ExplicitSpecialization &&
-          !isa<VarTemplatePartialSpecializationDecl>(Spec))
-        return Result;
-
+      if (Spec->getSpecializationKind() != TSK_ExplicitSpecialization ||
+          isa<VarTemplatePartialSpecializationDecl>(Spec)) {
         Result.addOuterTemplateArguments(&Spec->getTemplateInstantiationArgs());
 
         // If this variable template specialization was instantiated from a
         // specialized member that is a variable template, we're done.
         assert(Spec->getSpecializedTemplate() && "No variable template?");
         llvm::PointerUnion<VarTemplateDecl*,
                            VarTemplatePartialSpecializationDecl*> Specialized
-                             = Spec->getSpecializedTemplateOrPartial();
         if (VarTemplatePartialSpecializationDecl *Partial =
-              Specialized.dyn_cast<VarTemplatePartialSpecializationDecl *>()) {
           if (Partial->isMemberSpecialization())
             return Result;
         } else {
           VarTemplateDecl *Tmpl = Specialized.get<VarTemplateDecl *>();
           if (Tmpl->isMemberSpecialization())
             return Result;
         }
+      } else if (!Spec->isSpecializedMember())
+        return Result;
     }
 
     // If we have a template template parameter with translation unit context,
     // then we're performing substitution into a default template argument of
     // this template template parameter before we've constructed the template
     // that will own this template template parameter. In this case, we
     // use empty template parameter lists for all of the outer templates
     // to avoid performing any substitutions.
     if (Ctx->isTranslationUnit()) {
       if (TemplateTemplateParmDecl *TTP
                                       = dyn_cast<TemplateTemplateParmDecl>(D)) {
         for (unsigned I = 0, N = TTP->getDepth() + 1; I != N; ++I)
           Result.addOuterTemplateArguments(None);
         return Result;
       }
     }
   }
 
   while (!Ctx->isFileContext()) {
     // Add template arguments from a class template instantiation.
     if (ClassTemplateSpecializationDecl *Spec
           = dyn_cast<ClassTemplateSpecializationDecl>(Ctx)) {
-      // We're done when we hit an explicit specialization.
+      // We're done when we hit an explicit specialization ...
       if (Spec->getSpecializationKind() == TSK_ExplicitSpecialization &&
-          !isa<ClassTemplatePartialSpecializationDecl>(Spec))
+          !isa<ClassTemplatePartialSpecializationDecl>(Spec)) {
+        // ... unless this explicit specialization is a member of a template
+        // (DR727), in which case we need to skip past it to gather the
+        // enclosing template argument lists.
+        if (Spec->isSpecializedMember()) {
+          Ctx = Spec->getDeclContext();
+          RelativeToPrimary = false;
+          continue;
+        }
         break;
+      }
 
       Result.addOuterTemplateArguments(&Spec->getTemplateInstantiationArgs());
 
       // If this class template specialization was instantiated from a
       // specialized member that is a class template, we're done.
       assert(Spec->getSpecializedTemplate() && "No class template?");
       if (Spec->getSpecializedTemplate()->isMemberSpecialization())
         break;
     }
     // Add template arguments from a function template specialization.
     else if (FunctionDecl *Function = dyn_cast<FunctionDecl>(Ctx)) {
       if (!RelativeToPrimary &&
           (Function->getTemplateSpecializationKind() ==
                                                   TSK_ExplicitSpecialization &&
            !Function->getClassScopeSpecializationPattern()))
         break;
 
       if (const TemplateArgumentList *TemplateArgs
             = Function->getTemplateSpecializationArgs()) {
         // Add the template arguments for this specialization.
         Result.addOuterTemplateArguments(TemplateArgs);
 
         // If this function was instantiated from a specialized member that is

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

       CanonType);
 
   InstD->setAccess(D->getAccess());
   InstD->setInstantiationOfMemberClass(D, TSK_ImplicitInstantiation);
   InstD->setSpecializationKind(D->getSpecializationKind());
   InstD->setTypeAsWritten(WrittenTy);
   InstD->setExternLoc(D->getExternLoc());
   InstD->setTemplateKeywordLoc(D->getTemplateKeywordLoc());
+  InstD->setIsSpecializedMember(D->isSpecializedMember());
 
   Owner->addDecl(InstD);
 
   // Instantiate the members of the class-scope explicit specialization eagerly.
   // We don't have support for lazy instantiation of an explicit specialization
   // yet, and MSVC eagerly instantiates in this case.
   if (D->isThisDeclarationADefinition() &&
       SemaRef.InstantiateClass(D->getLocation(), InstD, D, TemplateArgs,

clang/lib/Serialization/ASTReaderDecl.cpp

     }
   }
 
   SmallVector<TemplateArgument, 8> TemplArgs;
   Record.readTemplateArgumentList(TemplArgs, /*Canonicalize*/ true);
   D->TemplateArgs = TemplateArgumentList::CreateCopy(C, TemplArgs);
   D->PointOfInstantiation = ReadSourceLocation();
   D->SpecializationKind = (TemplateSpecializationKind)Record.readInt();
+  D->IsSpecializedMember = Record.readInt();
 
   bool writtenAsCanonicalDecl = Record.readInt();
   if (writtenAsCanonicalDecl) {
     auto *CanonPattern = ReadDeclAs<ClassTemplateDecl>();
     if (D->isCanonicalDecl()) { // It's kept in the folding set.
       // Set this as, or find, the canonical declaration for this specialization
       ClassTemplateSpecializationDecl *CanonSpec;
       if (auto *Partial = dyn_cast<ClassTemplatePartialSpecializationDecl>(D)) {

   }
 
   SmallVector<TemplateArgument, 8> TemplArgs;
   Record.readTemplateArgumentList(TemplArgs, /*Canonicalize*/ true);
   D->TemplateArgs = TemplateArgumentList::CreateCopy(C, TemplArgs);
   D->PointOfInstantiation = ReadSourceLocation();
   D->SpecializationKind = (TemplateSpecializationKind)Record.readInt();
   D->IsCompleteDefinition = Record.readInt();
+  D->IsSpecializedMember = Record.readInt();
 
   bool writtenAsCanonicalDecl = Record.readInt();
   if (writtenAsCanonicalDecl) {
     auto *CanonPattern = ReadDeclAs<VarTemplateDecl>();
     if (D->isCanonicalDecl()) { // It's kept in the folding set.
       // FIXME: If it's already present, merge it.
       if (auto *Partial = dyn_cast<VarTemplatePartialSpecializationDecl>(D)) {
         CanonPattern->getCommonPtr()->PartialSpecializations

clang/lib/Serialization/ASTWriterDecl.cpp

   } else {
     Record.AddDeclRef(InstFrom.get<ClassTemplatePartialSpecializationDecl *>());
     Record.AddTemplateArgumentList(&D->getTemplateInstantiationArgs());
   }
 
   Record.AddTemplateArgumentList(&D->getTemplateArgs());
   Record.AddSourceLocation(D->getPointOfInstantiation());
   Record.push_back(D->getSpecializationKind());
+  Record.push_back(D->isSpecializedMember());
   Record.push_back(D->isCanonicalDecl());
 
   if (D->isCanonicalDecl()) {
     // When reading, we'll add it to the folding set of the following template.
     Record.AddDeclRef(D->getSpecializedTemplate()->getCanonicalDecl());
   }
 
   // Explicit info.

     Record.AddSourceLocation(D->getExternLoc());
     Record.AddSourceLocation(D->getTemplateKeywordLoc());
   }
 
   Record.AddTemplateArgumentList(&D->getTemplateArgs());
   Record.AddSourceLocation(D->getPointOfInstantiation());
   Record.push_back(D->getSpecializationKind());
   Record.push_back(D->IsCompleteDefinition);
+  Record.push_back(D->isSpecializedMember());
   Record.push_back(D->isCanonicalDecl());
 
   if (D->isCanonicalDecl()) {
     // When reading, we'll add it to the folding set of the following template.
     Record.AddDeclRef(D->getSpecializedTemplate()->getCanonicalDecl());
   }
 
   Code = serialization::DECL_VAR_TEMPLATE_SPECIALIZATION;

clang/test/SemaCXX/member-spec-dr727.cpp

+// RUN: %clang_cc1 -std=c++17 -verify %s -emit-llvm -o -
+
+// expected-no-diagnostics
+
+namespace PR39031 {
+template <class>
+struct S {
+  template <class>
+  struct Wrapper;
+
+  template <>
+  struct Wrapper<int> {
+    template <class T>
+    void f(T) {
+      T x;
+    }
+  };
+};
+
+void Run() {
+  S<int>::Wrapper<int>().f(1);
+}
+} // namespace PR39031
+
+template <class A, class B> struct is_same { static constexpr bool value = false; };
+template <class A> struct is_same<A, A> { static constexpr bool value = true; };
+
+namespace t1 {
+template <class Ap> struct A {
+  template <class Bp> struct B;
+
+  template <> struct B<short> {
+    template <class Cp> struct C {
+      static_assert(is_same<Cp, int>::value);
+      static_assert(is_same<Ap, char>::value);
+    };
+  };
+};
+
+A<char>::B<short>::C<int> x;
+}
+
+namespace t2 {
+template <class Ap>
+struct A {
+  template <class Bp> static constexpr int p = 0;
+  template <> static constexpr int p<int> = 1;
+};
+
+// FIXME: why aren't we selecting the specialization here?
+static_assert(A<char>::p<int> == 0);
+}