[clang] [Clang] Remove the wrong assumption when rebuilding SizeOfPackExprs for constraint normalization (PR #115120)
Younan Zhang via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 7 00:50:05 PST 2024
https://github.com/zyn0217 updated https://github.com/llvm/llvm-project/pull/115120
>From 0017e08259472b4397c8be5578da33448e3d82cd Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Wed, 6 Nov 2024 13:51:01 +0800
Subject: [PATCH 1/2] [Clang] Remove the wrong assumption when rebuilding
SizeOfPackExprs for constraint normalization
In 463a4f150, we assumed that all the template argument packs are
of size 1 when normalizing a constraint expression because I mistakenly
thought those packs were obtained from their injected template parameters.
This was wrong because we might be checking constraints when instantiating
a friend declaration within a class template specialization, where the
parent class template is specialized with non-dependent template arguments.
In that sense, we shouldn't assume any pack size nor expand anything in
such a scenario. Moreover, there are no intermediate (substituted but unexpanded)
AST nodes for template template parameters, so we have to special-case their
transformations by looking into the instantiation scope instead of extracting
anything from template arguments.
---
clang/lib/Sema/SemaTemplateInstantiate.cpp | 33 +++++++++----------
.../SemaTemplate/concepts-out-of-line-def.cpp | 19 +++++++++++
2 files changed, 35 insertions(+), 17 deletions(-)
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index de0ec0128905ff..081873030a1a59 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -1736,23 +1736,13 @@ namespace {
SourceLocation RParenLoc,
std::optional<unsigned> Length,
ArrayRef<TemplateArgument> PartialArgs) {
- if (SemaRef.CodeSynthesisContexts.back().Kind !=
- Sema::CodeSynthesisContext::ConstraintNormalization)
- return inherited::RebuildSizeOfPackExpr(OperatorLoc, Pack, PackLoc,
- RParenLoc, Length, PartialArgs);
-
-#ifndef NDEBUG
- for (auto *Iter = TemplateArgs.begin(); Iter != TemplateArgs.end();
- ++Iter)
- for (const TemplateArgument &TA : Iter->Args)
- assert(TA.getKind() != TemplateArgument::Pack || TA.pack_size() == 1);
-#endif
- Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(
- SemaRef, /*NewSubstitutionIndex=*/0);
- Decl *NewPack = TransformDecl(PackLoc, Pack);
- if (!NewPack)
- return ExprError();
-
+ Decl *NewPack = Pack;
+ if (SemaRef.CodeSynthesisContexts.back().Kind ==
+ Sema::CodeSynthesisContext::ConstraintNormalization) {
+ NewPack = TransformDecl(PackLoc, Pack);
+ if (!NewPack)
+ return ExprError();
+ }
return inherited::RebuildSizeOfPackExpr(OperatorLoc,
cast<NamedDecl>(NewPack), PackLoc,
RParenLoc, Length, PartialArgs);
@@ -1881,6 +1871,15 @@ Decl *TemplateInstantiator::TransformDecl(SourceLocation Loc, Decl *D) {
TemplateArgument Arg = TemplateArgs(TTP->getDepth(), TTP->getPosition());
if (TTP->isParameterPack()) {
+ // We might not have an index for pack expansion when normalizing
+ // constraint expressions. In that case, resort to instantiation scopes
+ // for the transformed declarations.
+ if (SemaRef.ArgumentPackSubstitutionIndex == -1 &&
+ SemaRef.CodeSynthesisContexts.back().Kind ==
+ Sema::CodeSynthesisContext::ConstraintNormalization) {
+ return SemaRef.FindInstantiatedDecl(Loc, cast<NamedDecl>(D),
+ TemplateArgs);
+ }
assert(Arg.getKind() == TemplateArgument::Pack &&
"Missing argument pack");
Arg = getPackSubstitutedTemplateArgument(getSema(), Arg);
diff --git a/clang/test/SemaTemplate/concepts-out-of-line-def.cpp b/clang/test/SemaTemplate/concepts-out-of-line-def.cpp
index fe8f74928fc370..d75df89f2a4234 100644
--- a/clang/test/SemaTemplate/concepts-out-of-line-def.cpp
+++ b/clang/test/SemaTemplate/concepts-out-of-line-def.cpp
@@ -702,3 +702,22 @@ class TTP;
C v;
} // namespace GH93099
+
+namespace GH115098 {
+
+template <typename... Ts> struct c {
+ template <typename T>
+ requires(sizeof...(Ts) > 0)
+ friend bool operator==(c, c);
+};
+
+template <typename... Ts> struct d {
+ template <typename T>
+ requires(sizeof...(Ts) > 0)
+ friend bool operator==(d, d);
+};
+
+template struct c<int>;
+template struct d<int, int>;
+
+} // namespace GH115098
>From c34b91878ae03468777f19535b0e9382619dc8f6 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Thu, 7 Nov 2024 16:46:25 +0800
Subject: [PATCH 2/2] Avoid layering violation
---
clang/include/clang/AST/ExprCXX.h | 2 ++
clang/lib/Sema/SemaTemplateInstantiate.cpp | 22 +++++++++++-----------
2 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h
index 57ab94bcb2010f..696a574833dad2 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -4326,6 +4326,8 @@ class SizeOfPackExpr final
/// Retrieve the parameter pack.
NamedDecl *getPack() const { return Pack; }
+ void setPack(NamedDecl *NewPack) { Pack = NewPack; }
+
/// Retrieve the length of the parameter pack.
///
/// This routine may only be invoked when the expression is not
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 081873030a1a59..9c98cf11e43dd0 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -1731,21 +1731,21 @@ namespace {
return inherited::TransformLambdaBody(E, Body);
}
- ExprResult RebuildSizeOfPackExpr(SourceLocation OperatorLoc,
- NamedDecl *Pack, SourceLocation PackLoc,
- SourceLocation RParenLoc,
- std::optional<unsigned> Length,
- ArrayRef<TemplateArgument> PartialArgs) {
- Decl *NewPack = Pack;
+ ExprResult TransformSizeOfPackExpr(SizeOfPackExpr *E) {
+ ExprResult Transformed = inherited::TransformSizeOfPackExpr(E);
+ if (!Transformed.isUsable())
+ return Transformed;
+ auto *TransformedExpr = cast<SizeOfPackExpr>(Transformed.get());
if (SemaRef.CodeSynthesisContexts.back().Kind ==
- Sema::CodeSynthesisContext::ConstraintNormalization) {
- NewPack = TransformDecl(PackLoc, Pack);
+ Sema::CodeSynthesisContext::ConstraintNormalization &&
+ TransformedExpr->getPack() == E->getPack()) {
+ Decl *NewPack =
+ TransformDecl(E->getPackLoc(), TransformedExpr->getPack());
if (!NewPack)
return ExprError();
+ TransformedExpr->setPack(cast<NamedDecl>(NewPack));
}
- return inherited::RebuildSizeOfPackExpr(OperatorLoc,
- cast<NamedDecl>(NewPack), PackLoc,
- RParenLoc, Length, PartialArgs);
+ return TransformedExpr;
}
ExprResult TransformRequiresExpr(RequiresExpr *E) {
More information about the cfe-commits
mailing list