[Clang] Handle default template arguments for alias CTAD guides #134807

zyn0217 · 2025-04-08T08:24:36Z

It's possible that some deduced template arguments come from default arguments, not just from the return type. So we need to recursively visit the default arguments of the parameter if it's referenced, thereby the template parameter referenced by the defualt arguments could come along to the synthesized deduction guide.

Fixes #134471

It's possible that some deduced template arguments come from default arguments, not just from the return type. So we need to recursively visit the default arguments if the parameter is referenced, thereby the template parameter referenced by the defualt arguments could come along to the synthesized deduction guide.

llvmbot · 2025-04-08T10:32:10Z

@llvm/pr-subscribers-clang

Author: Younan Zhang (zyn0217)

Changes

It's possible that some deduced template arguments come from default arguments, not just from the return type. So we need to recursively visit the default arguments of the parameter if it's referenced, thereby the template parameter referenced by the defualt arguments could come along to the synthesized deduction guide.

Fixes #134471

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

3 Files Affected:

(modified) clang/docs/ReleaseNotes.rst (+1)
(modified) clang/lib/Sema/SemaTemplateDeductionGuide.cpp (+17)
(modified) clang/test/SemaTemplate/deduction-guide.cpp (+39)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index e671183522565..aa57b995a6433 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -390,6 +390,7 @@ Bug Fixes to C++ Support
 - Clang no longer crashes when trying to unify the types of arrays with
   certain differences in qualifiers (this could happen during template argument
   deduction or when building a ternary operator). (#GH97005)
+- Fixed type alias CTAD issues involving default template arguments. (#GH134471)
 - The initialization kind of elements of structured bindings
   direct-list-initialized from an array is corrected to direct-initialization.
 - Clang no longer crashes when a coroutine is declared ``[[noreturn]]``. (#GH127327)
diff --git a/clang/lib/Sema/SemaTemplateDeductionGuide.cpp b/clang/lib/Sema/SemaTemplateDeductionGuide.cpp
index b4863cefc3fb4..30376ca774384 100644
--- a/clang/lib/Sema/SemaTemplateDeductionGuide.cpp
+++ b/clang/lib/Sema/SemaTemplateDeductionGuide.cpp
@@ -690,6 +690,23 @@ SmallVector<unsigned> TemplateParamsReferencedInTemplateArgumentList(
   SemaRef.MarkUsedTemplateParameters(
       DeducedArgs, TemplateParamsList->getDepth(), ReferencedTemplateParams);
 
+  auto MarkDefaultArgs = [&](auto *Param) {
+    if (!Param || !Param->hasDefaultArgument())
+      return;
+    SemaRef.MarkUsedTemplateParameters(
+        Param->getDefaultArgument().getArgument(),
+        TemplateParamsList->getDepth(), ReferencedTemplateParams);
+  };
+
+  for (unsigned Index = 0; Index < TemplateParamsList->size(); ++Index) {
+    if (!ReferencedTemplateParams[Index])
+      continue;
+    auto *Param = TemplateParamsList->getParam(Index);
+    MarkDefaultArgs(dyn_cast<TemplateTypeParmDecl>(Param));
+    MarkDefaultArgs(dyn_cast<NonTypeTemplateParmDecl>(Param));
+    MarkDefaultArgs(dyn_cast<TemplateTemplateParmDecl>(Param));
+  }
+
   SmallVector<unsigned> Results;
   for (unsigned Index = 0; Index < TemplateParamsList->size(); ++Index) {
     if (ReferencedTemplateParams[Index])
diff --git a/clang/test/SemaTemplate/deduction-guide.cpp b/clang/test/SemaTemplate/deduction-guide.cpp
index 6db132ca37c7e..76b6cb051b7dd 100644
--- a/clang/test/SemaTemplate/deduction-guide.cpp
+++ b/clang/test/SemaTemplate/deduction-guide.cpp
@@ -771,3 +771,42 @@ D d(24);
 // CHECK-NEXT:  `-ParmVarDecl {{.+}} 'U'
 
 } // namespace GH132616_DeductionGuide
+
+namespace GH133132 {
+
+template <class _Ty>
+struct A {};
+
+template <class T = int, class U = T>
+using AA = A<U>;
+
+AA a{};
+
+// CHECK-LABEL: Dumping GH133132::<deduction guide for AA>:
+// CHECK-NEXT:  FunctionTemplateDecl {{.+}} implicit <deduction guide for AA>
+// CHECK-NEXT:  |-TemplateTypeParmDecl {{.+}} class depth 0 index 0 T
+// CHECK-NEXT:  | `-TemplateArgument type 'int'
+// CHECK-NEXT:  |   `-BuiltinType {{.+}} 'int'
+// CHECK-NEXT:  |-TemplateTypeParmDecl {{.+}} class depth 0 index 1 U
+// CHECK-NEXT:  | `-TemplateArgument type 'T':'type-parameter-0-0'
+// CHECK-NEXT:  |   `-TemplateTypeParmType {{.+}} 'T' dependent depth 0 index 0
+// CHECK-NEXT:  |     `-TemplateTypeParm {{.+}} 'T'
+// CHECK-NEXT:  |-TypeTraitExpr {{.+}} 'bool' __is_deducible
+// CHECK-NEXT:  | |-DeducedTemplateSpecializationType {{.+}} 'GH133132::AA' dependent
+// CHECK-NEXT:  | | `-name: 'GH133132::AA'
+// CHECK-NEXT:  | |   `-TypeAliasTemplateDecl {{.+}} AA
+// CHECK-NEXT:  | `-TemplateSpecializationType {{.+}} 'A<U>' dependent
+// CHECK-NEXT:  |   |-name: 'A':'GH133132::A' qualified
+// CHECK-NEXT:  |   | `-ClassTemplateDecl {{.+}} A
+// CHECK-NEXT:  |   `-TemplateArgument type 'U':'type-parameter-0-1'
+// CHECK-NEXT:  |     `-SubstTemplateTypeParmType {{.+}} 'U' sugar dependent class depth 0 index 0 _Ty
+// CHECK-NEXT:  |       |-FunctionTemplate {{.+}} '<deduction guide for A>'
+// CHECK-NEXT:  |       `-TemplateTypeParmType {{.+}} 'U' dependent depth 0 index 1
+// CHECK-NEXT:  |         `-TemplateTypeParm {{.+}} 'U'
+// CHECK-NEXT:  |-CXXDeductionGuideDecl {{.+}} implicit <deduction guide for AA> 'auto () -> A<U>'
+// CHECK-NEXT:  `-CXXDeductionGuideDecl {{.+}} implicit used <deduction guide for AA> 'auto () -> A<int>' implicit_instantiation
+// CHECK-NEXT:    |-TemplateArgument type 'int'
+// CHECK-NEXT:    | `-BuiltinType {{.+}} 'int'
+// CHECK-NEXT:    `-TemplateArgument type 'int'
+// CHECK-NEXT:      `-BuiltinType {{.+}} 'int'
+}

mizvekov · 2025-04-09T15:58:50Z

clang/lib/Sema/SemaTemplateDeductionGuide.cpp

+  auto MarkDefaultArgs = [&](auto *Param) {
+    if (!Param || !Param->hasDefaultArgument())
+      return;
+    SemaRef.MarkUsedTemplateParameters(
+        Param->getDefaultArgument().getArgument(),
+        TemplateParamsList->getDepth(), ReferencedTemplateParams);
+  };
+
+  for (unsigned Index = 0; Index < TemplateParamsList->size(); ++Index) {
+    if (!ReferencedTemplateParams[Index])
+      continue;
+    auto *Param = TemplateParamsList->getParam(Index);
+    MarkDefaultArgs(dyn_cast<TemplateTypeParmDecl>(Param));
+    MarkDefaultArgs(dyn_cast<NonTypeTemplateParmDecl>(Param));
+    MarkDefaultArgs(dyn_cast<TemplateTemplateParmDecl>(Param));
+  }
+


I wonder if we are missing a step to get the converted template argument list, and should be calling this with that instead of the deduced arguments directly.

For example, it seems this will mark default arguments as used even if the corresponding parameter has a deduced argument.

I wonder if we are missing a step to get the converted template argument list, and should be calling this with that instead of the deduced arguments directly.

The fact that makes me think it sensible is, in the later template parameter transform, we do retain/tranaform the default arguments. That means, the transformed template parameters which has default arguments could end up referring to anything that won't otherwise appear in the synthesized template parameter list. And this is exactly where the issue raised.

Re the ConvertDeducedTemplateArguments, i think we probably want it, but I couldn't immediately recall what job it does to handle default arguments.

For example, it seems this will mark default arguments as used even if the corresponding parameter has a deduced argument.

That might not be a big deal, because as long as the default parameter doesn't end up in the argument list of RHS, it shouldn't affect the template argument deduction running against on the synthesized deductions guides -- we don't need to CodeGen for deduction guides nor it has to be fully instantiated for all of its parameters, right?

Re the ConvertDeducedTemplateArguments, i think we probably want it, but I couldn't immediately recall what job it does to handle default arguments.

I will produce a full list of template arguments, including default arguments where a parameter is not deduced.

It will also pack the arguments for template parameter packs, and do other things which might not be relevant here.

@mizvekov I looked into ConvertDeducedTemplateArguments and I'm afraid it won't help

In deduction guide synthesizing, we have a unique template parameter transform that does handle default parameters (which also covers non-alias deduction guide synthesizing) so when we find a parameter (e.g. U in example) used by the RHS (A<U>), we form a new TTP on top of the TTP U = T. And, as T is not referenced by A<U>, the declaration of which won't be created anyway, so we ended up with a problematic template <class U = <template-param-0-0>> in the eventual CTAD guide.

template <class _Ty> struct A {}; template <class T = int, class U = T> using AA = A<U>;

This is where the problem lies. And with ConvertDeducedTemplateArguments, which would substitute the default arguments with the deduced ones in non-rewrite mode, we end up building Subst* nodes that we never want in CTAD.

But we could augment ConvertDeducedTemplateArguments with the option of producing rewrites.
Or eventually I think the rewrite flag could be removed entirely, everything being handled automatically.

I am more worried about the big picture here, I find it strange that this operates on the Deduced arguments directly,
which is kind of an arbitrary / clang specific point where the problem is divided,
and that there might be more stuff related to conversions which don't happen correctly in CTAD.

But since I have too much on my plate to dive on this problem now, I digress, lets go ahead :)

clang/test/SemaTemplate/deduction-guide.cpp

cor3ntin

I am happy with that (modulo nits) when @mizvekov is

clang/lib/Sema/SemaTemplateDeductionGuide.cpp

mizvekov

LGTM, thanks!

…#134807) It's possible that some deduced template arguments come from default arguments, not just from the return type. So we need to recursively visit the default arguments of the parameter if it's referenced, thereby the template parameter referenced by the defualt arguments could come along to the synthesized deduction guide. Fixes llvm#134471

zyn0217 requested review from mizvekov, cor3ntin and erichkeane April 8, 2025 08:24

zyn0217 force-pushed the default-args-ctad branch from d93117d to 3f09d11 Compare April 8, 2025 08:25

Merge branch 'main' into default-args-ctad

a6c14db

zyn0217 marked this pull request as ready for review April 8, 2025 10:31

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Apr 8, 2025

mizvekov reviewed Apr 9, 2025

View reviewed changes

zyn0217 requested a review from mizvekov April 11, 2025 05:27

cor3ntin reviewed Apr 14, 2025

View reviewed changes

clang/test/SemaTemplate/deduction-guide.cpp Show resolved Hide resolved

Add tests for template/non-template parameters

0587264

cor3ntin reviewed Apr 15, 2025

View reviewed changes

clang/lib/Sema/SemaTemplateDeductionGuide.cpp Outdated Show resolved Hide resolved

cor3ntin approved these changes Apr 15, 2025

View reviewed changes

Address review feedback

7619b33

erichkeane approved these changes Apr 15, 2025

View reviewed changes

mizvekov approved these changes Apr 15, 2025

View reviewed changes

zyn0217 merged commit de528d6 into llvm:main Apr 17, 2025
11 of 12 checks passed

frederick-vs-ja mentioned this pull request May 11, 2025

[clang] crash on CTAD of alias template #139442

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Clang] Handle default template arguments for alias CTAD guides #134807

[Clang] Handle default template arguments for alias CTAD guides #134807

Uh oh!

zyn0217 commented Apr 8, 2025 •

edited

Loading

Uh oh!

llvmbot commented Apr 8, 2025

Uh oh!

mizvekov Apr 9, 2025

Uh oh!

mizvekov Apr 9, 2025

Uh oh!

zyn0217 Apr 10, 2025

Uh oh!

zyn0217 Apr 10, 2025

Uh oh!

mizvekov Apr 11, 2025

Uh oh!

zyn0217 Apr 14, 2025

Uh oh!

mizvekov Apr 15, 2025

Uh oh!

Uh oh!

cor3ntin left a comment

Uh oh!

Uh oh!

mizvekov left a comment

Uh oh!

Uh oh!

Uh oh!

[Clang] Handle default template arguments for alias CTAD guides #134807

[Clang] Handle default template arguments for alias CTAD guides #134807

Uh oh!

Conversation

zyn0217 commented Apr 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Apr 8, 2025

Uh oh!

mizvekov Apr 9, 2025

Choose a reason for hiding this comment

Uh oh!

mizvekov Apr 9, 2025

Choose a reason for hiding this comment

Uh oh!

zyn0217 Apr 10, 2025

Choose a reason for hiding this comment

Uh oh!

zyn0217 Apr 10, 2025

Choose a reason for hiding this comment

Uh oh!

mizvekov Apr 11, 2025

Choose a reason for hiding this comment

Uh oh!

zyn0217 Apr 14, 2025

Choose a reason for hiding this comment

Uh oh!

mizvekov Apr 15, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cor3ntin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mizvekov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

zyn0217 commented Apr 8, 2025 •

edited

Loading