[Clang][Concepts] Normalize SizeOfPackExpr's pack declaration (#110238)

SizeOfPackExpr has a pointer to the referenced pack declaration, which
is left as-is during the transformation process.

The situation could be subtle when a friend class template declaration
comes into play. The declaration per se would be instantiated into its
parent declaration context, and consequently, the template parameter
list would have a depth adjustment; however, as we don't evaluate
constraints during instantiation, those constraints would still
reference the original template parameters, which is fine for constraint
evaluation because we have handled friend cases in the template argument
collection.

However, things are different when we want to profile the constraint
expression with dependent template arguments. The hash algorithm of
SizeOfPackExpr takes its pack declaration as a factor, which is the
original template parameter that might still have untransformed template
depths after the constraint normalization.

This patch transforms the pack declaration when normalizing constraint
expressions and pluses a fix in HandleFunctionTemplateDecl() where the
associated declaration is incorrect for nested specifiers.

Note that the fix in HandleFunctionTemplateDecl(), as well as the
handling logic for NestedNameSpecifier, would be removed once Krystian's
refactoring patch lands. But I still want to incorporate it in the patch
for the correction purpose, though it hasn't caused any problems so far
- I just tripped over that in getFullyPackExpandedSize() when I tried to
extract the transformed declarations from the TemplateArgument.

Fixes #93099

---------

Co-authored-by: Matheus Izvekov <[email protected]>

Author: zyn0217

clang/docs/ReleaseNotes.rst

@@ -453,6 +453,8 @@ Bug Fixes to C++ Support
 - Mangle friend function templates with a constraint that depends on a template parameter from an enclosing template as members of the enclosing class. (#GH110247)
 - Fixed an issue in constraint evaluation, where type constraints on the lambda expression
   containing outer unexpanded parameters were not correctly expanded. (#GH101754)
+- Fixed a bug in constraint expression comparison where the ``sizeof...`` expression was not handled properly
+  in certain friend declarations. (#GH93099)
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^

clang/lib/Sema/SemaConcept.cpp

@@ -975,11 +975,14 @@ static const Expr *SubstituteConstraintExpressionWithoutSatisfaction(
   // parameters that the surrounding function hasn't been instantiated yet. Note
   // this may happen while we're comparing two templates' constraint
   // equivalence.
-  LocalInstantiationScope ScopeForParameters(S);
-  if (auto *FD = DeclInfo.getDecl()->getAsFunction())
+  std::optional<LocalInstantiationScope> ScopeForParameters;
+  if (const NamedDecl *ND = DeclInfo.getDecl();
+      ND && ND->isFunctionOrFunctionTemplate()) {
+    ScopeForParameters.emplace(S);
+    const FunctionDecl *FD = ND->getAsFunction();
     for (auto *PVD : FD->parameters()) {
       if (!PVD->isParameterPack()) {
-        ScopeForParameters.InstantiatedLocal(PVD, PVD);
+        ScopeForParameters->InstantiatedLocal(PVD, PVD);
         continue;
       }
       // This is hacky: we're mapping the parameter pack to a size-of-1 argument
@@ -998,9 +1001,10 @@ static const Expr *SubstituteConstraintExpressionWithoutSatisfaction(
       // that we can eliminate the Scope in the cases where the declarations are
       // not necessarily instantiated. It would also benefit the noexcept
       // specifier comparison.
-      ScopeForParameters.MakeInstantiatedLocalArgPack(PVD);
-      ScopeForParameters.InstantiatedLocalPackArg(PVD, PVD);
+      ScopeForParameters->MakeInstantiatedLocalArgPack(PVD);
+      ScopeForParameters->InstantiatedLocalPackArg(PVD, PVD);
     }
+  }
 
   std::optional<Sema::CXXThisScopeRAII> ThisScope;
 

clang/lib/Sema/SemaTemplateInstantiate.cpp

@@ -371,7 +371,7 @@ Response HandleFunctionTemplateDecl(const FunctionTemplateDecl *FTD,
                   Specialization->getTemplateInstantiationArgs().asArray();
           }
           Result.addOuterTemplateArguments(
-              const_cast<FunctionTemplateDecl *>(FTD), Arguments,
+              TSTy->getTemplateName().getAsTemplateDecl(), Arguments,
               /*Final=*/false);
         }
       }
@@ -1737,6 +1737,33 @@ namespace {
       return inherited::TransformLambdaBody(E, Body);
     }
 
+    ExprResult RebuildSizeOfPackExpr(SourceLocation OperatorLoc,
+                                     NamedDecl *Pack, SourceLocation PackLoc,
+                                     SourceLocation RParenLoc,
+                                     std::optional<unsigned> Length,
+                                     ArrayRef<TemplateArgument> PartialArgs) {
+      if (SemaRef.CodeSynthesisContexts.back().Kind !=
+          Sema::CodeSynthesisContext::ConstraintNormalization)
+        return inherited::RebuildSizeOfPackExpr(OperatorLoc, Pack, PackLoc,
+                                                RParenLoc, Length, PartialArgs);
+
+#ifndef NDEBUG
+      for (auto *Iter = TemplateArgs.begin(); Iter != TemplateArgs.end();
+           ++Iter)
+        for (const TemplateArgument &TA : Iter->Args)
+          assert(TA.getKind() != TemplateArgument::Pack || TA.pack_size() == 1);
+#endif
+      Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(
+          SemaRef, /*NewSubstitutionIndex=*/0);
+      Decl *NewPack = TransformDecl(PackLoc, Pack);
+      if (!NewPack)
+        return ExprError();
+
+      return inherited::RebuildSizeOfPackExpr(OperatorLoc,
+                                              cast<NamedDecl>(NewPack), PackLoc,
+                                              RParenLoc, Length, PartialArgs);
+    }
+
     ExprResult TransformRequiresExpr(RequiresExpr *E) {
       LocalInstantiationScope Scope(SemaRef, /*CombineWithOuterScope=*/true);
       ExprResult TransReq = inherited::TransformRequiresExpr(E);

clang/test/SemaTemplate/concepts-out-of-line-def.cpp

@@ -666,3 +666,37 @@ int foo() {
 }
 
 } // namespace eve
+
+namespace GH93099 {
+
+// Issues with sizeof...(expr)
+
+template <typename T = int> struct C {
+  template <int... N>
+    requires(sizeof...(N) > 0)
+  friend class NTTP;
+
+  template <class... Tp>
+    requires(sizeof...(Tp) > 0)
+  friend class TP;
+
+  template <template <typename> class... TTp>
+    requires(sizeof...(TTp) > 0)
+  friend class TTP;
+};
+
+template <int... N>
+  requires(sizeof...(N) > 0)
+class NTTP;
+
+template <class... Tp>
+  requires(sizeof...(Tp) > 0)
+class TP;
+
+template <template <typename> class... TTp>
+  requires(sizeof...(TTp) > 0)
+class TTP;
+
+C v;
+
+} // namespace GH93099