[clang] [Clang][Concepts] Fix the constraint equivalence checking involving parameter packs (PR #102131)

[via cfe-commits]

================
@@ -972,8 +972,30 @@ static const Expr *SubstituteConstraintExpressionWithoutSatisfaction(
   // equivalence.
   LocalInstantiationScope ScopeForParameters(S);
   if (auto *FD = DeclInfo.getDecl()->getAsFunction())
-    for (auto *PVD : FD->parameters())
-      ScopeForParameters.InstantiatedLocal(PVD, PVD);
+    for (auto *PVD : FD->parameters()) {
+      if (!PVD->isParameterPack()) {
+        ScopeForParameters.InstantiatedLocal(PVD, PVD);
+        continue;
+      }
+      // This is hacky: we're mapping the parameter pack to a size-of-1 argument
+      // to avoid building SubstTemplateTypeParmPackTypes for
+      // PackExpansionTypes. The SubstTemplateTypeParmPackType node would
+      // otherwise reference the AssociatedDecl of the template arguments, which
+      // is, in this case, the template declaration.
+      //
+      // However, as we're also calculating the redeclarations of the template,
+      // the canonical declarations thereof are actually themselves at the
+      // moment. So if we didn't expand these packs, we would end up with an
+      // incorrect profile difference because we will be profiling the
+      // canonical types!
+      //
+      // FIXME: Improve the "no-transform" machinery in FindInstantiatedDecl so
+      // that we can eliminate the Scope in the cases where the declarations are
+      // not necessarily instantiated. It would also benefit the noexcept
+      // specifier comparison.
----------------
cor3ntin wrote:

I think both approach are equally... in search of a better 3rd approach :D
I like the Decl approach because it doesn't mess too much with substitution at all, the only thing that should be affected is profiling. So I am somewhat more confident about its correctness, if that makes sense.
 
Any preference @mizvekov ?

https://github.com/llvm/llvm-project/pull/102131