[clang] df335b0 - [Clang] Preserve partially substituted pack indexing type/expressions (#116782)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Nov 25 00:16:43 PST 2024
Author: Younan Zhang
Date: 2024-11-25T16:16:39+08:00
New Revision: df335b09eac8a48d9afc06d71a86646ff6b08131
URL: https://github.com/llvm/llvm-project/commit/df335b09eac8a48d9afc06d71a86646ff6b08131
DIFF: https://github.com/llvm/llvm-project/commit/df335b09eac8a48d9afc06d71a86646ff6b08131.diff
LOG: [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 https://github.com/llvm/llvm-project/issues/116105
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/include/clang/AST/ExprCXX.h
clang/include/clang/AST/Type.h
clang/include/clang/AST/TypeProperties.td
clang/include/clang/Sema/Sema.h
clang/lib/AST/ASTContext.cpp
clang/lib/AST/ComputeDependence.cpp
clang/lib/AST/ExprCXX.cpp
clang/lib/AST/Type.cpp
clang/lib/Sema/SemaTemplateVariadic.cpp
clang/lib/Sema/TreeTransform.h
clang/lib/Serialization/ASTReaderStmt.cpp
clang/lib/Serialization/ASTWriterStmt.cpp
clang/test/SemaCXX/cxx2c-pack-indexing.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 8bd06fadfdc984..654204557ad913 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -714,6 +714,7 @@ Bug Fixes to C++ Support
assumption if they also occur inside of a dependent lambda. (#GH114787)
- Clang now uses valid deduced type locations when diagnosing functions with trailing return type
missing placeholder return type. (#GH78694)
+- Fixed a bug where bounds of partially expanded pack indexing expressions were checked too early. (#GH116105)
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h
index 696a574833dad2..1a24b8857674ca 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -4390,17 +4390,17 @@ class PackIndexingExpr final
unsigned TransformedExpressions : 31;
LLVM_PREFERRED_TYPE(bool)
- unsigned ExpandedToEmptyPack : 1;
+ unsigned FullySubstituted : 1;
PackIndexingExpr(QualType Type, SourceLocation EllipsisLoc,
SourceLocation RSquareLoc, Expr *PackIdExpr, Expr *IndexExpr,
ArrayRef<Expr *> SubstitutedExprs = {},
- bool ExpandedToEmptyPack = false)
+ bool FullySubstituted = false)
: Expr(PackIndexingExprClass, Type, VK_LValue, OK_Ordinary),
EllipsisLoc(EllipsisLoc), RSquareLoc(RSquareLoc),
SubExprs{PackIdExpr, IndexExpr},
TransformedExpressions(SubstitutedExprs.size()),
- ExpandedToEmptyPack(ExpandedToEmptyPack) {
+ FullySubstituted(FullySubstituted) {
auto *Exprs = getTrailingObjects<Expr *>();
std::uninitialized_copy(SubstitutedExprs.begin(), SubstitutedExprs.end(),
@@ -4424,12 +4424,16 @@ class PackIndexingExpr final
SourceLocation RSquareLoc, Expr *PackIdExpr,
Expr *IndexExpr, std::optional<int64_t> Index,
ArrayRef<Expr *> SubstitutedExprs = {},
- bool ExpandedToEmptyPack = false);
+ bool FullySubstituted = false);
static PackIndexingExpr *CreateDeserialized(ASTContext &Context,
unsigned NumTransformedExprs);
+ bool isFullySubstituted() const { return FullySubstituted; }
+
/// Determine if the expression was expanded to empty.
- bool expandsToEmptyPack() const { return ExpandedToEmptyPack; }
+ bool expandsToEmptyPack() const {
+ return isFullySubstituted() && TransformedExpressions == 0;
+ }
/// Determine the location of the 'sizeof' keyword.
SourceLocation getEllipsisLoc() const { return EllipsisLoc; }
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 1ed5c22361ca68..90a52b1dcbf624 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -5922,12 +5922,12 @@ class PackIndexingType final
unsigned Size : 31;
LLVM_PREFERRED_TYPE(bool)
- unsigned ExpandsToEmptyPack : 1;
+ unsigned FullySubstituted : 1;
protected:
friend class ASTContext; // ASTContext creates these.
PackIndexingType(const ASTContext &Context, QualType Canonical,
- QualType Pattern, Expr *IndexExpr, bool ExpandsToEmptyPack,
+ QualType Pattern, Expr *IndexExpr, bool FullySubstituted,
ArrayRef<QualType> Expansions = {});
public:
@@ -5951,7 +5951,9 @@ class PackIndexingType final
bool hasSelectedType() const { return getSelectedIndex() != std::nullopt; }
- bool expandsToEmptyPack() const { return ExpandsToEmptyPack; }
+ bool isFullySubstituted() const { return FullySubstituted; }
+
+ bool expandsToEmptyPack() const { return isFullySubstituted() && Size == 0; }
ArrayRef<QualType> getExpansions() const {
return {getExpansionsPtr(), Size};
@@ -5965,10 +5967,10 @@ class PackIndexingType final
if (hasSelectedType())
getSelectedType().Profile(ID);
else
- Profile(ID, Context, getPattern(), getIndexExpr(), expandsToEmptyPack());
+ Profile(ID, Context, getPattern(), getIndexExpr(), isFullySubstituted());
}
static void Profile(llvm::FoldingSetNodeID &ID, const ASTContext &Context,
- QualType Pattern, Expr *E, bool ExpandsToEmptyPack);
+ QualType Pattern, Expr *E, bool FullySubstituted);
private:
const QualType *getExpansionsPtr() const {
diff --git a/clang/include/clang/AST/TypeProperties.td b/clang/include/clang/AST/TypeProperties.td
index a8b9c920b617c0..6f1a76bd18fb50 100644
--- a/clang/include/clang/AST/TypeProperties.td
+++ b/clang/include/clang/AST/TypeProperties.td
@@ -473,12 +473,12 @@ let Class = PackIndexingType in {
def : Property<"indexExpression", ExprRef> {
let Read = [{ node->getIndexExpr() }];
}
- def : Property<"expandsToEmptyPack", Bool> {
- let Read = [{ node->expandsToEmptyPack() }];
+ def : Property<"isFullySubstituted", Bool> {
+ let Read = [{ node->isFullySubstituted() }];
}
def : Creator<[{
- return ctx.getPackIndexingType(pattern, indexExpression, expandsToEmptyPack);
+ return ctx.getPackIndexingType(pattern, indexExpression, isFullySubstituted);
}]>;
}
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 5fe23e0d0efd3b..24abd5d95dd844 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -14257,7 +14257,7 @@ class Sema final : public SemaBase {
SourceLocation EllipsisLoc, Expr *IndexExpr,
SourceLocation RSquareLoc,
ArrayRef<Expr *> ExpandedExprs = {},
- bool EmptyPack = false);
+ bool FullySubstituted = false);
/// Handle a C++1z fold-expression: ( expr op ... op expr ).
ExprResult ActOnCXXFoldExpr(Scope *S, SourceLocation LParenLoc, Expr *LHS,
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 3e6f0d628ca926..80e8c5b9df58e7 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -6223,13 +6223,11 @@ QualType ASTContext::getPackIndexingType(QualType Pattern, Expr *IndexExpr,
ArrayRef<QualType> Expansions,
int Index) const {
QualType Canonical;
- bool ExpandsToEmptyPack = FullySubstituted && Expansions.empty();
if (FullySubstituted && Index != -1) {
Canonical = getCanonicalType(Expansions[Index]);
} else {
llvm::FoldingSetNodeID ID;
- PackIndexingType::Profile(ID, *this, Pattern, IndexExpr,
- ExpandsToEmptyPack);
+ PackIndexingType::Profile(ID, *this, Pattern, IndexExpr, FullySubstituted);
void *InsertPos = nullptr;
PackIndexingType *Canon =
DependentPackIndexingTypes.FindNodeOrInsertPos(ID, InsertPos);
@@ -6238,7 +6236,7 @@ QualType ASTContext::getPackIndexingType(QualType Pattern, Expr *IndexExpr,
PackIndexingType::totalSizeToAlloc<QualType>(Expansions.size()),
TypeAlignment);
Canon = new (Mem) PackIndexingType(*this, QualType(), Pattern, IndexExpr,
- ExpandsToEmptyPack, Expansions);
+ FullySubstituted, Expansions);
DependentPackIndexingTypes.InsertNode(Canon, InsertPos);
}
Canonical = QualType(Canon, 0);
@@ -6248,7 +6246,7 @@ QualType ASTContext::getPackIndexingType(QualType Pattern, Expr *IndexExpr,
Allocate(PackIndexingType::totalSizeToAlloc<QualType>(Expansions.size()),
TypeAlignment);
auto *T = new (Mem) PackIndexingType(*this, Canonical, Pattern, IndexExpr,
- ExpandsToEmptyPack, Expansions);
+ FullySubstituted, Expansions);
Types.push_back(T);
return QualType(T, 0);
}
diff --git a/clang/lib/AST/ComputeDependence.cpp b/clang/lib/AST/ComputeDependence.cpp
index e37ebec0851951..07c4419e3cf407 100644
--- a/clang/lib/AST/ComputeDependence.cpp
+++ b/clang/lib/AST/ComputeDependence.cpp
@@ -388,9 +388,8 @@ ExprDependence clang::computeDependence(PackIndexingExpr *E) {
ExprDependence::Instantiation;
ArrayRef<Expr *> Exprs = E->getExpressions();
- if (Exprs.empty())
+ if (Exprs.empty() || !E->isFullySubstituted())
D |= PatternDep | ExprDependence::Instantiation;
-
else if (!E->getIndexExpr()->isInstantiationDependent()) {
std::optional<unsigned> Index = E->getSelectedIndex();
assert(Index && *Index < Exprs.size() && "pack index out of bound");
diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index 678c9245ab46e6..fc09d24fc30cb4 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -1717,9 +1717,9 @@ NonTypeTemplateParmDecl *SubstNonTypeTemplateParmExpr::getParameter() const {
PackIndexingExpr *PackIndexingExpr::Create(
ASTContext &Context, SourceLocation EllipsisLoc, SourceLocation RSquareLoc,
Expr *PackIdExpr, Expr *IndexExpr, std::optional<int64_t> Index,
- ArrayRef<Expr *> SubstitutedExprs, bool ExpandedToEmptyPack) {
+ ArrayRef<Expr *> SubstitutedExprs, bool FullySubstituted) {
QualType Type;
- if (Index && !SubstitutedExprs.empty())
+ if (Index && FullySubstituted && !SubstitutedExprs.empty())
Type = SubstitutedExprs[*Index]->getType();
else
Type = Context.DependentTy;
@@ -1728,7 +1728,7 @@ PackIndexingExpr *PackIndexingExpr::Create(
Context.Allocate(totalSizeToAlloc<Expr *>(SubstitutedExprs.size()));
return new (Storage)
PackIndexingExpr(Type, EllipsisLoc, RSquareLoc, PackIdExpr, IndexExpr,
- SubstitutedExprs, ExpandedToEmptyPack);
+ SubstitutedExprs, FullySubstituted);
}
NamedDecl *PackIndexingExpr::getPackDecl() const {
diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index b70f86ef31442d..edf20944f0b3ed 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -4031,12 +4031,12 @@ void DependentDecltypeType::Profile(llvm::FoldingSetNodeID &ID,
PackIndexingType::PackIndexingType(const ASTContext &Context,
QualType Canonical, QualType Pattern,
- Expr *IndexExpr, bool ExpandsToEmptyPack,
+ Expr *IndexExpr, bool FullySubstituted,
ArrayRef<QualType> Expansions)
: Type(PackIndexing, Canonical,
computeDependence(Pattern, IndexExpr, Expansions)),
Context(Context), Pattern(Pattern), IndexExpr(IndexExpr),
- Size(Expansions.size()), ExpandsToEmptyPack(ExpandsToEmptyPack) {
+ Size(Expansions.size()), FullySubstituted(FullySubstituted) {
std::uninitialized_copy(Expansions.begin(), Expansions.end(),
getTrailingObjects<QualType>());
@@ -4081,10 +4081,10 @@ PackIndexingType::computeDependence(QualType Pattern, Expr *IndexExpr,
void PackIndexingType::Profile(llvm::FoldingSetNodeID &ID,
const ASTContext &Context, QualType Pattern,
- Expr *E, bool ExpandsToEmptyPack) {
+ Expr *E, bool FullySubstituted) {
Pattern.Profile(ID);
E->Profile(ID, Context, true);
- ID.AddBoolean(ExpandsToEmptyPack);
+ ID.AddBoolean(FullySubstituted);
}
UnaryTransformType::UnaryTransformType(QualType BaseType,
diff --git a/clang/lib/Sema/SemaTemplateVariadic.cpp b/clang/lib/Sema/SemaTemplateVariadic.cpp
index 2ea2a368dd24cf..86d15f6324f4f5 100644
--- a/clang/lib/Sema/SemaTemplateVariadic.cpp
+++ b/clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -1157,10 +1157,12 @@ ExprResult Sema::ActOnPackIndexingExpr(Scope *S, Expr *PackExpression,
return Res;
}
-ExprResult
-Sema::BuildPackIndexingExpr(Expr *PackExpression, SourceLocation EllipsisLoc,
- Expr *IndexExpr, SourceLocation RSquareLoc,
- ArrayRef<Expr *> ExpandedExprs, bool EmptyPack) {
+ExprResult Sema::BuildPackIndexingExpr(Expr *PackExpression,
+ SourceLocation EllipsisLoc,
+ Expr *IndexExpr,
+ SourceLocation RSquareLoc,
+ ArrayRef<Expr *> ExpandedExprs,
+ bool FullySubstituted) {
std::optional<int64_t> Index;
if (!IndexExpr->isInstantiationDependent()) {
@@ -1174,8 +1176,8 @@ Sema::BuildPackIndexingExpr(Expr *PackExpression, SourceLocation EllipsisLoc,
IndexExpr = Res.get();
}
- if (Index && (!ExpandedExprs.empty() || EmptyPack)) {
- if (*Index < 0 || EmptyPack || *Index >= int64_t(ExpandedExprs.size())) {
+ if (Index && FullySubstituted) {
+ if (*Index < 0 || *Index >= int64_t(ExpandedExprs.size())) {
Diag(PackExpression->getBeginLoc(), diag::err_pack_index_out_of_bound)
<< *Index << PackExpression << ExpandedExprs.size();
return ExprError();
@@ -1184,7 +1186,7 @@ Sema::BuildPackIndexingExpr(Expr *PackExpression, SourceLocation EllipsisLoc,
return PackIndexingExpr::Create(getASTContext(), EllipsisLoc, RSquareLoc,
PackExpression, IndexExpr, Index,
- ExpandedExprs, EmptyPack);
+ ExpandedExprs, FullySubstituted);
}
TemplateArgumentLoc Sema::getTemplateArgumentPackExpansionPattern(
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 1465bba87724b9..9cf1b2d073a90f 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -3670,10 +3670,10 @@ class TreeTransform {
SourceLocation RSquareLoc,
Expr *PackIdExpression, Expr *IndexExpr,
ArrayRef<Expr *> ExpandedExprs,
- bool EmptyPack = false) {
+ bool FullySubstituted = false) {
return getSema().BuildPackIndexingExpr(PackIdExpression, EllipsisLoc,
IndexExpr, RSquareLoc, ExpandedExprs,
- EmptyPack);
+ FullySubstituted);
}
/// Build a new expression representing a call to a source location
@@ -6769,6 +6769,7 @@ TreeTransform<Derived>::TransformPackIndexingType(TypeLocBuilder &TLB,
if (Out.isNull())
return QualType();
SubtitutedTypes.push_back(Out);
+ FullySubstituted &= !Out->containsUnexpandedParameterPack();
}
// If we're supposed to retain a pack expansion, do so by temporarily
// forgetting the partially-substituted parameter pack.
@@ -15581,6 +15582,7 @@ TreeTransform<Derived>::TransformPackIndexingExpr(PackIndexingExpr *E) {
}
SmallVector<Expr *, 5> ExpandedExprs;
+ bool FullySubstituted = true;
if (!E->expandsToEmptyPack() && E->getExpressions().empty()) {
Expr *Pattern = E->getPackIdExpression();
SmallVector<UnexpandedParameterPack, 2> Unexpanded;
@@ -15605,7 +15607,7 @@ TreeTransform<Derived>::TransformPackIndexingExpr(PackIndexingExpr *E) {
return ExprError();
return getDerived().RebuildPackIndexingExpr(
E->getEllipsisLoc(), E->getRSquareLoc(), Pack.get(), IndexExpr.get(),
- {});
+ {}, /*FullySubstituted=*/false);
}
for (unsigned I = 0; I != *NumExpansions; ++I) {
Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(getSema(), I);
@@ -15617,6 +15619,7 @@ TreeTransform<Derived>::TransformPackIndexingExpr(PackIndexingExpr *E) {
OrigNumExpansions);
if (Out.isInvalid())
return true;
+ FullySubstituted = false;
}
ExpandedExprs.push_back(Out.get());
}
@@ -15633,6 +15636,7 @@ TreeTransform<Derived>::TransformPackIndexingExpr(PackIndexingExpr *E) {
OrigNumExpansions);
if (Out.isInvalid())
return true;
+ FullySubstituted = false;
ExpandedExprs.push_back(Out.get());
}
} else if (!E->expandsToEmptyPack()) {
@@ -15644,8 +15648,7 @@ TreeTransform<Derived>::TransformPackIndexingExpr(PackIndexingExpr *E) {
return getDerived().RebuildPackIndexingExpr(
E->getEllipsisLoc(), E->getRSquareLoc(), E->getPackIdExpression(),
- IndexExpr.get(), ExpandedExprs,
- /*EmptyPack=*/ExpandedExprs.size() == 0);
+ IndexExpr.get(), ExpandedExprs, FullySubstituted);
}
template<typename Derived>
diff --git a/clang/lib/Serialization/ASTReaderStmt.cpp b/clang/lib/Serialization/ASTReaderStmt.cpp
index c39a1950a6cf24..731ad0b64dc850 100644
--- a/clang/lib/Serialization/ASTReaderStmt.cpp
+++ b/clang/lib/Serialization/ASTReaderStmt.cpp
@@ -2191,7 +2191,7 @@ void ASTStmtReader::VisitSizeOfPackExpr(SizeOfPackExpr *E) {
void ASTStmtReader::VisitPackIndexingExpr(PackIndexingExpr *E) {
VisitExpr(E);
E->TransformedExpressions = Record.readInt();
- E->ExpandedToEmptyPack = Record.readInt();
+ E->FullySubstituted = Record.readInt();
E->EllipsisLoc = readSourceLocation();
E->RSquareLoc = readSourceLocation();
E->SubExprs[0] = Record.readStmt();
diff --git a/clang/lib/Serialization/ASTWriterStmt.cpp b/clang/lib/Serialization/ASTWriterStmt.cpp
index e7f567ff59a8ad..4994047d9fe10f 100644
--- a/clang/lib/Serialization/ASTWriterStmt.cpp
+++ b/clang/lib/Serialization/ASTWriterStmt.cpp
@@ -2191,7 +2191,7 @@ void ASTStmtWriter::VisitSizeOfPackExpr(SizeOfPackExpr *E) {
void ASTStmtWriter::VisitPackIndexingExpr(PackIndexingExpr *E) {
VisitExpr(E);
Record.push_back(E->TransformedExpressions);
- Record.push_back(E->ExpandedToEmptyPack);
+ Record.push_back(E->FullySubstituted);
Record.AddSourceLocation(E->getEllipsisLoc());
Record.AddSourceLocation(E->getRSquareLoc());
Record.AddStmt(E->getPackIdExpression());
diff --git a/clang/test/SemaCXX/cxx2c-pack-indexing.cpp b/clang/test/SemaCXX/cxx2c-pack-indexing.cpp
index 962dbb8137f289..cb679a6c3ad879 100644
--- a/clang/test/SemaCXX/cxx2c-pack-indexing.cpp
+++ b/clang/test/SemaCXX/cxx2c-pack-indexing.cpp
@@ -271,3 +271,37 @@ void f() {
}
} // namespace GH105903
+
+namespace GH116105 {
+
+template <unsigned long Np, class... Ts> using pack_type = Ts...[Np];
+
+template <unsigned long Np, auto... Ts> using pack_expr = decltype(Ts...[Np]);
+
+template <class...> struct types;
+
+template <class, long... Is> struct indices;
+
+template <class> struct repack;
+
+template <long... Idx> struct repack<indices<long, Idx...>> {
+ template <class... Ts>
+ using pack_type_alias = types<pack_type<Idx, Ts...>...>;
+
+ template <class... Ts>
+ using pack_expr_alias = types<pack_expr<Idx, Ts{}...>...>;
+};
+
+template <class... Args> struct mdispatch_ {
+ using Idx = __make_integer_seq<indices, long, sizeof...(Args)>;
+
+ static_assert(__is_same(
+ typename repack<Idx>::template pack_type_alias<Args...>, types<Args...>));
+
+ static_assert(__is_same(
+ typename repack<Idx>::template pack_expr_alias<Args...>, types<Args...>));
+};
+
+mdispatch_<int, int> d;
+
+} // namespace GH116105
More information about the cfe-commits
mailing list