[Clang] Fix handling of pack indexing types in constraints of redeclaration

[cor3ntin]

When checking the constraints of a potential redeclaration we inject the template parameter as template arguments to compare them.

This would make clang:

• try to expand these injected parameters (which are treated as packs of size 1)
• inject a SubstTemplateTypeParmPackType

Why expanding the pack indexing is wasteful, it's recoverable, as it was only partially expanded.

However, creating SubstTemplateTypeParmPackType nodes was problematic as, before we can establish that declarations are identical, these nodes won't canonicalize to the same types (at this point they have unrelated associated decl).

To avoid these issues, we track whether a set of template arguments are injected template parameters. And if they are, skip some substitutions/instantiations.

Cleanup pack indexing profiling as a drive-by.

Fixes #138255

[llvmbot]

@llvm/pr-subscribers-clang-modules

@llvm/pr-subscribers-clang

Author: cor3ntin (cor3ntin)

Changes

When checking the constraints of a potential redeclaration we inject the template parameter as template arguments to compare them.

This would make clang:

• try to expand these injected parameters (which are treated as packs of size 1)
• inject a SubstTemplateTypeParmPackType

Why expanding the pack indexing is wasteful, it's recoverable, as it was only partially expanded.

However, creating SubstTemplateTypeParmPackType nodes was problematic as, before we can establish that declarations are identical, these nodes won't canonicalize to the same types (at this point they have unrelated associated decl).

To avoid these issues, we track whether a set of template arguments are injected template parameters. And if they are, skip some substitutions/instantiations.

Cleanup pack indexing profiling as a drive-by.

Fixes #138255

---

Patch is 20.19 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/139057.diff

10 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+1)
• (modified) clang/include/clang/AST/ASTContext.h (+2-1)
• (modified) clang/include/clang/AST/Type.h (+3-10)
• (modified) clang/include/clang/Sema/Template.h (+41-10)
• (modified) clang/lib/AST/ASTContext.cpp (+19-24)
• (modified) clang/lib/AST/Type.cpp (+12-4)
• (modified) clang/lib/Sema/SemaTemplateInstantiate.cpp (+26-11)
• (modified) clang/lib/Sema/SemaTemplateVariadic.cpp (+7)
• (modified) clang/lib/Sema/TreeTransform.h (+1-1)
• (modified) clang/test/SemaTemplate/concepts-out-of-line-def.cpp (+59-1)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c52e285bde627..938d9578bae61 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -667,6 +667,7 @@ Bug Fixes to C++ Support
   whose type depends on itself. (#GH51347), (#GH55872)
 - Improved parser recovery of invalid requirement expressions. In turn, this
   fixes crashes from follow-on processing of the invalid requirement. (#GH138820)
+- Fixed the handling of pack indexing types in the constraints of a member function redeclaration. (#GH138255)
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index e107db458742e..1fdc488a76507 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -221,7 +221,8 @@ class ASTContext : public RefCountedBase<ASTContext> {
   mutable llvm::ContextualFoldingSet<DependentDecltypeType, ASTContext &>
       DependentDecltypeTypes;
 
-  mutable llvm::FoldingSet<PackIndexingType> DependentPackIndexingTypes;
+  mutable llvm::ContextualFoldingSet<PackIndexingType, ASTContext &>
+      DependentPackIndexingTypes;
 
   mutable llvm::FoldingSet<TemplateTypeParmType> TemplateTypeParmTypes;
   mutable llvm::FoldingSet<ObjCTypeParamType> ObjCTypeParamTypes;
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 773796a55eaa1..ba723e7e5b5b1 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -5976,7 +5976,6 @@ class PackIndexingType final
       private llvm::TrailingObjects<PackIndexingType, QualType> {
   friend TrailingObjects;
 
-  const ASTContext &Context;
   QualType Pattern;
   Expr *IndexExpr;
 
@@ -5987,9 +5986,8 @@ class PackIndexingType final
 
 protected:
   friend class ASTContext; // ASTContext creates these.
-  PackIndexingType(const ASTContext &Context, QualType Canonical,
-                   QualType Pattern, Expr *IndexExpr, bool FullySubstituted,
-                   ArrayRef<QualType> Expansions = {});
+  PackIndexingType(QualType Canonical, QualType Pattern, Expr *IndexExpr,
+                   bool FullySubstituted, ArrayRef<QualType> Expansions = {});
 
 public:
   Expr *getIndexExpr() const { return IndexExpr; }
@@ -6024,12 +6022,7 @@ class PackIndexingType final
     return T->getTypeClass() == PackIndexing;
   }
 
-  void Profile(llvm::FoldingSetNodeID &ID) {
-    if (hasSelectedType())
-      getSelectedType().Profile(ID);
-    else
-      Profile(ID, Context, getPattern(), getIndexExpr(), isFullySubstituted());
-  }
+  void Profile(llvm::FoldingSetNodeID &ID, const ASTContext &Context);
   static void Profile(llvm::FoldingSetNodeID &ID, const ASTContext &Context,
                       QualType Pattern, Expr *E, bool FullySubstituted);
 
diff --git a/clang/include/clang/Sema/Template.h b/clang/include/clang/Sema/Template.h
index f9a10cfafb1f7..2da91d16298c2 100644
--- a/clang/include/clang/Sema/Template.h
+++ b/clang/include/clang/Sema/Template.h
@@ -24,6 +24,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include <cassert>
 #include <optional>
+#include <tuple>
 #include <utility>
 
 namespace clang {
@@ -76,9 +77,18 @@ enum class TemplateSubstitutionKind : char {
   class MultiLevelTemplateArgumentList {
     /// The template argument list at a certain template depth
 
+    enum ListProperties {
+      /// A 'Final' substitution means that Subst* nodes won't be built
+      /// for the replacements.
+      IsFinal = 0x1,
+      /// Track if the arguments are injected template parameters.
+      /// Injected template parameters should not be expanded.
+      ArgumentsAreInjectedTemplateParams = 0x02,
+    };
+
     using ArgList = ArrayRef<TemplateArgument>;
     struct ArgumentListLevel {
-      llvm::PointerIntPair<Decl *, 1, bool> AssociatedDeclAndFinal;
+      llvm::PointerIntPair<Decl *, 3, unsigned> AssociatedDeclAndProperties;
       ArgList Args;
     };
     using ContainerType = SmallVector<ArgumentListLevel, 4>;
@@ -161,11 +171,19 @@ enum class TemplateSubstitutionKind : char {
     /// A template-like entity which owns the whole pattern being substituted.
     /// This will usually own a set of template parameters, or in some
     /// cases might even be a template parameter itself.
-    std::pair<Decl *, bool> getAssociatedDecl(unsigned Depth) const {
+    std::tuple<Decl *, bool, bool> getAssociatedDecl(unsigned Depth) const {
+      assert(NumRetainedOuterLevels <= Depth && Depth < getNumLevels());
+      auto AD = TemplateArgumentLists[getNumLevels() - Depth - 1]
+                    .AssociatedDeclAndProperties;
+      return {AD.getPointer(), AD.getInt() & IsFinal,
+              AD.getInt() & ArgumentsAreInjectedTemplateParams};
+    }
+
+    bool ArgumentsAreInjectedParameters(unsigned Depth) const {
       assert(NumRetainedOuterLevels <= Depth && Depth < getNumLevels());
       auto AD = TemplateArgumentLists[getNumLevels() - Depth - 1]
-                    .AssociatedDeclAndFinal;
-      return {AD.getPointer(), AD.getInt()};
+                    .AssociatedDeclAndProperties;
+      return AD.getInt() & ArgumentsAreInjectedTemplateParams;
     }
 
     /// Determine whether there is a non-NULL template argument at the
@@ -207,14 +225,15 @@ enum class TemplateSubstitutionKind : char {
     /// list.
     /// A 'Final' substitution means that Subst* nodes won't be built
     /// for the replacements.
-    void addOuterTemplateArguments(Decl *AssociatedDecl, ArgList Args,
-                                   bool Final) {
+    void
+    addOuterTemplateArguments(Decl *AssociatedDecl, ArgList Args, bool Final,
+                              bool ArgumentsAreInjectedTemplateParams = false) {
       assert(!NumRetainedOuterLevels &&
              "substituted args outside retained args?");
       assert(getKind() == TemplateSubstitutionKind::Specialization);
       TemplateArgumentLists.push_back(
           {{AssociatedDecl ? AssociatedDecl->getCanonicalDecl() : nullptr,
-            Final},
+            getProperties(Final, ArgumentsAreInjectedTemplateParams)},
            Args});
     }
 
@@ -239,15 +258,17 @@ enum class TemplateSubstitutionKind : char {
              "Replacing in an empty list?");
 
       if (!TemplateArgumentLists.empty()) {
-        assert((TemplateArgumentLists[0].AssociatedDeclAndFinal.getPointer() ||
-                TemplateArgumentLists[0].AssociatedDeclAndFinal.getPointer() ==
+        assert((TemplateArgumentLists[0]
+                    .AssociatedDeclAndProperties.getPointer() ||
+                TemplateArgumentLists[0]
+                        .AssociatedDeclAndProperties.getPointer() ==
                     AssociatedDecl) &&
                "Trying to change incorrect declaration?");
         TemplateArgumentLists[0].Args = Args;
       } else {
         --NumRetainedOuterLevels;
         TemplateArgumentLists.push_back(
-            {{AssociatedDecl, /*Final=*/false}, Args});
+            {{AssociatedDecl, /*Properties=*/0}, Args});
       }
     }
 
@@ -292,6 +313,16 @@ enum class TemplateSubstitutionKind : char {
         llvm::errs() << "\n";
       }
     }
+
+    static unsigned getProperties(bool IsFinal, bool IsInjected) {
+      unsigned Props = 0;
+      if (IsFinal)
+        Props |= MultiLevelTemplateArgumentList::IsFinal;
+      if (IsInjected)
+        Props |=
+            MultiLevelTemplateArgumentList::ArgumentsAreInjectedTemplateParams;
+      return Props;
+    }
   };
 
   /// The context in which partial ordering of function templates occurs.
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 1ed16748dff1a..6ebef70b2d44a 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -940,7 +940,7 @@ ASTContext::ASTContext(LangOptions &LOpts, SourceManager &SM,
       DependentSizedMatrixTypes(this_()),
       FunctionProtoTypes(this_(), FunctionProtoTypesLog2InitSize),
       DependentTypeOfExprTypes(this_()), DependentDecltypeTypes(this_()),
-      TemplateSpecializationTypes(this_()),
+      DependentPackIndexingTypes(this_()), TemplateSpecializationTypes(this_()),
       DependentTemplateSpecializationTypes(this_()),
       DependentBitIntTypes(this_()), SubstTemplateTemplateParmPacks(this_()),
       DeducedTemplates(this_()), ArrayParameterTypes(this_()),
@@ -6433,34 +6433,29 @@ QualType ASTContext::getPackIndexingType(QualType Pattern, Expr *IndexExpr,
                                          ArrayRef<QualType> Expansions,
                                          UnsignedOrNone Index) const {
   QualType Canonical;
-  if (FullySubstituted && Index) {
+  if (FullySubstituted && Index)
     Canonical = getCanonicalType(Expansions[*Index]);
-  } else {
-    llvm::FoldingSetNodeID ID;
-    PackIndexingType::Profile(ID, *this, Pattern.getCanonicalType(), IndexExpr,
-                              FullySubstituted);
-    void *InsertPos = nullptr;
-    PackIndexingType *Canon =
-        DependentPackIndexingTypes.FindNodeOrInsertPos(ID, InsertPos);
-    if (!Canon) {
-      void *Mem = Allocate(
-          PackIndexingType::totalSizeToAlloc<QualType>(Expansions.size()),
-          TypeAlignment);
-      Canon = new (Mem)
-          PackIndexingType(*this, QualType(), Pattern.getCanonicalType(),
-                           IndexExpr, FullySubstituted, Expansions);
-      DependentPackIndexingTypes.InsertNode(Canon, InsertPos);
-    }
-    Canonical = QualType(Canon, 0);
-  }
+  else if (!Pattern.isCanonical())
+    Canonical = getPackIndexingType(Pattern.getCanonicalType(), IndexExpr,
+                                    FullySubstituted, Expansions, Index);
+
+  llvm::FoldingSetNodeID ID;
+  PackIndexingType::Profile(ID, *this, Pattern, IndexExpr, FullySubstituted);
+  void *InsertPos = nullptr;
+  PackIndexingType *Canon =
+      DependentPackIndexingTypes.FindNodeOrInsertPos(ID, InsertPos);
+  if (Canon)
+    return QualType(Canon, 0);
 
   void *Mem =
       Allocate(PackIndexingType::totalSizeToAlloc<QualType>(Expansions.size()),
                TypeAlignment);
-  auto *T = new (Mem) PackIndexingType(*this, Canonical, Pattern, IndexExpr,
-                                       FullySubstituted, Expansions);
-  Types.push_back(T);
-  return QualType(T, 0);
+
+  Canon = new (Mem) PackIndexingType(Canonical, Pattern, IndexExpr,
+                                     FullySubstituted, Expansions);
+  DependentPackIndexingTypes.InsertNode(Canon, InsertPos);
+  Types.push_back(Canon);
+  return QualType(Canon, 0);
 }
 
 /// getUnaryTransformationType - We don't unique these, since the memory
diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index 31e4bcd7535ea..bb4fe24cf94d1 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -4126,14 +4126,14 @@ void DependentDecltypeType::Profile(llvm::FoldingSetNodeID &ID,
   E->Profile(ID, Context, true);
 }
 
-PackIndexingType::PackIndexingType(const ASTContext &Context,
-                                   QualType Canonical, QualType Pattern,
+PackIndexingType::PackIndexingType(QualType Canonical, QualType Pattern,
                                    Expr *IndexExpr, bool FullySubstituted,
                                    ArrayRef<QualType> Expansions)
     : Type(PackIndexing, Canonical,
            computeDependence(Pattern, IndexExpr, Expansions)),
-      Context(Context), Pattern(Pattern), IndexExpr(IndexExpr),
-      Size(Expansions.size()), FullySubstituted(FullySubstituted) {
+      Pattern(Pattern), IndexExpr(IndexExpr), Size(Expansions.size()),
+      FullySubstituted(FullySubstituted) {
+
   llvm::uninitialized_copy(Expansions, getTrailingObjects<QualType>());
 }
 
@@ -4174,6 +4174,14 @@ PackIndexingType::computeDependence(QualType Pattern, Expr *IndexExpr,
   return TD;
 }
 
+void PackIndexingType::Profile(llvm::FoldingSetNodeID &ID,
+                               const ASTContext &Context) {
+  if (hasSelectedType() && isFullySubstituted())
+    getSelectedType().Profile(ID);
+  else
+    Profile(ID, Context, getPattern(), getIndexExpr(), isFullySubstituted());
+}
+
 void PackIndexingType::Profile(llvm::FoldingSetNodeID &ID,
                                const ASTContext &Context, QualType Pattern,
                                Expr *E, bool FullySubstituted) {
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index fb490bcac6e91..75f42e94cf323 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -338,7 +338,7 @@ Response HandleFunctionTemplateDecl(Sema &SemaRef,
         const_cast<FunctionTemplateDecl *>(FTD),
         const_cast<FunctionTemplateDecl *>(FTD)->getInjectedTemplateArgs(
             SemaRef.Context),
-        /*Final=*/false);
+        /*Final=*/false, /*InjectedTemplateParams=*/true);
 
     NestedNameSpecifier *NNS = FTD->getTemplatedDecl()->getQualifier();
 
@@ -363,18 +363,20 @@ Response HandleFunctionTemplateDecl(Sema &SemaRef,
           // This meets the contract in
           // TreeTransform::TryExpandParameterPacks that the template arguments
           // for unexpanded parameters should be of a Pack kind.
+          bool Injected = false;
           if (TSTy->isCurrentInstantiation()) {
             auto *RD = TSTy->getCanonicalTypeInternal()->getAsCXXRecordDecl();
-            if (ClassTemplateDecl *CTD = RD->getDescribedClassTemplate())
+            if (ClassTemplateDecl *CTD = RD->getDescribedClassTemplate()) {
               Arguments = CTD->getInjectedTemplateArgs(SemaRef.Context);
-            else if (auto *Specialization =
-                         dyn_cast<ClassTemplateSpecializationDecl>(RD))
+              Injected = true;
+            } else if (auto *Specialization =
+                           dyn_cast<ClassTemplateSpecializationDecl>(RD))
               Arguments =
                   Specialization->getTemplateInstantiationArgs().asArray();
           }
           Result.addOuterTemplateArguments(
               TSTy->getTemplateName().getAsTemplateDecl(), Arguments,
-              /*Final=*/false);
+              /*Final=*/false, /*InjectedTemplateParams=*/Injected);
         }
       }
 
@@ -399,7 +401,7 @@ Response HandleRecordDecl(Sema &SemaRef, const CXXRecordDecl *Rec,
       Result.addOuterTemplateArguments(
           const_cast<CXXRecordDecl *>(Rec),
           ClassTemplate->getInjectedTemplateArgs(SemaRef.Context),
-          /*Final=*/false);
+          /*Final=*/false, /*InjectedTemplateParams=*/true);
   }
 
   if (const MemberSpecializationInfo *MSInfo =
@@ -1902,7 +1904,9 @@ Decl *TemplateInstantiator::TransformDecl(SourceLocation Loc, Decl *D) {
 
       TemplateArgument Arg = TemplateArgs(TTP->getDepth(), TTP->getPosition());
 
-      if (TTP->isParameterPack()) {
+      if (TemplateArgs.ArgumentsAreInjectedParameters(TTP->getDepth())) {
+        Arg = getTemplateArgumentPackPatternForRewrite(Arg);
+      } else if (TTP->isParameterPack()) {
         assert(Arg.getKind() == TemplateArgument::Pack &&
                "Missing argument pack");
         Arg = getPackSubstitutedTemplateArgument(getSema(), Arg);
@@ -2060,14 +2064,17 @@ TemplateName TemplateInstantiator::TransformTemplateName(
         return Arg.getAsTemplate();
       }
 
-      auto [AssociatedDecl, Final] =
+      auto [AssociatedDecl, Final, ArgumentsAreInjectedTemplateParams] =
           TemplateArgs.getAssociatedDecl(TTP->getDepth());
       UnsignedOrNone PackIndex = std::nullopt;
       if (TTP->isParameterPack()) {
         assert(Arg.getKind() == TemplateArgument::Pack &&
                "Missing argument pack");
 
-        if (!getSema().ArgPackSubstIndex) {
+        if (ArgumentsAreInjectedTemplateParams)
+          Arg = getTemplateArgumentPackPatternForRewrite(Arg);
+
+        else if (!getSema().ArgPackSubstIndex) {
           // We have the template argument pack to substitute, but we're not
           // actually expanding the enclosing pack expansion yet. So, just
           // keep the entire argument pack.
@@ -2139,7 +2146,7 @@ TemplateInstantiator::TransformTemplateParmRefExpr(DeclRefExpr *E,
     return Arg.getAsExpr();
   }
 
-  auto [AssociatedDecl, Final] =
+  auto [AssociatedDecl, Final, ArgumentsAreInjectedTemplateParams] =
       TemplateArgs.getAssociatedDecl(NTTP->getDepth());
   UnsignedOrNone PackIndex = std::nullopt;
   if (NTTP->isParameterPack()) {
@@ -2592,7 +2599,7 @@ TemplateInstantiator::TransformTemplateTypeParmType(TypeLocBuilder &TLB,
       return NewT;
     }
 
-    auto [AssociatedDecl, Final] =
+    auto [AssociatedDecl, Final, ArgumentsAreInjectedTemplateParams] =
         TemplateArgs.getAssociatedDecl(T->getDepth());
     UnsignedOrNone PackIndex = std::nullopt;
     if (T->isParameterPack()) {
@@ -2600,6 +2607,14 @@ TemplateInstantiator::TransformTemplateTypeParmType(TypeLocBuilder &TLB,
              "Missing argument pack");
 
       if (!getSema().ArgPackSubstIndex) {
+
+        if (ArgumentsAreInjectedTemplateParams) {
+          TemplateTypeParmTypeLoc NewTL =
+              TLB.push<TemplateTypeParmTypeLoc>(TL.getType());
+          NewTL.setNameLoc(TL.getNameLoc());
+          return TL.getType();
+        }
+
         // We have the template argument pack, but we're not expanding the
         // enclosing pack expansion yet. Just save the template argument
         // pack for later substitution.
diff --git a/clang/lib/Sema/SemaTemplateVariadic.cpp b/clang/lib/Sema/SemaTemplateVariadic.cpp
index 5f0e968ff18c4..ff2f6f94d1b54 100644
--- a/clang/lib/Sema/SemaTemplateVariadic.cpp
+++ b/clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -823,6 +823,13 @@ bool Sema::CheckParameterPacksForExpansion(
         continue;
       }
 
+      // Template arguments that are injected template parameters
+      // Cannot be expanded, even if they constitute a pack expansion.
+      if (TemplateArgs.ArgumentsAreInjectedParameters(Depth)) {
+        ShouldExpand = false;
+        continue;
+      }
+
       // Determine the size of the argument pack.
       ArrayRef<TemplateArgument> Pack =
           TemplateArgs(Depth, Index).getPackAsArray();
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 8b4b79c6ec039..317e8351f1d50 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -6884,7 +6884,7 @@ TreeTransform<Derived>::TransformPackIndexingType(TypeLocBuilder &TLB,
     assert(!Unexpanded.empty() && "Pack expansion without parameter packs?");
     // Determine whether the set of unexpanded parameter packs can and should
     // be expanded.
-    bool ShouldExpand = true;
+    bool ShouldExpand = false;
     bool RetainExpansion = false;
     UnsignedOrNone NumExpansions = std::nullopt;
     if (getDerived().TryExpandParameterPacks(TL.getEllipsisLoc(), SourceRange(),
diff --git a/clang/test/SemaTemplate/concepts-out-of-line-def.cpp b/clang/test/SemaTemplate/concepts-out-of-line-def.cpp
index 5af4ec75cae90..191b40310e8be 100644
--- a/clang/test/SemaTemplate/concepts-out-of-line-def.cpp
+++ b/clang/test/SemaTemplate/concepts-out-of-line-def.cpp
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -std=c++20 -verify %s
+// RUN: %clang_cc1 -std=c++20 -Wno-c++26-extensions -verify %s
+// RUN: %clang_cc1 -std=c++2c -Wno-c++26-extensions -verify %s
+
 
 static constexpr int PRIMARY = 0;
 static constexpr int SPECIALIZATION_CONCEPT = 1;
@@ -779,3 +781,59 @@ template <typename T>
 consteval void S::mfn() requires (bool(&fn)) {}
 
 }
+
+
+namespace GH138255 {
+
+  template <typename... T>
+  concept C = true;
+
+  struct Func {
+      template<typename... Ts>
+      requires C<Ts...[0]>
+      static auto buggy() -> void;
+
+      template<typename... Ts>
+      requires C<Ts...[0]>
+      friend auto fr() -> void;
+
+      template<typename... Ts>
+      requires C<Ts...[0]>
+      friend auto fr2() -> void{}; // expected-note{{previous definition is here}}
+  };
+
+  template<typename... Ts>
+  requires C<Ts...[0]>
+  auto Func::buggy() -> void {}
+
+  template<typename... Ts>
+  requires C<Ts...[0]>
+  auto fr() -> void {}
+
+  template<typename... Ts>
+  requires C<Ts...[0]>
+  auto fr2() -> void {} // expected-error{{redefinition of 'fr2'}}
+
+
+  template <typename... Ts>
+  requires C<Ts...[0]>
+  struct Class;
+
+  template <typename... Ts>
+  requires C<Ts...[0]>
+  struct Class;
+
+
+  template <typename...>
+  struct TplClass {
+      template<typen...
[truncated]

[zyn0217]

Thanks for working on it. Will take a closer look later

[zyn0217]

This is not true because Final flag is placed on the Subst* nodes now

[zyn0217]

I think this LGTM in general, but please wait for some time in case @erichkeane @mizvekov have some ideas

[zyn0217]

maybe it needs a better name than getTemplateArgumentPackPatternForRewrite because it's extended to handle injected template arguments.

How about getTemplateArgumentPackPattern?

[zyn0217]

This should be no-op because we always set it to true in CheckParameterPacksForExpansion

[cor3ntin]

Yup, I just thought it was cleaner / more consistent

[cor3ntin]

Note that I tried #106585 - but it doesn't fix this issue (and the present patch does not address the tests cases of @sdkrystian's PR)

[erichkeane]

On its face, and thinking about it for ~10 minutes, this seems completely reasonable. I think @zyn0217 has some reasonable comments, so make him happy, and I am.

[mizvekov]

Pre-existing issue, but this confuses identity with canonicalization.

This is basically saying that all PackIndexTypes which are fully substituted and have identical selected types are also identical, which can't be right as they can also differ in other ways, such as the Index expression.

[zyn0217]

Suggested change

	TemplateArgument getTemplateArgumentorUnsubstitutedExpansionPattern(
	TemplateArgument getTemplateArgumentOrUnsubstitutedExpansionPattern(

[zyn0217]

one nit, else LG!

[cor3ntin]

After chatting with @zyn0217, we realized there was a better approach. #139276