[clang] Concepts: support pack expansions for type constraints

This reverts an earlier attempt to support these expansions,
which was limited to type arguments and which subverted the purpose
of SubstTemplateTypeParmType.

This propagates the ArgumentPackSubstitutionIndex along with the
AssociatedConstraint, so that the pack expansion works, without
needing any new transforms or otherwise any changes to the template
instanntiation process.

This keeps the tests from the reverted commits, and adds a few more
showing the new solution also works for NTTPs.

This patch is incomplete:
* It is likely missing plumbing the ArgumentPackSubstitutionIndex
  into more places.
* The Normalization cache is not adapted so it indexes on the
  ArgumentPackSubstitutionIndex as well.

One new test is added, which in any case shows an ambiguous call,
but if the normalization cache were corrected, the reason for
ambighuity would change from subsumption both ways, to neither
subsumes the other.

I am not sure if the current language rules would allow
for a test case where the pack index would break an ambiguity,
this is left for future consideration.

Fixes #131798

Author: mizvekov

clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp

@@ -356,7 +356,7 @@ static std::vector<FixItHint> handleReturnType(const FunctionDecl *Function,
   if (!TypeText)
     return {};
 
-  SmallVector<const Expr *, 3> ExistingConstraints;
+  SmallVector<AssociatedConstraint, 3> ExistingConstraints;
   Function->getAssociatedConstraints(ExistingConstraints);
   if (!ExistingConstraints.empty()) {
     // FIXME - Support adding new constraints to existing ones. Do we need to
@@ -404,7 +404,7 @@ handleTrailingTemplateType(const FunctionTemplateDecl *FunctionTemplate,
   if (!ConditionText)
     return {};
 
-  SmallVector<const Expr *, 3> ExistingConstraints;
+  SmallVector<AssociatedConstraint, 3> ExistingConstraints;
   Function->getAssociatedConstraints(ExistingConstraints);
   if (!ExistingConstraints.empty()) {
     // FIXME - Support adding new constraints to existing ones. Do we need to

clang/docs/ReleaseNotes.rst

@@ -370,6 +370,9 @@ Bug Fixes to C++ Support
 - Clang now uses the parameter location for abbreviated function templates in ``extern "C"``. (#GH46386)
 - Clang will emit an error instead of crash when use co_await or co_yield in
   C++26 braced-init-list template parameter initialization. (#GH78426)
+- Improved fix for an issue with pack expansions of type constraints, where this
+  now also works if the constraint has non-type or template template parameters.
+  (#GH131798)
 - Fixes matching of nested template template parameters. (#GH130362)
 - Correctly diagnoses template template paramters which have a pack parameter
   not in the last position.

clang/include/clang/AST/ASTConcept.h

@@ -229,12 +229,15 @@ class TypeConstraint {
   /// type-constraint.
   Expr *ImmediatelyDeclaredConstraint = nullptr;
   ConceptReference *ConceptRef;
+  int ArgumentPackSubstitutionIndex;
 
 public:
   TypeConstraint(ConceptReference *ConceptRef,
-                 Expr *ImmediatelyDeclaredConstraint)
+                 Expr *ImmediatelyDeclaredConstraint,
+                 int ArgumentPackSubstitutionIndex)
       : ImmediatelyDeclaredConstraint(ImmediatelyDeclaredConstraint),
-        ConceptRef(ConceptRef) {}
+        ConceptRef(ConceptRef),
+        ArgumentPackSubstitutionIndex(ArgumentPackSubstitutionIndex) {}
 
   /// \brief Get the immediately-declared constraint expression introduced by
   /// this type-constraint, that is - the constraint expression that is added to
@@ -245,6 +248,10 @@ class TypeConstraint {
 
   ConceptReference *getConceptReference() const { return ConceptRef; }
 
+  int getArgumentPackSubstitutionIndex() const {
+    return ArgumentPackSubstitutionIndex;
+  }
+
   // FIXME: Instead of using these concept related functions the callers should
   // directly work with the corresponding ConceptReference.
   ConceptDecl *getNamedConcept() const { return ConceptRef->getNamedConcept(); }

clang/include/clang/AST/Decl.h

@@ -78,6 +78,18 @@ class UnresolvedSetImpl;
 class VarTemplateDecl;
 enum class ImplicitParamKind;
 
+// Holds a constraint expression along with a pack expansion index, if
+// expanded.
+struct AssociatedConstraint {
+  const Expr *ConstraintExpr;
+  int ArgumentPackSubstitutionIndex;
+
+  explicit AssociatedConstraint(const Expr *ConstraintExpr,
+                                int ArgumentPackSubstitutionIndex = -1)
+      : ConstraintExpr(ConstraintExpr),
+        ArgumentPackSubstitutionIndex(ArgumentPackSubstitutionIndex) {}
+};
+
 /// The top declaration context.
 class TranslationUnitDecl : public Decl,
                             public DeclContext,
@@ -2631,9 +2643,10 @@ class FunctionDecl : public DeclaratorDecl,
   ///
   /// Use this instead of getTrailingRequiresClause for concepts APIs that
   /// accept an ArrayRef of constraint expressions.
-  void getAssociatedConstraints(SmallVectorImpl<const Expr *> &AC) const {
+  void
+  getAssociatedConstraints(SmallVectorImpl<AssociatedConstraint> &AC) const {
     if (auto *TRC = getTrailingRequiresClause())
-      AC.push_back(TRC);
+      AC.emplace_back(TRC);
   }
 
   /// Get the message that indicates why this function was deleted.

clang/include/clang/AST/DeclTemplate.h

@@ -195,7 +195,8 @@ class TemplateParameterList final
   ///
   /// The constraints in the resulting list are to be treated as if in a
   /// conjunction ("and").
-  void getAssociatedConstraints(llvm::SmallVectorImpl<const Expr *> &AC) const;
+  void getAssociatedConstraints(
+      llvm::SmallVectorImpl<AssociatedConstraint> &AC) const;
 
   bool hasAssociatedConstraints() const;
 
@@ -422,7 +423,8 @@ class TemplateDecl : public NamedDecl {
   /// including constraint-expressions derived from the requires-clause,
   /// trailing requires-clause (for functions and methods) and constrained
   /// template parameters.
-  void getAssociatedConstraints(llvm::SmallVectorImpl<const Expr *> &AC) const;
+  void getAssociatedConstraints(
+      llvm::SmallVectorImpl<AssociatedConstraint> &AC) const;
 
   bool hasAssociatedConstraints() const;
 
@@ -1341,7 +1343,8 @@ class TemplateTypeParmDecl final : public TypeDecl,
   }
 
   void setTypeConstraint(ConceptReference *CR,
-                         Expr *ImmediatelyDeclaredConstraint);
+                         Expr *ImmediatelyDeclaredConstraint,
+                         int ArgumentPackSubstitutionIndex);
 
   /// Determine whether this template parameter has a type-constraint.
   bool hasTypeConstraint() const {
@@ -1353,9 +1356,11 @@ class TemplateTypeParmDecl final : public TypeDecl,
   ///
   /// Use this instead of getTypeConstraint for concepts APIs that
   /// accept an ArrayRef of constraint expressions.
-  void getAssociatedConstraints(llvm::SmallVectorImpl<const Expr *> &AC) const {
+  void getAssociatedConstraints(
+      llvm::SmallVectorImpl<AssociatedConstraint> &AC) const {
     if (HasTypeConstraint)
-      AC.push_back(getTypeConstraint()->getImmediatelyDeclaredConstraint());
+      AC.emplace_back(getTypeConstraint()->getImmediatelyDeclaredConstraint(),
+                      getTypeConstraint()->getArgumentPackSubstitutionIndex());
   }
 
   SourceRange getSourceRange() const override LLVM_READONLY;
@@ -1574,9 +1579,10 @@ class NonTypeTemplateParmDecl final
   ///
   /// Use this instead of getPlaceholderImmediatelyDeclaredConstraint for
   /// concepts APIs that accept an ArrayRef of constraint expressions.
-  void getAssociatedConstraints(llvm::SmallVectorImpl<const Expr *> &AC) const {
+  void getAssociatedConstraints(
+      llvm::SmallVectorImpl<AssociatedConstraint> &AC) const {
     if (Expr *E = getPlaceholderTypeConstraint())
-      AC.push_back(E);
+      AC.emplace_back(E);
   }
 
   // Implement isa/cast/dyncast/etc.
@@ -2169,7 +2175,8 @@ class ClassTemplatePartialSpecializationDecl
   ///
   /// The constraints in the resulting list are to be treated as if in a
   /// conjunction ("and").
-  void getAssociatedConstraints(llvm::SmallVectorImpl<const Expr *> &AC) const {
+  void getAssociatedConstraints(
+      llvm::SmallVectorImpl<AssociatedConstraint> &AC) const {
     TemplateParams->getAssociatedConstraints(AC);
   }
 
@@ -2943,7 +2950,8 @@ class VarTemplatePartialSpecializationDecl
   ///
   /// The constraints in the resulting list are to be treated as if in a
   /// conjunction ("and").
-  void getAssociatedConstraints(llvm::SmallVectorImpl<const Expr *> &AC) const {
+  void getAssociatedConstraints(
+      llvm::SmallVectorImpl<AssociatedConstraint> &AC) const {
     TemplateParams->getAssociatedConstraints(AC);
   }
 

clang/include/clang/Sema/Sema.h

@@ -14551,13 +14551,14 @@ class Sema final : public SemaBase {
   /// \returns true if an error occurred and satisfaction could not be checked,
   /// false otherwise.
   bool CheckConstraintSatisfaction(
-      const NamedDecl *Template, ArrayRef<const Expr *> ConstraintExprs,
+      const NamedDecl *Template,
+      ArrayRef<AssociatedConstraint> AssociatedConstraints,
       const MultiLevelTemplateArgumentList &TemplateArgLists,
       SourceRange TemplateIDRange, ConstraintSatisfaction &Satisfaction) {
     llvm::SmallVector<Expr *, 4> Converted;
-    return CheckConstraintSatisfaction(Template, ConstraintExprs, Converted,
-                                       TemplateArgLists, TemplateIDRange,
-                                       Satisfaction);
+    return CheckConstraintSatisfaction(Template, AssociatedConstraints,
+                                       Converted, TemplateArgLists,
+                                       TemplateIDRange, Satisfaction);
   }
 
   /// \brief Check whether the given list of constraint expressions are
@@ -14583,7 +14584,8 @@ class Sema final : public SemaBase {
   /// \returns true if an error occurred and satisfaction could not be checked,
   /// false otherwise.
   bool CheckConstraintSatisfaction(
-      const NamedDecl *Template, ArrayRef<const Expr *> ConstraintExprs,
+      const NamedDecl *Template,
+      ArrayRef<AssociatedConstraint> AssociatedConstraints,
       llvm::SmallVectorImpl<Expr *> &ConvertedConstraints,
       const MultiLevelTemplateArgumentList &TemplateArgList,
       SourceRange TemplateIDRange, ConstraintSatisfaction &Satisfaction);
@@ -14661,7 +14663,7 @@ class Sema final : public SemaBase {
 
   const NormalizedConstraint *getNormalizedAssociatedConstraints(
       const NamedDecl *ConstrainedDecl,
-      ArrayRef<const Expr *> AssociatedConstraints);
+      ArrayRef<AssociatedConstraint> AssociatedConstraints);
 
   /// \brief Check whether the given declaration's associated constraints are
   /// at least as constrained than another declaration's according to the
@@ -14672,17 +14674,18 @@ class Sema final : public SemaBase {
   ///
   /// \returns true if an error occurred, false otherwise.
   bool IsAtLeastAsConstrained(const NamedDecl *D1,
-                              MutableArrayRef<const Expr *> AC1,
+                              MutableArrayRef<AssociatedConstraint> AC1,
                               const NamedDecl *D2,
-                              MutableArrayRef<const Expr *> AC2, bool &Result);
+                              MutableArrayRef<AssociatedConstraint> AC2,
+                              bool &Result);
 
   /// If D1 was not at least as constrained as D2, but would've been if a pair
   /// of atomic constraints involved had been declared in a concept and not
   /// repeated in two separate places in code.
   /// \returns true if such a diagnostic was emitted, false otherwise.
   bool MaybeEmitAmbiguousAtomicConstraintsDiagnostic(
-      const NamedDecl *D1, ArrayRef<const Expr *> AC1, const NamedDecl *D2,
-      ArrayRef<const Expr *> AC2);
+      const NamedDecl *D1, ArrayRef<AssociatedConstraint> AC1,
+      const NamedDecl *D2, ArrayRef<AssociatedConstraint> AC2);
 
 private:
   /// Caches pairs of template-like decls whose associated constraints were

clang/include/clang/Sema/SemaConcept.h

@@ -114,7 +114,8 @@ struct NormalizedConstraint {
 
 private:
   static std::optional<NormalizedConstraint>
-  fromConstraintExprs(Sema &S, const NamedDecl *D, ArrayRef<const Expr *> E);
+  fromAssociatedConstraints(Sema &S, const NamedDecl *D,
+                            ArrayRef<AssociatedConstraint> ACs);
   static std::optional<NormalizedConstraint>
   fromConstraintExpr(Sema &S, const NamedDecl *D, const Expr *E);
 };
@@ -138,7 +139,7 @@ struct alignas(ConstraintAlignment) FoldExpandedConstraint {
 
 const NormalizedConstraint *getNormalizedAssociatedConstraints(
     Sema &S, const NamedDecl *ConstrainedDecl,
-    ArrayRef<const Expr *> AssociatedConstraints);
+    ArrayRef<AssociatedConstraint> AssociatedConstraints);
 
 /// \brief SubsumptionChecker establishes subsumption
 /// between two set of constraints.
@@ -149,8 +150,10 @@ class SubsumptionChecker {
 
   SubsumptionChecker(Sema &SemaRef, SubsumptionCallable Callable = {});
 
-  std::optional<bool> Subsumes(const NamedDecl *DP, ArrayRef<const Expr *> P,
-                               const NamedDecl *DQ, ArrayRef<const Expr *> Q);
+  std::optional<bool> Subsumes(const NamedDecl *DP,
+                               ArrayRef<AssociatedConstraint> P,
+                               const NamedDecl *DQ,
+                               ArrayRef<AssociatedConstraint> Q);
 
   bool Subsumes(const NormalizedConstraint *P, const NormalizedConstraint *Q);
 

clang/lib/AST/ASTImporter.cpp

@@ -5975,7 +5975,8 @@ ASTNodeImporter::VisitTemplateTypeParmDecl(TemplateTypeParmDecl *D) {
     if (Err)
       return std::move(Err);
 
-    ToD->setTypeConstraint(ToConceptRef, ToIDC);
+    ToD->setTypeConstraint(ToConceptRef, ToIDC,
+                           TC->getArgumentPackSubstitutionIndex());
   }
 
   if (Error Err = importTemplateParameterDefaultArgument(D, ToD))

clang/lib/AST/DeclTemplate.cpp

@@ -223,20 +223,21 @@ static bool AdoptTemplateParameterList(TemplateParameterList *Params,
   return Invalid;
 }
 
-void TemplateParameterList::
-getAssociatedConstraints(llvm::SmallVectorImpl<const Expr *> &AC) const {
+void TemplateParameterList::getAssociatedConstraints(
+    llvm::SmallVectorImpl<AssociatedConstraint> &ACs) const {
   if (HasConstrainedParameters)
     for (const NamedDecl *Param : *this) {
       if (const auto *TTP = dyn_cast<TemplateTypeParmDecl>(Param)) {
         if (const auto *TC = TTP->getTypeConstraint())
-          AC.push_back(TC->getImmediatelyDeclaredConstraint());
+          ACs.emplace_back(TC->getImmediatelyDeclaredConstraint(),
+                           TC->getArgumentPackSubstitutionIndex());
       } else if (const auto *NTTP = dyn_cast<NonTypeTemplateParmDecl>(Param)) {
         if (const Expr *E = NTTP->getPlaceholderTypeConstraint())
-          AC.push_back(E);
+          ACs.emplace_back(E);
       }
     }
   if (HasRequiresClause)
-    AC.push_back(getRequiresClause());
+    ACs.emplace_back(getRequiresClause());
 }
 
 bool TemplateParameterList::hasAssociatedConstraints() const {
@@ -286,12 +287,12 @@ TemplateDecl::TemplateDecl(Kind DK, DeclContext *DC, SourceLocation L,
 
 void TemplateDecl::anchor() {}
 
-void TemplateDecl::
-getAssociatedConstraints(llvm::SmallVectorImpl<const Expr *> &AC) const {
-  TemplateParams->getAssociatedConstraints(AC);
+void TemplateDecl::getAssociatedConstraints(
+    llvm::SmallVectorImpl<AssociatedConstraint> &ACs) const {
+  TemplateParams->getAssociatedConstraints(ACs);
   if (auto *FD = dyn_cast_or_null<FunctionDecl>(getTemplatedDecl()))
     if (const Expr *TRC = FD->getTrailingRequiresClause())
-      AC.push_back(TRC);
+      ACs.emplace_back(TRC);
 }
 
 bool TemplateDecl::hasAssociatedConstraints() const {
@@ -748,14 +749,15 @@ bool TemplateTypeParmDecl::isParameterPack() const {
 }
 
 void TemplateTypeParmDecl::setTypeConstraint(
-    ConceptReference *Loc, Expr *ImmediatelyDeclaredConstraint) {
+    ConceptReference *Loc, Expr *ImmediatelyDeclaredConstraint,
+    int ArgumentPackSubstitutionIndex) {
   assert(HasTypeConstraint &&
          "HasTypeConstraint=true must be passed at construction in order to "
          "call setTypeConstraint");
   assert(!TypeConstraintInitialized &&
          "TypeConstraint was already initialized!");
-  new (getTrailingObjects<TypeConstraint>())
-      TypeConstraint(Loc, ImmediatelyDeclaredConstraint);
+  new (getTrailingObjects<TypeConstraint>()) TypeConstraint(
+      Loc, ImmediatelyDeclaredConstraint, ArgumentPackSubstitutionIndex);
   TypeConstraintInitialized = true;
 }
 

clang/lib/Sema/SemaCodeComplete.cpp

@@ -5463,8 +5463,9 @@ class ConceptInfo {
   // that T is attached to in order to gather the relevant constraints.
   ConceptInfo(const TemplateTypeParmType &BaseType, Scope *S) {
     auto *TemplatedEntity = getTemplatedEntity(BaseType.getDecl(), S);
-    for (const Expr *E : constraintsForTemplatedEntity(TemplatedEntity))
-      believe(E, &BaseType);
+    for (const AssociatedConstraint &AC :
+         constraintsForTemplatedEntity(TemplatedEntity))
+      believe(AC.ConstraintExpr, &BaseType);
   }
 
   std::vector<Member> members() {
@@ -5696,9 +5697,9 @@ class ConceptInfo {
 
   // Gets all the type constraint expressions that might apply to the type
   // variables associated with DC (as returned by getTemplatedEntity()).
-  static SmallVector<const Expr *, 1>
+  static SmallVector<AssociatedConstraint, 1>
   constraintsForTemplatedEntity(DeclContext *DC) {
-    SmallVector<const Expr *, 1> Result;
+    SmallVector<AssociatedConstraint, 1> Result;
     if (DC == nullptr)
       return Result;
     // Primary templates can have constraints.