[Clang] Remove the PackExpansion restrictions for rewrite substitution

[zyn0217]

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 cases 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.

Fixes #124715

[llvmbot]

@llvm/pr-subscribers-clang

Author: Younan Zhang (zyn0217)

Changes

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, we rebuild the template arguments using TransformTemplateArgument(), which did not guarantee the establishment of PackExpansions as getInjectedTemplateArguments() does.

This patch fixes that by making it call RebuildPackExpansion() if the transformed arguments are still having unexpanded parameter packs.

No release note as I think this is a good candidate for backporting.

Fixes #124715

---

Full diff: https://github.com/llvm/llvm-project/pull/126206.diff

2 Files Affected:

• (modified) clang/lib/Sema/SemaTemplateInstantiate.cpp (+7-1)
• (modified) clang/test/AST/ast-dump-ctad-alias.cpp (+44)

diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 12e98a33d07853e..522bd418e2f1881 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -1627,8 +1627,14 @@ namespace {
           TemplateArgumentLoc Input = SemaRef.getTrivialTemplateArgumentLoc(
               pack, QualType(), SourceLocation{});
           TemplateArgumentLoc Output;
-          if (SemaRef.SubstTemplateArgument(Input, TemplateArgs, Output))
+          if (TransformTemplateArgument(Input, Output, Uneval))
             return true; // fails
+          if (Output.getArgument().containsUnexpandedParameterPack())
+            // FIXME: Is EllipsisLoc necessary? This pack expansion merely
+            // serves as a placeholder type for future rewrite-substitution
+            // (e.g. into constraint expressions.)
+            Output =
+                RebuildPackExpansion(Output, SourceLocation{}, std::nullopt);
           TArgs.push_back(Output.getArgument());
         }
         Output = SemaRef.getTrivialTemplateArgumentLoc(
diff --git a/clang/test/AST/ast-dump-ctad-alias.cpp b/clang/test/AST/ast-dump-ctad-alias.cpp
index b1631f7822ce017..464808af784c2c3 100644
--- a/clang/test/AST/ast-dump-ctad-alias.cpp
+++ b/clang/test/AST/ast-dump-ctad-alias.cpp
@@ -156,3 +156,47 @@ 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 Lambda, class... Args>
+Struct(Lambda lambda, Args... args) -> Struct<Lambda, Args...>;
+
+template <class T, class... Ts> using Alias = Struct<T, Ts...>;
+
+void foo() {
+  Alias([](int) {}, 0);
+}
+
+// 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 '<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'
+// CHECK-NEXT: | | | `-TemplateArgument {{.*}} type 'Ts...':'type-parameter-0-1...'
+// CHECK-NEXT: | | |   `-PackExpansionType {{.*}} 'Ts...' dependent
+// CHECK-NEXT: | | |     `-TemplateTypeParmType {{.*}} 'Ts' dependent contains_unexpanded_pack depth 0 index 1 pack
+// CHECK-NEXT: | | |       `-TemplateTypeParm {{.*}} 'Ts'
+// CHECK-NEXT: | | `-TypeTraitExpr {{.*}} 'bool' __is_deducible
+
+} // namespace GH124715

[shafik]

I see the test references #124715 but you don't mention it in the summary and you don't have a release note. Does the test exercise the crash?

[zyn0217]

I see the test references #124715 but you don't mention it in the summary and you don't have a release note. Does the test exercise the crash?

Oh, thanks for spotting that - I forgot it when I updated the commit body.

Will add a release note once we backport it to the 20 branch.

[hokein]

Thanks! Please add a release note.