[clang] Track function template instantiation from definition

[mizvekov]

This fixes instantiation of definition for friend function templates,
when the declaration found and the one containing the definition
have different template contexts.

In these cases, the the function declaration corresponding to the
definition is not available; it may not even be instantiated at all.

So this patch adds a bit which tracks which function template
declaration was instantiated from the member template.
It's used to find which primary template serves as a context
for the purpose of obtaining the template arguments needed
to instantiate the definition.

Fixes #55509

[llvmbot]

@llvm/pr-subscribers-clang-modules

@llvm/pr-subscribers-clang

Author: Matheus Izvekov (mizvekov)

Changes

This fixes handling of friend function templates instances when their template context changes, such as when a new friend declaration is introduced after an instance was already created from a previous declaration.

Instead of producing one function template instance per primary template, this patch makes it so clang produces one instance per primary template redeclaration, tracking this new instance as a redeclation of the previous instance.

Fixes #55509

---

Full diff: https://github.com/llvm/llvm-project/pull/110387.diff

7 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+1)
• (modified) clang/lib/Sema/SemaDecl.cpp (+13-6)
• (modified) clang/lib/Sema/SemaDeclCXX.cpp (+3)
• (modified) clang/lib/Sema/SemaTemplate.cpp (+13-10)
• (modified) clang/lib/Sema/SemaTemplateDeduction.cpp (+3-16)
• (modified) clang/lib/Sema/SemaTemplateInstantiateDecl.cpp (+12-3)
• (added) clang/test/SemaTemplate/GH55509.cpp (+84)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 2e9560f553d94f..d4c80054dfefde 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -446,6 +446,7 @@ Bug Fixes to C++ Support
 - Fixed an assertion failure in debug mode, and potential crashes in release mode, when
   diagnosing a failed cast caused indirectly by a failed implicit conversion to the type of the constructor parameter.
 - Fixed an assertion failure by adjusting integral to boolean vector conversions (#GH108326)
+- Clang is now better at keeping track of friend function template instance contexts. (#GH55509)
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 1bf0e800a36228..b3909c7c96d631 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -3150,7 +3150,13 @@ void Sema::mergeDeclAttributes(NamedDecl *New, Decl *Old,
 
   // Re-declaration cannot add abi_tag's.
   if (const auto *NewAbiTagAttr = New->getAttr<AbiTagAttr>()) {
-    if (const auto *OldAbiTagAttr = Old->getAttr<AbiTagAttr>()) {
+    const AbiTagAttr *OldAbiTagAttr = nullptr;
+    for (auto *D = Old; D; D = D->getPreviousDecl()) {
+      OldAbiTagAttr = D->getAttr<AbiTagAttr>();
+      if (OldAbiTagAttr)
+        break;
+    }
+    if (OldAbiTagAttr) {
       for (const auto &NewTag : NewAbiTagAttr->tags()) {
         if (!llvm::is_contained(OldAbiTagAttr->tags(), NewTag)) {
           Diag(NewAbiTagAttr->getLocation(),
@@ -12021,11 +12027,12 @@ bool Sema::CheckFunctionDeclaration(Scope *S, FunctionDecl *NewFD,
 
     } else {
       if (shouldLinkDependentDeclWithPrevious(NewFD, OldDecl)) {
-        auto *OldFD = cast<FunctionDecl>(OldDecl);
-        // This needs to happen first so that 'inline' propagates.
-        NewFD->setPreviousDeclaration(OldFD);
-        if (NewFD->isCXXClassMember())
-          NewFD->setAccess(OldFD->getAccess());
+        if (auto *OldFD = cast<FunctionDecl>(OldDecl); OldFD != NewFD) {
+          // This needs to happen first so that 'inline' propagates.
+          NewFD->setPreviousDeclaration(OldFD);
+          if (NewFD->isCXXClassMember())
+            NewFD->setAccess(OldFD->getAccess());
+        }
       }
     }
   } else if (!getLangOpts().CPlusPlus && MayNeedOverloadableChecks &&
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index d8cdfcf8c6ec05..aa881b535e4ee0 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -491,6 +491,9 @@ bool Sema::MergeCXXFunctionDecl(FunctionDecl *New, FunctionDecl *Old,
       continue;
     }
 
+    if (PrevForDefaultArgs->getPrimaryTemplate() != New->getPrimaryTemplate())
+      continue;
+
     // We found the right previous declaration.
     break;
   }
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 99423b01114cc6..6e94831bcb2d38 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -9272,16 +9272,19 @@ bool Sema::CheckFunctionTemplateSpecialization(
     MarkUnusedFileScopedDecl(Specialization);
   }
 
-  // Turn the given function declaration into a function template
-  // specialization, with the template arguments from the previous
-  // specialization.
-  // Take copies of (semantic and syntactic) template argument lists.
-  TemplateArgumentList *TemplArgs = TemplateArgumentList::CreateCopy(
-      Context, Specialization->getTemplateSpecializationArgs()->asArray());
-  FD->setFunctionTemplateSpecialization(
-      Specialization->getPrimaryTemplate(), TemplArgs, /*InsertPos=*/nullptr,
-      SpecInfo->getTemplateSpecializationKind(),
-      ExplicitTemplateArgs ? &ConvertedTemplateArgs[Specialization] : nullptr);
+  if (FD != Specialization) {
+    // Turn the given function declaration into a function template
+    // specialization, with the template arguments from the previous
+    // specialization.
+    // Take copies of (semantic and syntactic) template argument lists.
+    TemplateArgumentList *TemplArgs = TemplateArgumentList::CreateCopy(
+        Context, Specialization->getTemplateSpecializationArgs()->asArray());
+    FD->setFunctionTemplateSpecialization(
+        Specialization->getPrimaryTemplate(), TemplArgs, /*InsertPos=*/nullptr,
+        SpecInfo->getTemplateSpecializationKind(),
+        ExplicitTemplateArgs ? &ConvertedTemplateArgs[Specialization]
+                             : nullptr);
+  }
 
   // A function template specialization inherits the target attributes
   // of its template.  (We require the attributes explicitly in the
diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp b/clang/lib/Sema/SemaTemplateDeduction.cpp
index cc095ae67ac400..29d25947e919b7 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -3949,28 +3949,15 @@ TemplateDeductionResult Sema::FinishTemplateArgumentDeduction(
       TemplateArgumentList::CreateCopy(Context, CanonicalBuilder);
   Info.reset(SugaredDeducedArgumentList, CanonicalDeducedArgumentList);
 
+  FunctionTemplate = FunctionTemplate->getMostRecentDecl();
+
   // Substitute the deduced template arguments into the function template
   // declaration to produce the function template specialization.
   DeclContext *Owner = FunctionTemplate->getDeclContext();
   if (FunctionTemplate->getFriendObjectKind())
     Owner = FunctionTemplate->getLexicalDeclContext();
   FunctionDecl *FD = FunctionTemplate->getTemplatedDecl();
-  // additional check for inline friend,
-  // ```
-  //   template <class F1> int foo(F1 X);
-  //   template <int A1> struct A {
-  //     template <class F1> friend int foo(F1 X) { return A1; }
-  //   };
-  //   template struct A<1>;
-  //   int a = foo(1.0);
-  // ```
-  const FunctionDecl *FDFriend;
-  if (FD->getFriendObjectKind() == Decl::FriendObjectKind::FOK_None &&
-      FD->isDefined(FDFriend, /*CheckForPendingFriendDefinition*/ true) &&
-      FDFriend->getFriendObjectKind() != Decl::FriendObjectKind::FOK_None) {
-    FD = const_cast<FunctionDecl *>(FDFriend);
-    Owner = FD->getLexicalDeclContext();
-  }
+
   MultiLevelTemplateArgumentList SubstArgs(
       FunctionTemplate, CanonicalDeducedArgumentList->asArray(),
       /*Final=*/false);
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index c3cb9d5d8c2c3d..3427c9e7566dc4 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -2136,6 +2136,7 @@ static QualType adjustFunctionTypeForInstantiation(ASTContext &Context,
 Decl *TemplateDeclInstantiator::VisitFunctionDecl(
     FunctionDecl *D, TemplateParameterList *TemplateParams,
     RewriteKind FunctionRewriteKind) {
+  FunctionDecl *PrevFunc = nullptr;
   // Check whether there is already a function template specialization for
   // this declaration.
   FunctionTemplateDecl *FunctionTemplate = D->getDescribedFunctionTemplate();
@@ -2146,9 +2147,15 @@ Decl *TemplateDeclInstantiator::VisitFunctionDecl(
     FunctionDecl *SpecFunc
       = FunctionTemplate->findSpecialization(Innermost, InsertPos);
 
-    // If we already have a function template specialization, return it.
-    if (SpecFunc)
-      return SpecFunc;
+    if (SpecFunc) {
+      if (!SpecFunc->isTemplateInstantiation())
+        return SpecFunc;
+
+      for (auto *Redecl : SpecFunc->redecls())
+        if (Redecl->getPrimaryTemplate() == FunctionTemplate)
+          return Redecl;
+    }
+    PrevFunc = SpecFunc;
   }
 
   bool isFriend;
@@ -2342,6 +2349,8 @@ Decl *TemplateDeclInstantiator::VisitFunctionDecl(
                              : Sema::LookupOrdinaryName,
       D->isLocalExternDecl() ? RedeclarationKind::ForExternalRedeclaration
                              : SemaRef.forRedeclarationInCurContext());
+  if (PrevFunc)
+    Previous.addDecl(PrevFunc);
 
   if (DependentFunctionTemplateSpecializationInfo *DFTSI =
           D->getDependentSpecializationInfo()) {
diff --git a/clang/test/SemaTemplate/GH55509.cpp b/clang/test/SemaTemplate/GH55509.cpp
new file mode 100644
index 00000000000000..9edd164f1e2dc5
--- /dev/null
+++ b/clang/test/SemaTemplate/GH55509.cpp
@@ -0,0 +1,84 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++26 %s
+
+namespace t1 {
+  template<int N> struct A {
+    template<class C> friend auto cica(const A<N-1>&, C) {
+      return N;
+    }
+  };
+
+  template<> struct A<0> {
+    template<class C> friend auto cica(const A<0>&, C);
+    // expected-note@-1 {{declared here}}
+  };
+
+  void test() {
+    cica(A<0>{}, 0);
+    // expected-error@-1 {{function 'cica<int>' with deduced return type cannot be used before it is defined}}
+
+    (void)A<1>{};
+    cica(A<0>{}, 0);
+  }
+} // namespace t1
+namespace t2 {
+  template<int N> struct A {
+    template<class C> friend auto cica(const A<N-1>&, C) {
+      return N;
+    }
+  };
+
+  template<> struct A<0> {
+    template<class C> friend auto cica(const A<0>&, C);
+  };
+
+  template <int N, class = decltype(cica(A<N>{}, nullptr))>
+  void MakeCica();
+  // expected-note@-1 {{candidate function}}
+
+  template <int N> void MakeCica(A<N+1> = {});
+  // expected-note@-1 {{candidate function}}
+
+  void test() {
+    MakeCica<0>();
+
+    MakeCica<0>();
+    // expected-error@-1 {{call to 'MakeCica' is ambiguous}}
+  }
+} // namespace t2
+namespace t3 {
+  template<int N> struct A {
+    template<class C> friend auto cica(const A<N-1>&, C) {
+      return N-1;
+    }
+  };
+
+  template<> struct A<0> {
+    template<class C> friend auto cica(const A<0>&, C);
+  };
+
+  template <int N, class AT, class = decltype(cica(AT{}, nullptr))>
+  static constexpr bool MakeCica(int);
+
+  template <int N, class AT>
+  static constexpr bool MakeCica(short, A<N+1> = {});
+
+  template <int N, class AT = A<N>, class Val = decltype(MakeCica<N, AT>(0))>
+  static constexpr bool has_cica = Val{};
+
+  constexpr bool cica2 = has_cica<0> || has_cica<0>;
+} // namespace t3
+namespace regression1 {
+  template <class> class A;
+
+  template <class T> [[gnu::abi_tag("TAG")]] void foo(A<T>);
+
+  template <class> struct A {
+    friend void foo <>(A);
+  };
+
+  template struct A<int>;
+
+  template <class T> [[gnu::abi_tag("TAG")]] void foo(A<T>) {}
+
+  template void foo<int>(A<int>);
+} // namespace regression1

[zyn0217]

Thanks!

[zyn0217]

Everything makes sense to me now, thank you!

[cor3ntin]

LGTM once @zyn0217 outstanding comments are resolved.

[mizvekov]

I pushed a new implementation for this fix.

I have updated the implementation of InstantiateFunctionDefinition so that it doesn't depend anymore on having the exact function declaration instantiation which has the same template context as the definition.

This avoids the more expensive aspect of the previous approach.
While keeping track of separate function declaration instantiations per primary template redeclaration increases source representation accuracy, It would be better to, in the future, go that way across that board for all templates, not just function templates, so that the impact can be better represented.

[zyn0217]

Thanks for taking another look at it.

My main concern is that changes in getTemplateInstantiationArgs() would conflict with @sdkrystian's recent work, so if these changes are necessary, I suggest we merge this one after the refactoring patch lands, WDYT?

[mizvekov]

Now this is rebased on top of the getTemplateInstantiationArgs refactoring.