[clang] [Clang] Remove the PackExpansion restrictions for rewrite substitution (PR #126206)
Younan Zhang via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 13 22:36:52 PST 2025
https://github.com/zyn0217 updated https://github.com/llvm/llvm-project/pull/126206
>From 2ce86d8842b7b37141d4a415830880b9d1d30260 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Fri, 7 Feb 2025 17:15:05 +0800
Subject: [PATCH 1/2] [Clang] Remove the PackExpansion restrictions for rewrite
substitution
When substituting for rewrite purposes, as in rebuilding constraints for a synthesized
deduction guide, it assumed that packs were in PackExpansion* form, such that the
instantiator could extract a pattern.
For type aliases CTAD, while rebuilding their associated constraints, this might not be
the case because we'll call TransformTemplateArgument() for the alias
template arguments, where there might be e.g. a non-pack expansion type into a pack expansion,
so the assumption wouldn't hold.
This patch fixes that by making it treat the non-pack expansions as direct patterns when rewriting.
---
clang/lib/Sema/SemaTemplate.cpp | 2 +-
clang/lib/Sema/SemaTemplateInstantiate.cpp | 20 ++++++----
clang/test/AST/ast-dump-ctad-alias.cpp | 46 ++++++++++++++++++++++
3 files changed, 60 insertions(+), 8 deletions(-)
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 35ece88c603dd..1891613c48fa6 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -4905,7 +4905,7 @@ bool Sema::CheckTemplateTypeArgument(
[[fallthrough]];
}
default: {
- // We allow instantiateing a template with template argument packs when
+ // We allow instantiating a template with template argument packs when
// building deduction guides.
if (Arg.getKind() == TemplateArgument::Pack &&
CodeSynthesisContexts.back().Kind ==
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 12e98a33d0785..4922d0893bb3d 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -1627,7 +1627,7 @@ namespace {
TemplateArgumentLoc Input = SemaRef.getTrivialTemplateArgumentLoc(
pack, QualType(), SourceLocation{});
TemplateArgumentLoc Output;
- if (SemaRef.SubstTemplateArgument(Input, TemplateArgs, Output))
+ if (TransformTemplateArgument(Input, Output, Uneval))
return true; // fails
TArgs.push_back(Output.getArgument());
}
@@ -2041,9 +2041,11 @@ TemplateName TemplateInstantiator::TransformTemplateName(
// We're rewriting the template parameter as a reference to another
// template parameter.
if (Arg.getKind() == TemplateArgument::Pack) {
- assert(Arg.pack_size() == 1 && Arg.pack_begin()->isPackExpansion() &&
+ assert(Arg.pack_size() == 1 &&
"unexpected pack arguments in template rewrite");
- Arg = Arg.pack_begin()->getPackExpansionPattern();
+ Arg = *Arg.pack_begin();
+ if (Arg.isPackExpansion())
+ Arg = Arg.getPackExpansionPattern();
}
assert(Arg.getKind() == TemplateArgument::Template &&
"unexpected nontype template argument kind in template rewrite");
@@ -2126,9 +2128,11 @@ TemplateInstantiator::TransformTemplateParmRefExpr(DeclRefExpr *E,
// We're rewriting the template parameter as a reference to another
// template parameter.
if (Arg.getKind() == TemplateArgument::Pack) {
- assert(Arg.pack_size() == 1 && Arg.pack_begin()->isPackExpansion() &&
+ assert(Arg.pack_size() == 1 &&
"unexpected pack arguments in template rewrite");
- Arg = Arg.pack_begin()->getPackExpansionPattern();
+ Arg = *Arg.pack_begin();
+ if (Arg.isPackExpansion())
+ Arg = Arg.getPackExpansionPattern();
}
assert(Arg.getKind() == TemplateArgument::Expression &&
"unexpected nontype template argument kind in template rewrite");
@@ -2592,9 +2596,11 @@ TemplateInstantiator::TransformTemplateTypeParmType(TypeLocBuilder &TLB,
// We're rewriting the template parameter as a reference to another
// template parameter.
if (Arg.getKind() == TemplateArgument::Pack) {
- assert(Arg.pack_size() == 1 && Arg.pack_begin()->isPackExpansion() &&
+ assert(Arg.pack_size() == 1 &&
"unexpected pack arguments in template rewrite");
- Arg = Arg.pack_begin()->getPackExpansionPattern();
+ Arg = *Arg.pack_begin();
+ if (Arg.isPackExpansion())
+ Arg = Arg.getPackExpansionPattern();
}
assert(Arg.getKind() == TemplateArgument::Type &&
"unexpected nontype template argument kind in template rewrite");
diff --git a/clang/test/AST/ast-dump-ctad-alias.cpp b/clang/test/AST/ast-dump-ctad-alias.cpp
index b1631f7822ce0..f39a4cee518ce 100644
--- a/clang/test/AST/ast-dump-ctad-alias.cpp
+++ b/clang/test/AST/ast-dump-ctad-alias.cpp
@@ -156,3 +156,49 @@ ATemplatedClass2 test2(list);
// CHECK-NEXT: |-TypeTraitExpr {{.*}} 'bool' __is_deducible
} // namespace GH90209
+
+namespace GH124715 {
+
+template <class T, class... Args>
+concept invocable = true;
+
+template <class T, class... Args> struct Struct {
+ template <class U>
+ requires invocable<U, Args...>
+ Struct(U, Args...) {}
+};
+
+template <class...> struct Packs {};
+
+template <class Lambda, class... Args>
+Struct(Lambda lambda, Args... args) -> Struct<Lambda, Args...>;
+
+template <class T, class... Ts> using Alias = Struct<T, Packs<Ts...>>;
+
+void foo() {
+ Alias([](int) {}, Packs<int>());
+}
+
+// CHECK: |-FunctionTemplateDecl {{.*}} implicit <deduction guide for Alias>
+// CHECK-NEXT: | |-TemplateTypeParmDecl {{.*}} class depth 0 index 0 T
+// CHECK-NEXT: | |-TemplateTypeParmDecl {{.*}} class depth 0 index 1 ... Ts
+// CHECK-NEXT: | |-TemplateTypeParmDecl {{.*}} class depth 0 index 2 U
+// CHECK-NEXT: | |-BinaryOperator {{.*}} 'bool' '&&'
+// CHECK-NEXT: | | |-ConceptSpecializationExpr {{.*}} 'bool' Concept {{.*}} 'invocable'
+// CHECK-NEXT: | | | |-ImplicitConceptSpecializationDecl {{.*}}
+// CHECK-NEXT: | | | | |-TemplateArgument type 'type-parameter-0-2'
+// CHECK-NEXT: | | | | | `-TemplateTypeParmType {{.*}} 'type-parameter-0-2' dependent depth 0 index 2
+// CHECK-NEXT: | | | | `-TemplateArgument pack '<Packs<type-parameter-0-1...>>'
+// CHECK-NEXT: | | | | `-TemplateArgument type 'Packs<type-parameter-0-1...>'
+// CHECK-NEXT: | | | | `-TemplateSpecializationType {{.*}} 'Packs<type-parameter-0-1...>' dependent
+// CHECK-NEXT: | | | | |-name: 'GH124715::Packs'
+// CHECK-NEXT: | | | | | `-ClassTemplateDecl {{.*}} Packs
+// CHECK-NEXT: | | | | `-TemplateArgument pack '<type-parameter-0-1...>'
+// CHECK-NEXT: | | | | `-TemplateArgument type 'type-parameter-0-1...'
+// CHECK-NEXT: | | | | `-PackExpansionType {{.*}} 'type-parameter-0-1...' dependent
+// CHECK-NEXT: | | | | `-TemplateTypeParmType {{.*}} 'type-parameter-0-1' dependent contains_unexpanded_pack depth 0 index 1 pack
+// CHECK-NEXT: | | | |-TemplateArgument {{.*}} type 'U':'type-parameter-0-2'
+// CHECK-NEXT: | | | | `-TemplateTypeParmType {{.*}} 'U' dependent depth 0 index 2
+// CHECK-NEXT: | | | | `-TemplateTypeParm {{.*}} 'U'
+
+} // namespace GH124715
>From 7a789e399ea264d7d8d8281cb75e202e4644e8b8 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Fri, 14 Feb 2025 14:36:16 +0800
Subject: [PATCH 2/2] Extract a function
---
clang/lib/Sema/SemaTemplateInstantiate.cpp | 36 +++++++++-------------
1 file changed, 15 insertions(+), 21 deletions(-)
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 4922d0893bb3d..5c15e4c4870a9 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -1466,6 +1466,18 @@ namespace {
}
}
+ static TemplateArgument
+ getTemplateArgumentPackPatternForRewrite(const TemplateArgument &TA) {
+ if (TA.getKind() != TemplateArgument::Pack)
+ return TA;
+ assert(TA.pack_size() == 1 &&
+ "unexpected pack arguments in template rewrite");
+ TemplateArgument Arg = *TA.pack_begin();
+ if (Arg.isPackExpansion())
+ Arg = Arg.getPackExpansionPattern();
+ return Arg;
+ }
+
/// Transform the given declaration by instantiating a reference to
/// this declaration.
Decl *TransformDecl(SourceLocation Loc, Decl *D);
@@ -2040,13 +2052,7 @@ TemplateName TemplateInstantiator::TransformTemplateName(
if (TemplateArgs.isRewrite()) {
// We're rewriting the template parameter as a reference to another
// template parameter.
- if (Arg.getKind() == TemplateArgument::Pack) {
- assert(Arg.pack_size() == 1 &&
- "unexpected pack arguments in template rewrite");
- Arg = *Arg.pack_begin();
- if (Arg.isPackExpansion())
- Arg = Arg.getPackExpansionPattern();
- }
+ Arg = getTemplateArgumentPackPatternForRewrite(Arg);
assert(Arg.getKind() == TemplateArgument::Template &&
"unexpected nontype template argument kind in template rewrite");
return Arg.getAsTemplate();
@@ -2127,13 +2133,7 @@ TemplateInstantiator::TransformTemplateParmRefExpr(DeclRefExpr *E,
if (TemplateArgs.isRewrite()) {
// We're rewriting the template parameter as a reference to another
// template parameter.
- if (Arg.getKind() == TemplateArgument::Pack) {
- assert(Arg.pack_size() == 1 &&
- "unexpected pack arguments in template rewrite");
- Arg = *Arg.pack_begin();
- if (Arg.isPackExpansion())
- Arg = Arg.getPackExpansionPattern();
- }
+ Arg = getTemplateArgumentPackPatternForRewrite(Arg);
assert(Arg.getKind() == TemplateArgument::Expression &&
"unexpected nontype template argument kind in template rewrite");
// FIXME: This can lead to the same subexpression appearing multiple times
@@ -2595,13 +2595,7 @@ TemplateInstantiator::TransformTemplateTypeParmType(TypeLocBuilder &TLB,
if (TemplateArgs.isRewrite()) {
// We're rewriting the template parameter as a reference to another
// template parameter.
- if (Arg.getKind() == TemplateArgument::Pack) {
- assert(Arg.pack_size() == 1 &&
- "unexpected pack arguments in template rewrite");
- Arg = *Arg.pack_begin();
- if (Arg.isPackExpansion())
- Arg = Arg.getPackExpansionPattern();
- }
+ Arg = getTemplateArgumentPackPatternForRewrite(Arg);
assert(Arg.getKind() == TemplateArgument::Type &&
"unexpected nontype template argument kind in template rewrite");
QualType NewT = Arg.getAsType();
More information about the cfe-commits
mailing list