[clang] Redeclare function templates instances per primary template

mizvekov · mizvekov · commit 362444a8fbb1 · 2024-09-28T18:57:32.000-03:00
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
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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

Original file line number	Diff line number	Diff line change
`@@ -491,6 +491,9 @@ bool Sema::MergeCXXFunctionDecl(FunctionDecl New, FunctionDecl Old,`
`491`	`491`	`continue;`
`492`	`492`	`}`
`493`	`493`
	`494`	`+ if (PrevForDefaultArgs->getPrimaryTemplate() != New->getPrimaryTemplate())`
	`495`	`+ continue;`
	`496`	`+`
`494`	`497`	`// We found the right previous declaration.`
`495`	`498`	`break;`
`496`	`499`	`}`