release/20.x: [Clang] Remove the PackExpansion restrictions for rewrite substitution #127174
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Younan Zhang (zyn0217) ChangesThis backports c08b80e with a release note towards 20 so that we could resolve some pains in CTAD. Full diff: https://github.com/llvm/llvm-project/pull/127174.diff 4 Files Affected:
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
^^^^^^^^^^^^^^^^^^^^^^^^^
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
|
hokein
approved these changes
Feb 14, 2025
|
Ping @tstellar thanks. |
|
@zyn0217 (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This backports c08b80e with a release note towards 20 so that we could resolve some pains in CTAD.