[llvm-branch-commits] [clang] release/20.x: [Clang] Remove the PackExpansion restrictions for rewrite substitution (PR #127174)
Younan Zhang via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Thu Feb 13 23:06:48 PST 2025
https://github.com/zyn0217 created https://github.com/llvm/llvm-project/pull/127174
This backports [commit-sha] with a release note towards 20 so that we could resolve some pains in CTAD.
>From ec69f0589eb471877ccd488f6d7d4fd43bcf0d02 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 | 32 +++++++--------
clang/test/AST/ast-dump-ctad-alias.cpp | 46 ++++++++++++++++++++++
3 files changed, 63 insertions(+), 17 deletions(-)
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 3944c4f67bab9..f4045debf4521 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 c45d3ffe2508b..eec56b7493bad 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -1467,6 +1467,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);
@@ -1624,7 +1636,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());
}
@@ -2036,11 +2048,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 && Arg.pack_begin()->isPackExpansion() &&
- "unexpected pack arguments in template rewrite");
- Arg = Arg.pack_begin()->getPackExpansionPattern();
- }
+ Arg = getTemplateArgumentPackPatternForRewrite(Arg);
assert(Arg.getKind() == TemplateArgument::Template &&
"unexpected nontype template argument kind in template rewrite");
return Arg.getAsTemplate();
@@ -2121,11 +2129,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 && Arg.pack_begin()->isPackExpansion() &&
- "unexpected pack arguments in template rewrite");
- Arg = Arg.pack_begin()->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
@@ -2578,11 +2582,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 && Arg.pack_begin()->isPackExpansion() &&
- "unexpected pack arguments in template rewrite");
- Arg = Arg.pack_begin()->getPackExpansionPattern();
- }
+ Arg = getTemplateArgumentPackPatternForRewrite(Arg);
assert(Arg.getKind() == TemplateArgument::Type &&
"unexpected nontype template argument kind in template rewrite");
QualType NewT = Arg.getAsType();
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 9df5e299c71f7bcc0fcabe520c8d93081343cd0c Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Fri, 14 Feb 2025 14:58:20 +0800
Subject: [PATCH 2/2] Add a release note towards 20
---
clang/docs/ReleaseNotes.rst | 2 ++
1 file changed, 2 insertions(+)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index d8a94703bd9c5..d62397dc1a837 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1045,6 +1045,8 @@ Bug Fixes to C++ Support
template parameter. Now, such expression can be used with ``static_assert`` and ``constexpr``. (#GH123498)
- Correctly determine the implicit constexprness of lambdas in dependent contexts. (#GH97958) (#GH114234)
- Fix that some dependent immediate expressions did not cause immediate escalation (#GH119046)
+- Fixed a substitution bug in transforming CTAD aliases when the type alias contains a non-pack template argument
+ corresponding to a pack parameter (#GH124715)
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
More information about the llvm-branch-commits
mailing list