[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

144

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

36

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

     return PointOfInstantiation;
   }
 
   void setPointOfInstantiation(SourceLocation Loc) {
     assert(Loc.isValid() && "point of instantiation must be valid!");
     PointOfInstantiation = Loc;
   }
 
+  /// If this class template specialization was declared at class scope, as
+  /// opposed to out-of-line (DR727). For example, G<int> below:
+  ///
+  /// \code
+  /// template <class T> struct S {
+  ///   template <class> struct G {};
+  ///   template <> struct G<int> {};
+  /// };
+  /// \endcode
+  bool isClassScopeSpecialization() const {
+    return !getFirstDecl()->isOutOfLine();
+  }
+
   /// 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()))

     ExplicitInfo->TemplateKeywordLoc = Loc;
   }
 
   /// Gets the location of the template keyword, if present.
   SourceLocation getTemplateKeywordLoc() const {
     return ExplicitInfo ? ExplicitInfo->TemplateKeywordLoc : SourceLocation();
   }
 
+  /// If this variable template specialization was declared at class scope, as
+  /// opposed to out-of-line (DR727). For example, v<int> below:
+  ///
+  /// \code
+  /// template <class T> struct S {
+  ///   template <class> static int v;
+  ///   template <> static int v<int>;
+  /// };
+  /// \endcode
+  bool isClassScopeSpecialization() const {
+    return !getFirstDecl()->isOutOfLine();
+  }
+
   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/Sema/SemaTemplateInstantiate.cpp

Sema::getTemplateInstantiationArgs(NamedDecl *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))
+          !isa<VarTemplatePartialSpecializationDecl>(Spec) &&
+          !Spec->isClassScopeSpecialization())
         return Result;
 
       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*,
if (Ctx->isTranslationUnit()) {
       }
     }
   }
 
   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->isClassScopeSpecialization()) {
+          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/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);
+}