Skip to content

Commit 9a089c5

Browse files
mizvekovmizvekov
committed
[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.
1 parent e7ecd60 commit 9a089c5

21 files changed

+276
-151
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,7 @@ static std::vector<FixItHint> handleReturnType(const FunctionDecl *Function,
356356
if (!TypeText)
357357
return {};
358358

359-
SmallVector<const Expr *, 3> ExistingConstraints;
359+
SmallVector<AssociatedConstraint, 3> ExistingConstraints;
360360
Function->getAssociatedConstraints(ExistingConstraints);
361361
if (!ExistingConstraints.empty()) {
362362
// FIXME - Support adding new constraints to existing ones. Do we need to
@@ -404,7 +404,7 @@ handleTrailingTemplateType(const FunctionTemplateDecl *FunctionTemplate,
404404
if (!ConditionText)
405405
return {};
406406

407-
SmallVector<const Expr *, 3> ExistingConstraints;
407+
SmallVector<AssociatedConstraint, 3> ExistingConstraints;
408408
Function->getAssociatedConstraints(ExistingConstraints);
409409
if (!ExistingConstraints.empty()) {
410410
// FIXME - Support adding new constraints to existing ones. Do we need to

‎clang/docs/ReleaseNotes.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,8 @@ Bug Fixes to C++ Support
346346
- Clang now uses the parameter location for abbreviated function templates in ``extern "C"``. (#GH46386)
347347
- Clang will emit an error instead of crash when use co_await or co_yield in
348348
C++26 braced-init-list template parameter initialization. (#GH78426)
349+
- Improved fix for an issue with pack expansions of type constraints, where this
350+
now also works if the constraint has non-type or template template parameters.
349351
- Fixes matching of nested template template parameters. (#GH130362)
350352
- Correctly diagnoses template template paramters which have a pack parameter
351353
not in the last position.

‎clang/include/clang/AST/ASTConcept.h

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -229,12 +229,15 @@ class TypeConstraint {
229229
/// type-constraint.
230230
Expr *ImmediatelyDeclaredConstraint = nullptr;
231231
ConceptReference *ConceptRef;
232+
int ArgumentPackSubstitutionIndex;
232233

233234
public:
234235
TypeConstraint(ConceptReference *ConceptRef,
235-
Expr *ImmediatelyDeclaredConstraint)
236+
Expr *ImmediatelyDeclaredConstraint,
237+
int ArgumentPackSubstitutionIndex)
236238
: ImmediatelyDeclaredConstraint(ImmediatelyDeclaredConstraint),
237-
ConceptRef(ConceptRef) {}
239+
ConceptRef(ConceptRef),
240+
ArgumentPackSubstitutionIndex(ArgumentPackSubstitutionIndex) {}
238241

239242
/// \brief Get the immediately-declared constraint expression introduced by
240243
/// this type-constraint, that is - the constraint expression that is added to
@@ -245,6 +248,10 @@ class TypeConstraint {
245248

246249
ConceptReference *getConceptReference() const { return ConceptRef; }
247250

251+
int getArgumentPackSubstitutionIndex() const {
252+
return ArgumentPackSubstitutionIndex;
253+
}
254+
248255
// FIXME: Instead of using these concept related functions the callers should
249256
// directly work with the corresponding ConceptReference.
250257
ConceptDecl *getNamedConcept() const { return ConceptRef->getNamedConcept(); }

‎clang/include/clang/AST/Decl.h

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,16 @@ class UnresolvedSetImpl;
7878
class VarTemplateDecl;
7979
enum class ImplicitParamKind;
8080

81+
struct AssociatedConstraint {
82+
const Expr *ConstraintExpr;
83+
int ArgumentPackSubstitutionIndex;
84+
85+
constexpr AssociatedConstraint(const Expr *ConstraintExpr,
86+
int ArgumentPackSubstitutionIndex)
87+
: ConstraintExpr(ConstraintExpr),
88+
ArgumentPackSubstitutionIndex(ArgumentPackSubstitutionIndex) {}
89+
};
90+
8191
/// The top declaration context.
8292
class TranslationUnitDecl : public Decl,
8393
public DeclContext,
@@ -2631,9 +2641,10 @@ class FunctionDecl : public DeclaratorDecl,
26312641
///
26322642
/// Use this instead of getTrailingRequiresClause for concepts APIs that
26332643
/// accept an ArrayRef of constraint expressions.
2634-
void getAssociatedConstraints(SmallVectorImpl<const Expr *> &AC) const {
2644+
void
2645+
getAssociatedConstraints(SmallVectorImpl<AssociatedConstraint> &AC) const {
26352646
if (auto *TRC = getTrailingRequiresClause())
2636-
AC.push_back(TRC);
2647+
AC.emplace_back(TRC, /*ArgumentPackSubstitutionIndex=*/-1);
26372648
}
26382649

26392650
/// Get the message that indicates why this function was deleted.

‎clang/include/clang/AST/DeclTemplate.h

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,8 @@ class TemplateParameterList final
195195
///
196196
/// The constraints in the resulting list are to be treated as if in a
197197
/// conjunction ("and").
198-
void getAssociatedConstraints(llvm::SmallVectorImpl<const Expr *> &AC) const;
198+
void getAssociatedConstraints(
199+
llvm::SmallVectorImpl<AssociatedConstraint> &AC) const;
199200

200201
bool hasAssociatedConstraints() const;
201202

@@ -422,7 +423,8 @@ class TemplateDecl : public NamedDecl {
422423
/// including constraint-expressions derived from the requires-clause,
423424
/// trailing requires-clause (for functions and methods) and constrained
424425
/// template parameters.
425-
void getAssociatedConstraints(llvm::SmallVectorImpl<const Expr *> &AC) const;
426+
void getAssociatedConstraints(
427+
llvm::SmallVectorImpl<AssociatedConstraint> &AC) const;
426428

427429
bool hasAssociatedConstraints() const;
428430

@@ -1341,7 +1343,8 @@ class TemplateTypeParmDecl final : public TypeDecl,
13411343
}
13421344

13431345
void setTypeConstraint(ConceptReference *CR,
1344-
Expr *ImmediatelyDeclaredConstraint);
1346+
Expr *ImmediatelyDeclaredConstraint,
1347+
int ArgumentPackSubstitutionIndex);
13451348

13461349
/// Determine whether this template parameter has a type-constraint.
13471350
bool hasTypeConstraint() const {
@@ -1353,9 +1356,11 @@ class TemplateTypeParmDecl final : public TypeDecl,
13531356
///
13541357
/// Use this instead of getTypeConstraint for concepts APIs that
13551358
/// accept an ArrayRef of constraint expressions.
1356-
void getAssociatedConstraints(llvm::SmallVectorImpl<const Expr *> &AC) const {
1359+
void getAssociatedConstraints(
1360+
llvm::SmallVectorImpl<AssociatedConstraint> &AC) const {
13571361
if (HasTypeConstraint)
1358-
AC.push_back(getTypeConstraint()->getImmediatelyDeclaredConstraint());
1362+
AC.emplace_back(getTypeConstraint()->getImmediatelyDeclaredConstraint(),
1363+
getTypeConstraint()->getArgumentPackSubstitutionIndex());
13591364
}
13601365

13611366
SourceRange getSourceRange() const override LLVM_READONLY;
@@ -1574,9 +1579,10 @@ class NonTypeTemplateParmDecl final
15741579
///
15751580
/// Use this instead of getPlaceholderImmediatelyDeclaredConstraint for
15761581
/// concepts APIs that accept an ArrayRef of constraint expressions.
1577-
void getAssociatedConstraints(llvm::SmallVectorImpl<const Expr *> &AC) const {
1582+
void getAssociatedConstraints(
1583+
llvm::SmallVectorImpl<AssociatedConstraint> &AC) const {
15781584
if (Expr *E = getPlaceholderTypeConstraint())
1579-
AC.push_back(E);
1585+
AC.emplace_back(E, /*ArgumentPackSubstitutionIndex=*/-1);
15801586
}
15811587

15821588
// Implement isa/cast/dyncast/etc.
@@ -2169,7 +2175,8 @@ class ClassTemplatePartialSpecializationDecl
21692175
///
21702176
/// The constraints in the resulting list are to be treated as if in a
21712177
/// conjunction ("and").
2172-
void getAssociatedConstraints(llvm::SmallVectorImpl<const Expr *> &AC) const {
2178+
void getAssociatedConstraints(
2179+
llvm::SmallVectorImpl<AssociatedConstraint> &AC) const {
21732180
TemplateParams->getAssociatedConstraints(AC);
21742181
}
21752182

@@ -2943,7 +2950,8 @@ class VarTemplatePartialSpecializationDecl
29432950
///
29442951
/// The constraints in the resulting list are to be treated as if in a
29452952
/// conjunction ("and").
2946-
void getAssociatedConstraints(llvm::SmallVectorImpl<const Expr *> &AC) const {
2953+
void getAssociatedConstraints(
2954+
llvm::SmallVectorImpl<AssociatedConstraint> &AC) const {
29472955
TemplateParams->getAssociatedConstraints(AC);
29482956
}
29492957

‎clang/include/clang/Sema/Sema.h

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14550,13 +14550,14 @@ class Sema final : public SemaBase {
1455014550
/// \returns true if an error occurred and satisfaction could not be checked,
1455114551
/// false otherwise.
1455214552
bool CheckConstraintSatisfaction(
14553-
const NamedDecl *Template, ArrayRef<const Expr *> ConstraintExprs,
14553+
const NamedDecl *Template,
14554+
ArrayRef<AssociatedConstraint> AssociatedConstraints,
1455414555
const MultiLevelTemplateArgumentList &TemplateArgLists,
1455514556
SourceRange TemplateIDRange, ConstraintSatisfaction &Satisfaction) {
1455614557
llvm::SmallVector<Expr *, 4> Converted;
14557-
return CheckConstraintSatisfaction(Template, ConstraintExprs, Converted,
14558-
TemplateArgLists, TemplateIDRange,
14559-
Satisfaction);
14558+
return CheckConstraintSatisfaction(Template, AssociatedConstraints,
14559+
Converted, TemplateArgLists,
14560+
TemplateIDRange, Satisfaction);
1456014561
}
1456114562

1456214563
/// \brief Check whether the given list of constraint expressions are
@@ -14582,7 +14583,8 @@ class Sema final : public SemaBase {
1458214583
/// \returns true if an error occurred and satisfaction could not be checked,
1458314584
/// false otherwise.
1458414585
bool CheckConstraintSatisfaction(
14585-
const NamedDecl *Template, ArrayRef<const Expr *> ConstraintExprs,
14586+
const NamedDecl *Template,
14587+
ArrayRef<AssociatedConstraint> AssociatedConstraints,
1458614588
llvm::SmallVectorImpl<Expr *> &ConvertedConstraints,
1458714589
const MultiLevelTemplateArgumentList &TemplateArgList,
1458814590
SourceRange TemplateIDRange, ConstraintSatisfaction &Satisfaction);
@@ -14659,7 +14661,8 @@ class Sema final : public SemaBase {
1465914661
bool First = true);
1466014662

1466114663
const NormalizedConstraint *getNormalizedAssociatedConstraints(
14662-
NamedDecl *ConstrainedDecl, ArrayRef<const Expr *> AssociatedConstraints);
14664+
NamedDecl *ConstrainedDecl,
14665+
ArrayRef<AssociatedConstraint> AssociatedConstraints);
1466314666

1466414667
/// \brief Check whether the given declaration's associated constraints are
1466514668
/// at least as constrained than another declaration's according to the
@@ -14669,17 +14672,19 @@ class Sema final : public SemaBase {
1466914672
/// at least constrained than D2, and false otherwise.
1467014673
///
1467114674
/// \returns true if an error occurred, false otherwise.
14672-
bool IsAtLeastAsConstrained(NamedDecl *D1, MutableArrayRef<const Expr *> AC1,
14673-
NamedDecl *D2, MutableArrayRef<const Expr *> AC2,
14675+
bool IsAtLeastAsConstrained(NamedDecl *D1,
14676+
MutableArrayRef<AssociatedConstraint> AC1,
14677+
NamedDecl *D2,
14678+
MutableArrayRef<AssociatedConstraint> AC2,
1467414679
bool &Result);
1467514680

1467614681
/// If D1 was not at least as constrained as D2, but would've been if a pair
1467714682
/// of atomic constraints involved had been declared in a concept and not
1467814683
/// repeated in two separate places in code.
1467914684
/// \returns true if such a diagnostic was emitted, false otherwise.
1468014685
bool MaybeEmitAmbiguousAtomicConstraintsDiagnostic(
14681-
NamedDecl *D1, ArrayRef<const Expr *> AC1, NamedDecl *D2,
14682-
ArrayRef<const Expr *> AC2);
14686+
NamedDecl *D1, ArrayRef<AssociatedConstraint> AC1, NamedDecl *D2,
14687+
ArrayRef<AssociatedConstraint> AC2);
1468314688

1468414689
private:
1468514690
/// Caches pairs of template-like decls whose associated constraints were

‎clang/include/clang/Sema/SemaConcept.h

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,12 @@ class Sema;
2929
enum { ConstraintAlignment = 8 };
3030

3131
struct alignas(ConstraintAlignment) AtomicConstraint {
32-
const Expr *ConstraintExpr;
32+
AssociatedConstraint AC;
3333
NamedDecl *ConstraintDecl;
3434
std::optional<ArrayRef<TemplateArgumentLoc>> ParameterMapping;
3535

36-
AtomicConstraint(const Expr *ConstraintExpr, NamedDecl *ConstraintDecl)
37-
: ConstraintExpr(ConstraintExpr), ConstraintDecl(ConstraintDecl) {};
36+
AtomicConstraint(const AssociatedConstraint &AC, NamedDecl *ConstraintDecl)
37+
: AC(AC), ConstraintDecl(ConstraintDecl) {};
3838

3939
bool hasMatchingParameterMapping(ASTContext &C,
4040
const AtomicConstraint &Other) const {
@@ -70,7 +70,14 @@ struct alignas(ConstraintAlignment) AtomicConstraint {
7070
// We do not actually substitute the parameter mappings into the
7171
// constraint expressions, therefore the constraint expressions are
7272
// the originals, and comparing them will suffice.
73-
if (ConstraintExpr != Other.ConstraintExpr)
73+
if (AC.ConstraintExpr != Other.AC.ConstraintExpr)
74+
return false;
75+
76+
// FIXME: As the normalization cache doesn't take
77+
// ArgumentPackSubstitutionIndex into account,
78+
// this won't have an effect.
79+
if (AC.ArgumentPackSubstitutionIndex !=
80+
Other.AC.ArgumentPackSubstitutionIndex)
7481
return false;
7582

7683
// Check that the parameter lists are identical
@@ -152,9 +159,11 @@ struct NormalizedConstraint {
152159

153160
private:
154161
static std::optional<NormalizedConstraint>
155-
fromConstraintExprs(Sema &S, NamedDecl *D, ArrayRef<const Expr *> E);
162+
fromConstraintAssociatedConstraints(Sema &S, NamedDecl *D,
163+
ArrayRef<AssociatedConstraint> ACs);
156164
static std::optional<NormalizedConstraint>
157-
fromConstraintExpr(Sema &S, NamedDecl *D, const Expr *E);
165+
fromConstraintAssociatedConstraint(Sema &S, NamedDecl *D,
166+
AssociatedConstraint AC);
158167
};
159168

160169
struct alignas(ConstraintAlignment) NormalizedConstraintPair {
@@ -180,7 +189,7 @@ struct alignas(ConstraintAlignment) FoldExpandedConstraint {
180189

181190
const NormalizedConstraint *getNormalizedAssociatedConstraints(
182191
Sema &S, NamedDecl *ConstrainedDecl,
183-
ArrayRef<const Expr *> AssociatedConstraints);
192+
ArrayRef<AssociatedConstraint> AssociatedConstraints);
184193

185194
template <typename AtomicSubsumptionEvaluator>
186195
bool subsumes(const NormalForm &PDNF, const NormalForm &QCNF,
@@ -226,8 +235,8 @@ bool subsumes(const NormalForm &PDNF, const NormalForm &QCNF,
226235
}
227236

228237
template <typename AtomicSubsumptionEvaluator>
229-
bool subsumes(Sema &S, NamedDecl *DP, ArrayRef<const Expr *> P, NamedDecl *DQ,
230-
ArrayRef<const Expr *> Q, bool &Subsumes,
238+
bool subsumes(Sema &S, NamedDecl *DP, ArrayRef<AssociatedConstraint> P,
239+
NamedDecl *DQ, ArrayRef<AssociatedConstraint> Q, bool &Subsumes,
231240
const AtomicSubsumptionEvaluator &E) {
232241
// C++ [temp.constr.order] p2
233242
// In order to determine if a constraint P subsumes a constraint Q, P is

‎clang/lib/AST/ASTImporter.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5983,7 +5983,8 @@ ASTNodeImporter::VisitTemplateTypeParmDecl(TemplateTypeParmDecl *D) {
59835983
if (Err)
59845984
return std::move(Err);
59855985

5986-
ToD->setTypeConstraint(ToConceptRef, ToIDC);
5986+
ToD->setTypeConstraint(ToConceptRef, ToIDC,
5987+
TC->getArgumentPackSubstitutionIndex());
59875988
}
59885989

59895990
if (Error Err = importTemplateParameterDefaultArgument(D, ToD))

‎clang/lib/AST/DeclTemplate.cpp

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -223,20 +223,21 @@ static bool AdoptTemplateParameterList(TemplateParameterList *Params,
223223
return Invalid;
224224
}
225225

226-
void TemplateParameterList::
227-
getAssociatedConstraints(llvm::SmallVectorImpl<const Expr *> &AC) const {
226+
void TemplateParameterList::getAssociatedConstraints(
227+
llvm::SmallVectorImpl<AssociatedConstraint> &ACs) const {
228228
if (HasConstrainedParameters)
229229
for (const NamedDecl *Param : *this) {
230230
if (const auto *TTP = dyn_cast<TemplateTypeParmDecl>(Param)) {
231231
if (const auto *TC = TTP->getTypeConstraint())
232-
AC.push_back(TC->getImmediatelyDeclaredConstraint());
232+
ACs.emplace_back(TC->getImmediatelyDeclaredConstraint(),
233+
TC->getArgumentPackSubstitutionIndex());
233234
} else if (const auto *NTTP = dyn_cast<NonTypeTemplateParmDecl>(Param)) {
234235
if (const Expr *E = NTTP->getPlaceholderTypeConstraint())
235-
AC.push_back(E);
236+
ACs.emplace_back(E, /*ArgumentPackSubstitutionIndex=*/-1);
236237
}
237238
}
238239
if (HasRequiresClause)
239-
AC.push_back(getRequiresClause());
240+
ACs.emplace_back(getRequiresClause(), /*ArgumentPackSubstitutionIndex=*/-1);
240241
}
241242

242243
bool TemplateParameterList::hasAssociatedConstraints() const {
@@ -286,12 +287,12 @@ TemplateDecl::TemplateDecl(Kind DK, DeclContext *DC, SourceLocation L,
286287

287288
void TemplateDecl::anchor() {}
288289

289-
void TemplateDecl::
290-
getAssociatedConstraints(llvm::SmallVectorImpl<const Expr *> &AC) const {
291-
TemplateParams->getAssociatedConstraints(AC);
290+
void TemplateDecl::getAssociatedConstraints(
291+
llvm::SmallVectorImpl<AssociatedConstraint> &ACs) const {
292+
TemplateParams->getAssociatedConstraints(ACs);
292293
if (auto *FD = dyn_cast_or_null<FunctionDecl>(getTemplatedDecl()))
293294
if (const Expr *TRC = FD->getTrailingRequiresClause())
294-
AC.push_back(TRC);
295+
ACs.emplace_back(TRC, /*ArgumentPackSubstitutionIndex=*/-1);
295296
}
296297

297298
bool TemplateDecl::hasAssociatedConstraints() const {
@@ -748,14 +749,15 @@ bool TemplateTypeParmDecl::isParameterPack() const {
748749
}
749750

750751
void TemplateTypeParmDecl::setTypeConstraint(
751-
ConceptReference *Loc, Expr *ImmediatelyDeclaredConstraint) {
752+
ConceptReference *Loc, Expr *ImmediatelyDeclaredConstraint,
753+
int ArgumentPackSubstitutionIndex) {
752754
assert(HasTypeConstraint &&
753755
"HasTypeConstraint=true must be passed at construction in order to "
754756
"call setTypeConstraint");
755757
assert(!TypeConstraintInitialized &&
756758
"TypeConstraint was already initialized!");
757-
new (getTrailingObjects<TypeConstraint>())
758-
TypeConstraint(Loc, ImmediatelyDeclaredConstraint);
759+
new (getTrailingObjects<TypeConstraint>()) TypeConstraint(
760+
Loc, ImmediatelyDeclaredConstraint, ArgumentPackSubstitutionIndex);
759761
TypeConstraintInitialized = true;
760762
}
761763

‎clang/lib/Sema/SemaCodeComplete.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5463,8 +5463,9 @@ class ConceptInfo {
54635463
// that T is attached to in order to gather the relevant constraints.
54645464
ConceptInfo(const TemplateTypeParmType &BaseType, Scope *S) {
54655465
auto *TemplatedEntity = getTemplatedEntity(BaseType.getDecl(), S);
5466-
for (const Expr *E : constraintsForTemplatedEntity(TemplatedEntity))
5467-
believe(E, &BaseType);
5466+
for (const AssociatedConstraint &AC :
5467+
constraintsForTemplatedEntity(TemplatedEntity))
5468+
believe(AC.ConstraintExpr, &BaseType);
54685469
}
54695470

54705471
std::vector<Member> members() {
@@ -5696,9 +5697,9 @@ class ConceptInfo {
56965697

56975698
// Gets all the type constraint expressions that might apply to the type
56985699
// variables associated with DC (as returned by getTemplatedEntity()).
5699-
static SmallVector<const Expr *, 1>
5700+
static SmallVector<AssociatedConstraint, 1>
57005701
constraintsForTemplatedEntity(DeclContext *DC) {
5701-
SmallVector<const Expr *, 1> Result;
5702+
SmallVector<AssociatedConstraint, 1> Result;
57025703
if (DC == nullptr)
57035704
return Result;
57045705
// Primary templates can have constraints.

0 commit comments

Comments
 (0)