Skip to content

Commit df335b0

Browse files
zyn0217zyn0217
authored
[Clang] Preserve partially substituted pack indexing type/expressions (#116782)
Substituting into pack indexing types/expressions can still result in unexpanded types/expressions, such as `PackIndexingType` or `PackIndexingExpr`. To handle these cases correctly, we should defer the pack size checks to the next round of transformation, when the patterns can be fully expanded. To that end, the `FullySubstituted` flag is now necessary for computing the dependencies of `PackIndexingExprs`. Conveniently, this flag can also represent the prior `ExpandsToEmpty` status with an additional emptiness check. Therefore, I converted all stored flags to use `FullySubstituted`. Fixes #116105
1 parent 2585b6e commit df335b0

File tree

14 files changed

+85
-42
lines changed

14 files changed

+85
-42
lines changed

‎clang/docs/ReleaseNotes.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -714,6 +714,7 @@ Bug Fixes to C++ Support
714714
assumption if they also occur inside of a dependent lambda. (#GH114787)
715715
- Clang now uses valid deduced type locations when diagnosing functions with trailing return type
716716
missing placeholder return type. (#GH78694)
717+
- Fixed a bug where bounds of partially expanded pack indexing expressions were checked too early. (#GH116105)
717718

718719
Bug Fixes to AST Handling
719720
^^^^^^^^^^^^^^^^^^^^^^^^^

‎clang/include/clang/AST/ExprCXX.h

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4390,17 +4390,17 @@ class PackIndexingExpr final
43904390
unsigned TransformedExpressions : 31;
43914391

43924392
LLVM_PREFERRED_TYPE(bool)
4393-
unsigned ExpandedToEmptyPack : 1;
4393+
unsigned FullySubstituted : 1;
43944394

43954395
PackIndexingExpr(QualType Type, SourceLocation EllipsisLoc,
43964396
SourceLocation RSquareLoc, Expr *PackIdExpr, Expr *IndexExpr,
43974397
ArrayRef<Expr *> SubstitutedExprs = {},
4398-
bool ExpandedToEmptyPack = false)
4398+
bool FullySubstituted = false)
43994399
: Expr(PackIndexingExprClass, Type, VK_LValue, OK_Ordinary),
44004400
EllipsisLoc(EllipsisLoc), RSquareLoc(RSquareLoc),
44014401
SubExprs{PackIdExpr, IndexExpr},
44024402
TransformedExpressions(SubstitutedExprs.size()),
4403-
ExpandedToEmptyPack(ExpandedToEmptyPack) {
4403+
FullySubstituted(FullySubstituted) {
44044404

44054405
auto *Exprs = getTrailingObjects<Expr *>();
44064406
std::uninitialized_copy(SubstitutedExprs.begin(), SubstitutedExprs.end(),
@@ -4424,12 +4424,16 @@ class PackIndexingExpr final
44244424
SourceLocation RSquareLoc, Expr *PackIdExpr,
44254425
Expr *IndexExpr, std::optional<int64_t> Index,
44264426
ArrayRef<Expr *> SubstitutedExprs = {},
4427-
bool ExpandedToEmptyPack = false);
4427+
bool FullySubstituted = false);
44284428
static PackIndexingExpr *CreateDeserialized(ASTContext &Context,
44294429
unsigned NumTransformedExprs);
44304430

4431+
bool isFullySubstituted() const { return FullySubstituted; }
4432+
44314433
/// Determine if the expression was expanded to empty.
4432-
bool expandsToEmptyPack() const { return ExpandedToEmptyPack; }
4434+
bool expandsToEmptyPack() const {
4435+
return isFullySubstituted() && TransformedExpressions == 0;
4436+
}
44334437

44344438
/// Determine the location of the 'sizeof' keyword.
44354439
SourceLocation getEllipsisLoc() const { return EllipsisLoc; }

‎clang/include/clang/AST/Type.h

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5922,12 +5922,12 @@ class PackIndexingType final
59225922
unsigned Size : 31;
59235923

59245924
LLVM_PREFERRED_TYPE(bool)
5925-
unsigned ExpandsToEmptyPack : 1;
5925+
unsigned FullySubstituted : 1;
59265926

59275927
protected:
59285928
friend class ASTContext; // ASTContext creates these.
59295929
PackIndexingType(const ASTContext &Context, QualType Canonical,
5930-
QualType Pattern, Expr *IndexExpr, bool ExpandsToEmptyPack,
5930+
QualType Pattern, Expr *IndexExpr, bool FullySubstituted,
59315931
ArrayRef<QualType> Expansions = {});
59325932

59335933
public:
@@ -5951,7 +5951,9 @@ class PackIndexingType final
59515951

59525952
bool hasSelectedType() const { return getSelectedIndex() != std::nullopt; }
59535953

5954-
bool expandsToEmptyPack() const { return ExpandsToEmptyPack; }
5954+
bool isFullySubstituted() const { return FullySubstituted; }
5955+
5956+
bool expandsToEmptyPack() const { return isFullySubstituted() && Size == 0; }
59555957

59565958
ArrayRef<QualType> getExpansions() const {
59575959
return {getExpansionsPtr(), Size};
@@ -5965,10 +5967,10 @@ class PackIndexingType final
59655967
if (hasSelectedType())
59665968
getSelectedType().Profile(ID);
59675969
else
5968-
Profile(ID, Context, getPattern(), getIndexExpr(), expandsToEmptyPack());
5970+
Profile(ID, Context, getPattern(), getIndexExpr(), isFullySubstituted());
59695971
}
59705972
static void Profile(llvm::FoldingSetNodeID &ID, const ASTContext &Context,
5971-
QualType Pattern, Expr *E, bool ExpandsToEmptyPack);
5973+
QualType Pattern, Expr *E, bool FullySubstituted);
59725974

59735975
private:
59745976
const QualType *getExpansionsPtr() const {

‎clang/include/clang/AST/TypeProperties.td

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -473,12 +473,12 @@ let Class = PackIndexingType in {
473473
def : Property<"indexExpression", ExprRef> {
474474
let Read = [{ node->getIndexExpr() }];
475475
}
476-
def : Property<"expandsToEmptyPack", Bool> {
477-
let Read = [{ node->expandsToEmptyPack() }];
476+
def : Property<"isFullySubstituted", Bool> {
477+
let Read = [{ node->isFullySubstituted() }];
478478
}
479479

480480
def : Creator<[{
481-
return ctx.getPackIndexingType(pattern, indexExpression, expandsToEmptyPack);
481+
return ctx.getPackIndexingType(pattern, indexExpression, isFullySubstituted);
482482
}]>;
483483
}
484484

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14257,7 +14257,7 @@ class Sema final : public SemaBase {
1425714257
SourceLocation EllipsisLoc, Expr *IndexExpr,
1425814258
SourceLocation RSquareLoc,
1425914259
ArrayRef<Expr *> ExpandedExprs = {},
14260-
bool EmptyPack = false);
14260+
bool FullySubstituted = false);
1426114261

1426214262
/// Handle a C++1z fold-expression: ( expr op ... op expr ).
1426314263
ExprResult ActOnCXXFoldExpr(Scope *S, SourceLocation LParenLoc, Expr *LHS,

‎clang/lib/AST/ASTContext.cpp

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6223,13 +6223,11 @@ QualType ASTContext::getPackIndexingType(QualType Pattern, Expr *IndexExpr,
62236223
ArrayRef<QualType> Expansions,
62246224
int Index) const {
62256225
QualType Canonical;
6226-
bool ExpandsToEmptyPack = FullySubstituted && Expansions.empty();
62276226
if (FullySubstituted && Index != -1) {
62286227
Canonical = getCanonicalType(Expansions[Index]);
62296228
} else {
62306229
llvm::FoldingSetNodeID ID;
6231-
PackIndexingType::Profile(ID, *this, Pattern, IndexExpr,
6232-
ExpandsToEmptyPack);
6230+
PackIndexingType::Profile(ID, *this, Pattern, IndexExpr, FullySubstituted);
62336231
void *InsertPos = nullptr;
62346232
PackIndexingType *Canon =
62356233
DependentPackIndexingTypes.FindNodeOrInsertPos(ID, InsertPos);
@@ -6238,7 +6236,7 @@ QualType ASTContext::getPackIndexingType(QualType Pattern, Expr *IndexExpr,
62386236
PackIndexingType::totalSizeToAlloc<QualType>(Expansions.size()),
62396237
TypeAlignment);
62406238
Canon = new (Mem) PackIndexingType(*this, QualType(), Pattern, IndexExpr,
6241-
ExpandsToEmptyPack, Expansions);
6239+
FullySubstituted, Expansions);
62426240
DependentPackIndexingTypes.InsertNode(Canon, InsertPos);
62436241
}
62446242
Canonical = QualType(Canon, 0);
@@ -6248,7 +6246,7 @@ QualType ASTContext::getPackIndexingType(QualType Pattern, Expr *IndexExpr,
62486246
Allocate(PackIndexingType::totalSizeToAlloc<QualType>(Expansions.size()),
62496247
TypeAlignment);
62506248
auto *T = new (Mem) PackIndexingType(*this, Canonical, Pattern, IndexExpr,
6251-
ExpandsToEmptyPack, Expansions);
6249+
FullySubstituted, Expansions);
62526250
Types.push_back(T);
62536251
return QualType(T, 0);
62546252
}

‎clang/lib/AST/ComputeDependence.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -388,9 +388,8 @@ ExprDependence clang::computeDependence(PackIndexingExpr *E) {
388388
ExprDependence::Instantiation;
389389

390390
ArrayRef<Expr *> Exprs = E->getExpressions();
391-
if (Exprs.empty())
391+
if (Exprs.empty() || !E->isFullySubstituted())
392392
D |= PatternDep | ExprDependence::Instantiation;
393-
394393
else if (!E->getIndexExpr()->isInstantiationDependent()) {
395394
std::optional<unsigned> Index = E->getSelectedIndex();
396395
assert(Index && *Index < Exprs.size() && "pack index out of bound");

‎clang/lib/AST/ExprCXX.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1717,9 +1717,9 @@ NonTypeTemplateParmDecl *SubstNonTypeTemplateParmExpr::getParameter() const {
17171717
PackIndexingExpr *PackIndexingExpr::Create(
17181718
ASTContext &Context, SourceLocation EllipsisLoc, SourceLocation RSquareLoc,
17191719
Expr *PackIdExpr, Expr *IndexExpr, std::optional<int64_t> Index,
1720-
ArrayRef<Expr *> SubstitutedExprs, bool ExpandedToEmptyPack) {
1720+
ArrayRef<Expr *> SubstitutedExprs, bool FullySubstituted) {
17211721
QualType Type;
1722-
if (Index && !SubstitutedExprs.empty())
1722+
if (Index && FullySubstituted && !SubstitutedExprs.empty())
17231723
Type = SubstitutedExprs[*Index]->getType();
17241724
else
17251725
Type = Context.DependentTy;
@@ -1728,7 +1728,7 @@ PackIndexingExpr *PackIndexingExpr::Create(
17281728
Context.Allocate(totalSizeToAlloc<Expr *>(SubstitutedExprs.size()));
17291729
return new (Storage)
17301730
PackIndexingExpr(Type, EllipsisLoc, RSquareLoc, PackIdExpr, IndexExpr,
1731-
SubstitutedExprs, ExpandedToEmptyPack);
1731+
SubstitutedExprs, FullySubstituted);
17321732
}
17331733

17341734
NamedDecl *PackIndexingExpr::getPackDecl() const {

‎clang/lib/AST/Type.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4031,12 +4031,12 @@ void DependentDecltypeType::Profile(llvm::FoldingSetNodeID &ID,
40314031

40324032
PackIndexingType::PackIndexingType(const ASTContext &Context,
40334033
QualType Canonical, QualType Pattern,
4034-
Expr *IndexExpr, bool ExpandsToEmptyPack,
4034+
Expr *IndexExpr, bool FullySubstituted,
40354035
ArrayRef<QualType> Expansions)
40364036
: Type(PackIndexing, Canonical,
40374037
computeDependence(Pattern, IndexExpr, Expansions)),
40384038
Context(Context), Pattern(Pattern), IndexExpr(IndexExpr),
4039-
Size(Expansions.size()), ExpandsToEmptyPack(ExpandsToEmptyPack) {
4039+
Size(Expansions.size()), FullySubstituted(FullySubstituted) {
40404040

40414041
std::uninitialized_copy(Expansions.begin(), Expansions.end(),
40424042
getTrailingObjects<QualType>());
@@ -4081,10 +4081,10 @@ PackIndexingType::computeDependence(QualType Pattern, Expr *IndexExpr,
40814081

40824082
void PackIndexingType::Profile(llvm::FoldingSetNodeID &ID,
40834083
const ASTContext &Context, QualType Pattern,
4084-
Expr *E, bool ExpandsToEmptyPack) {
4084+
Expr *E, bool FullySubstituted) {
40854085
Pattern.Profile(ID);
40864086
E->Profile(ID, Context, true);
4087-
ID.AddBoolean(ExpandsToEmptyPack);
4087+
ID.AddBoolean(FullySubstituted);
40884088
}
40894089

40904090
UnaryTransformType::UnaryTransformType(QualType BaseType,

‎clang/lib/Sema/SemaTemplateVariadic.cpp

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1157,10 +1157,12 @@ ExprResult Sema::ActOnPackIndexingExpr(Scope *S, Expr *PackExpression,
11571157
return Res;
11581158
}
11591159

1160-
ExprResult
1161-
Sema::BuildPackIndexingExpr(Expr *PackExpression, SourceLocation EllipsisLoc,
1162-
Expr *IndexExpr, SourceLocation RSquareLoc,
1163-
ArrayRef<Expr *> ExpandedExprs, bool EmptyPack) {
1160+
ExprResult Sema::BuildPackIndexingExpr(Expr *PackExpression,
1161+
SourceLocation EllipsisLoc,
1162+
Expr *IndexExpr,
1163+
SourceLocation RSquareLoc,
1164+
ArrayRef<Expr *> ExpandedExprs,
1165+
bool FullySubstituted) {
11641166

11651167
std::optional<int64_t> Index;
11661168
if (!IndexExpr->isInstantiationDependent()) {
@@ -1174,8 +1176,8 @@ Sema::BuildPackIndexingExpr(Expr *PackExpression, SourceLocation EllipsisLoc,
11741176
IndexExpr = Res.get();
11751177
}
11761178

1177-
if (Index && (!ExpandedExprs.empty() || EmptyPack)) {
1178-
if (*Index < 0 || EmptyPack || *Index >= int64_t(ExpandedExprs.size())) {
1179+
if (Index && FullySubstituted) {
1180+
if (*Index < 0 || *Index >= int64_t(ExpandedExprs.size())) {
11791181
Diag(PackExpression->getBeginLoc(), diag::err_pack_index_out_of_bound)
11801182
<< *Index << PackExpression << ExpandedExprs.size();
11811183
return ExprError();
@@ -1184,7 +1186,7 @@ Sema::BuildPackIndexingExpr(Expr *PackExpression, SourceLocation EllipsisLoc,
11841186

11851187
return PackIndexingExpr::Create(getASTContext(), EllipsisLoc, RSquareLoc,
11861188
PackExpression, IndexExpr, Index,
1187-
ExpandedExprs, EmptyPack);
1189+
ExpandedExprs, FullySubstituted);
11881190
}
11891191

11901192
TemplateArgumentLoc Sema::getTemplateArgumentPackExpansionPattern(

0 commit comments

Comments
 (0)