[Clang][Sema] Retain the expanding index for unevaluated type constraints

[zyn0217]

(This continues the effort of #86265, fixing another piece of issue in constraint evaluation on variadic lambdas.)

We need the depth of the primary template parameters for constraint substitution. To that end, we avoided substituting type constraints by copying the constraint expression when instantiating a template. This, however, has left an issue in that for lambda's parameters, they can reference outer template packs that would be expanded in the process of an instantiation, where these parameters would make their way into the constraint evaluation, wherein we have no other way to expand them later in evaluation. For example,

template <class... Ts> void foo() {
   bar([](C<Ts> auto value) {}...);
}

The lambda references a pack Ts that should be expanded when instantiating foo(). The Ts along with the constraint expression would not be transformed until constraint evaluation, and at that point, we would have no chance to expand Ts anyhow.

This patch takes an approach that transforms Ts from an unexpanded TemplateTypeParmType into a SubstTemplateTypeParmType with the current pack substitution index, such that we could use that to expand the type during evaluation.

Fixes #101754

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Younan Zhang (zyn0217)

Changes

(This continues the effort of #86265, fixing another piece of issue in constraint evaluation on variadic lambdas.)

We need the depth of the primary template parameters for constraint substitution. To that end, we avoided substituting type constraints by copying the constraint expression when instantiating a template. This, however, has left an issue in that for lambda's parameters, they can reference outer template packs that would be expanded in the process of an instantiation, where these parameters would make their way into the constraint evaluation, wherein we have no other way to expand them later in evaluation. For example,

template <class... Ts> void foo() {
   bar([](C<Ts> auto value) {}...);
}

The lambda references a pack Ts that should be expanded when instantiating foo(). The Ts along with the constraint expression would not be transformed until constraint evaluation, and at that point, we would have no chance to expand Ts anyhow.

This patch takes an approach that transforms Ts from an unexpanded TemplateTypeParmType into a SubstTemplateTypeParmType with the current pack substitution index, such that we could use that to expand the type during evaluation.

Fixes #101754

---

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

6 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+2)
• (modified) clang/include/clang/Sema/Sema.h (+1)
• (modified) clang/lib/Sema/SemaTemplate.cpp (+4-2)
• (modified) clang/lib/Sema/SemaTemplateInstantiate.cpp (+124-3)
• (modified) clang/lib/Sema/SemaType.cpp (+6-2)
• (modified) clang/test/SemaCXX/fold_lambda_with_variadics.cpp (+54)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f4535db7356194..c069bb0b4fa597 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -407,6 +407,8 @@ Bug Fixes to C++ Support
 - Clang now uses the correct set of template argument lists when comparing the constraints of
   out-of-line definitions and member templates explicitly specialized for a given implicit instantiation of
   a class template. (#GH102320)
+- Fixed an issue in constraint evaluation, where type constraints on the lambda expression
+  containing outer unexpanded parameters were not correctly expanded. (#GH101754)
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index b86861ce7e8cfa..f711fe952d35e9 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -11252,6 +11252,7 @@ class Sema final : public SemaBase {
                             ConceptDecl *NamedConcept, NamedDecl *FoundDecl,
                             const TemplateArgumentListInfo *TemplateArgs,
                             TemplateTypeParmDecl *ConstrainedParameter,
+                            QualType ConstrainedType,
                             SourceLocation EllipsisLoc);
 
   bool AttachTypeConstraint(AutoTypeLoc TL,
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 16f4542d785715..bdfb0f4c95afb8 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -1134,7 +1134,8 @@ bool Sema::BuildTypeConstraint(const CXXScopeSpec &SS,
       SS.isSet() ? SS.getWithLocInContext(Context) : NestedNameSpecifierLoc(),
       ConceptName, CD, /*FoundDecl=*/USD ? cast<NamedDecl>(USD) : CD,
       TypeConstr->LAngleLoc.isValid() ? &TemplateArgs : nullptr,
-      ConstrainedParameter, EllipsisLoc);
+      ConstrainedParameter, Context.getTypeDeclType(ConstrainedParameter),
+      EllipsisLoc);
 }
 
 template <typename ArgumentLocAppender>
@@ -1191,6 +1192,7 @@ bool Sema::AttachTypeConstraint(NestedNameSpecifierLoc NS,
                                 ConceptDecl *NamedConcept, NamedDecl *FoundDecl,
                                 const TemplateArgumentListInfo *TemplateArgs,
                                 TemplateTypeParmDecl *ConstrainedParameter,
+                                QualType ConstrainedType,
                                 SourceLocation EllipsisLoc) {
   // C++2a [temp.param]p4:
   //     [...] If Q is of the form C<A1, ..., An>, then let E' be
@@ -1199,7 +1201,7 @@ bool Sema::AttachTypeConstraint(NestedNameSpecifierLoc NS,
     TemplateArgs ? ASTTemplateArgumentListInfo::Create(Context,
                                                        *TemplateArgs) : nullptr;
 
-  QualType ParamAsArgument(ConstrainedParameter->getTypeForDecl(), 0);
+  QualType ParamAsArgument = ConstrainedType;
 
   ExprResult ImmediatelyDeclaredConstraint = formImmediatelyDeclaredConstraint(
       *this, NS, NameInfo, NamedConcept, FoundDecl,
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 7481c700019dc8..34635ef3860803 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -1646,6 +1646,21 @@ namespace {
                                            SubstTemplateTypeParmPackTypeLoc TL,
                                            bool SuppressObjCLifetime);
 
+    QualType
+    TransformSubstTemplateTypeParmType(TypeLocBuilder &TLB,
+                                       SubstTemplateTypeParmTypeLoc TL) {
+      if (SemaRef.CodeSynthesisContexts.back().Kind !=
+          Sema::CodeSynthesisContext::ConstraintSubstitution)
+        return inherited::TransformSubstTemplateTypeParmType(TLB, TL);
+
+      auto PackIndex = TL.getTypePtr()->getPackIndex();
+      std::optional<Sema::ArgumentPackSubstitutionIndexRAII> SubstIndex;
+      if (SemaRef.ArgumentPackSubstitutionIndex == -1 && PackIndex)
+        SubstIndex.emplace(SemaRef, *PackIndex);
+
+      return inherited::TransformSubstTemplateTypeParmType(TLB, TL);
+    }
+
     CXXRecordDecl::LambdaDependencyKind
     ComputeLambdaDependency(LambdaScopeInfo *LSI) {
       if (auto TypeAlias = getEnclosingTypeAliasTemplateDecl(getSema());
@@ -3056,6 +3071,58 @@ namespace {
 
 } // namespace
 
+namespace {
+
+struct ExpandPackedTypeConstraints
+    : TreeTransform<ExpandPackedTypeConstraints> {
+
+  using inherited = TreeTransform<ExpandPackedTypeConstraints>;
+
+  ExpandPackedTypeConstraints(Sema &SemaRef) : inherited(SemaRef) {}
+
+  using inherited::TransformTemplateTypeParmType;
+
+  QualType TransformTemplateTypeParmType(TypeLocBuilder &TLB,
+                                         TemplateTypeParmTypeLoc TL, bool) {
+    const TemplateTypeParmType *T = TL.getTypePtr();
+    if (!T->isParameterPack()) {
+      TemplateTypeParmTypeLoc NewTL =
+          TLB.push<TemplateTypeParmTypeLoc>(TL.getType());
+      NewTL.setNameLoc(TL.getNameLoc());
+      return TL.getType();
+    }
+
+    assert(SemaRef.ArgumentPackSubstitutionIndex != -1);
+
+    QualType Result = SemaRef.Context.getSubstTemplateTypeParmType(
+        TL.getType(), T->getDecl(), T->getIndex(),
+        SemaRef.ArgumentPackSubstitutionIndex);
+    SubstTemplateTypeParmTypeLoc NewTL =
+        TLB.push<SubstTemplateTypeParmTypeLoc>(Result);
+    NewTL.setNameLoc(TL.getNameLoc());
+    return Result;
+  }
+
+  QualType TransformSubstTemplateTypeParmType(TypeLocBuilder &TLB,
+                                              SubstTemplateTypeParmTypeLoc TL) {
+    const SubstTemplateTypeParmType *T = TL.getTypePtr();
+    if (T->getPackIndex()) {
+      SubstTemplateTypeParmTypeLoc TypeLoc =
+          TLB.push<SubstTemplateTypeParmTypeLoc>(TL.getType());
+      TypeLoc.setNameLoc(TL.getNameLoc());
+      return TypeLoc.getType();
+    }
+    return inherited::TransformSubstTemplateTypeParmType(TLB, TL);
+  }
+
+  bool SubstTemplateArguments(ArrayRef<TemplateArgumentLoc> Args,
+                              TemplateArgumentListInfo &Out) {
+    return inherited::TransformTemplateArguments(Args.begin(), Args.end(), Out);
+  }
+};
+
+} // namespace
+
 bool Sema::SubstTypeConstraint(
     TemplateTypeParmDecl *Inst, const TypeConstraint *TC,
     const MultiLevelTemplateArgumentList &TemplateArgs,
@@ -3064,9 +3131,62 @@ bool Sema::SubstTypeConstraint(
       TC->getTemplateArgsAsWritten();
 
   if (!EvaluateConstraints) {
-      Inst->setTypeConstraint(TC->getConceptReference(),
-                              TC->getImmediatelyDeclaredConstraint());
-      return false;
+    bool ShouldExpandExplicitTemplateArgs =
+        TemplArgInfo && ArgumentPackSubstitutionIndex != -1 &&
+        llvm::any_of(TemplArgInfo->arguments(), [](auto &Arg) {
+          return Arg.getArgument().containsUnexpandedParameterPack();
+        });
+
+    // We want to transform the packs into Subst* nodes for type constraints
+    // inside a pack expansion. For example,
+    //
+    //  template <class... Ts> void foo() {
+    //    bar([](C<Ts> auto value) {}...);
+    //  }
+    //
+    // As we expand Ts in the process of instantiating foo(), and retain
+    // the original template depths of Ts until the constraint evaluation, we
+    // would otherwise have no chance to expand Ts by the time of evaluating
+    // C<auto, Ts>.
+    //
+    // So we form a Subst* node for Ts along with a proper substitution index
+    // here, and substitute the node with a complete MLTAL later in evaluation.
+    if (ShouldExpandExplicitTemplateArgs) {
+      TemplateArgumentListInfo InstArgs;
+      InstArgs.setLAngleLoc(TemplArgInfo->LAngleLoc);
+      InstArgs.setRAngleLoc(TemplArgInfo->RAngleLoc);
+      if (ExpandPackedTypeConstraints(*this).SubstTemplateArguments(
+              TemplArgInfo->arguments(), InstArgs))
+        return true;
+
+      // The type of the original parameter.
+      auto *ConstraintExpr = TC->getImmediatelyDeclaredConstraint();
+      QualType ConstrainedType;
+
+      if (auto *FE = dyn_cast<CXXFoldExpr>(ConstraintExpr)) {
+        assert(FE->getLHS());
+        ConstraintExpr = FE->getLHS();
+      }
+      auto *CSE = cast<ConceptSpecializationExpr>(ConstraintExpr);
+      assert(!CSE->getTemplateArguments().empty() &&
+             "Empty template arguments?");
+      ConstrainedType = CSE->getTemplateArguments()[0].getAsType();
+      assert(!ConstrainedType.isNull() &&
+             "Failed to extract the original ConstrainedType?");
+
+      return AttachTypeConstraint(
+          TC->getNestedNameSpecifierLoc(), TC->getConceptNameInfo(),
+          TC->getNamedConcept(),
+          /*FoundDecl=*/TC->getConceptReference()->getFoundDecl(), &InstArgs,
+          Inst, ConstrainedType,
+          Inst->isParameterPack()
+              ? cast<CXXFoldExpr>(TC->getImmediatelyDeclaredConstraint())
+                    ->getEllipsisLoc()
+              : SourceLocation());
+    }
+    Inst->setTypeConstraint(TC->getConceptReference(),
+                            TC->getImmediatelyDeclaredConstraint());
+    return false;
   }
 
   TemplateArgumentListInfo InstArgs;
@@ -3082,6 +3202,7 @@ bool Sema::SubstTypeConstraint(
       TC->getNestedNameSpecifierLoc(), TC->getConceptNameInfo(),
       TC->getNamedConcept(),
       /*FoundDecl=*/TC->getConceptReference()->getFoundDecl(), &InstArgs, Inst,
+      Context.getTypeDeclType(Inst),
       Inst->isParameterPack()
           ? cast<CXXFoldExpr>(TC->getImmediatelyDeclaredConstraint())
                 ->getEllipsisLoc()
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 950bd6db0359d1..62a7357efcf9cc 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -3035,7 +3035,9 @@ InventTemplateParameter(TypeProcessingState &state, QualType T,
             AutoLoc.getNestedNameSpecifierLoc(), AutoLoc.getConceptNameInfo(),
             AutoLoc.getNamedConcept(), /*FoundDecl=*/AutoLoc.getFoundDecl(),
             AutoLoc.hasExplicitTemplateArgs() ? &TAL : nullptr,
-            InventedTemplateParam, D.getEllipsisLoc());
+            InventedTemplateParam,
+            S.Context.getTypeDeclType(InventedTemplateParam),
+            D.getEllipsisLoc());
       }
     } else {
       // The 'auto' appears in the decl-specifiers; we've not finished forming
@@ -3072,7 +3074,9 @@ InventTemplateParameter(TypeProcessingState &state, QualType T,
             /*FoundDecl=*/
             USD ? cast<NamedDecl>(USD) : CD,
             TemplateId->LAngleLoc.isValid() ? &TemplateArgsInfo : nullptr,
-            InventedTemplateParam, D.getEllipsisLoc());
+            InventedTemplateParam,
+            S.Context.getTypeDeclType(InventedTemplateParam),
+            D.getEllipsisLoc());
       }
     }
   }
diff --git a/clang/test/SemaCXX/fold_lambda_with_variadics.cpp b/clang/test/SemaCXX/fold_lambda_with_variadics.cpp
index 14e242f009dc51..2257a4c2d975a8 100644
--- a/clang/test/SemaCXX/fold_lambda_with_variadics.cpp
+++ b/clang/test/SemaCXX/fold_lambda_with_variadics.cpp
@@ -179,3 +179,57 @@ void foo() {
 }
 
 } // namespace GH99877
+
+namespace GH101754 {
+
+template <typename... Ts> struct Overloaded : Ts... {
+  using Ts::operator()...;
+};
+
+template <typename... Ts> Overloaded(Ts...) -> Overloaded<Ts...>;
+
+template <class T, class U>
+concept same_as = __is_same(T, U);  // #same_as
+
+template <typename... Ts> constexpr auto foo() {
+  return Overloaded{[](same_as<Ts> auto value) { return value; }...}; // #lambda
+}
+
+static_assert(foo<int, double>()(123) == 123);
+static_assert(foo<int, double>()(2.718) == 2.718);
+
+static_assert(foo<int, double>()('c'));
+// expected-error@-1 {{no matching function}}
+
+// expected-note@#lambda {{constraints not satisfied}}
+// expected-note@#lambda {{'same_as<char, int>' evaluated to false}}
+// expected-note@#same_as {{evaluated to false}}
+
+// expected-note@#lambda {{constraints not satisfied}}
+// expected-note@#lambda {{'same_as<char, double>' evaluated to false}}
+// expected-note@#same_as {{evaluated to false}}
+
+template <class T, class U, class V>
+concept C = same_as<T, U> && same_as<U, V>; // #C
+
+template <typename... Ts> constexpr auto bar() {
+  return ([]<class Up>() {
+    return Overloaded{[](C<Up, Ts> auto value) { // #bar
+      return value;
+    }...};
+  }.template operator()<Ts>(), ...);
+}
+static_assert(bar<int, float>()(3.14f)); // OK, bar() returns the last overload i.e. <float>.
+
+static_assert(bar<int, float>()(123));
+// expected-error@-1 {{no matching function}}
+// expected-note@#bar {{constraints not satisfied}}
+// expected-note@#bar {{'C<int, float, int>' evaluated to false}}
+// expected-note@#C {{evaluated to false}}
+
+// expected-note@#bar {{constraints not satisfied}}
+// expected-note@#bar {{'C<int, float, float>' evaluated to false}}
+// expected-note@#C {{evaluated to false}}
+// expected-note@#same_as 2{{evaluated to false}}
+
+} // namespace GH101754

[zyn0217]

Slightly ping

[erichkeane]

I like the solution here, I think this is a good way of making our constraint instantiation more stable. Thanks!