[clang] CTAD: use index and depth to retrieve template parameter for TemplateParamsReferencedInTemplateArgumentList #98013

hokein · 2024-07-08T11:25:44Z

As described in #90209 (comment), Clang may not preserve enough information during template argument deduction. This can result in a merely canonical TemplateTypeParmType with a null Decl, leading to an incomplete template parameter list for the synthesized deduction guide.

This patch addresses the issue by using the index and depth information to retrieve the corresponding template parameter, rather than relying on TTP->getDecl().

Fixes #90209

llvmbot · 2024-07-08T11:26:14Z

@llvm/pr-subscribers-clang

Author: Haojian Wu (hokein)

Changes

As described in #90209 (comment), Clang may not preserve enough information during template argument deduction. This can result in a merely canonical TemplateTypeParmType with a null Decl, leading to an incomplete template parameter list for the synthesized deduction guide.

This patch addresses the issue by using the index and depth information to retrieve the corresponding template parameter, rather than relying on TTP->getDecl().

Fixes #90209

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

2 Files Affected:

(modified) clang/lib/Sema/SemaTemplate.cpp (+23-8)
(modified) clang/test/AST/ast-dump-ctad-alias.cpp (+25)

diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 07b3f793b3a29..46be2e90df847 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -2653,20 +2653,34 @@ struct ConvertConstructorToDeductionGuideTransform {
 // Find all template parameters that appear in the given DeducedArgs.
 // Return the indices of the template parameters in the TemplateParams.
 SmallVector<unsigned> TemplateParamsReferencedInTemplateArgumentList(
-    ArrayRef<NamedDecl *> TemplateParams,
+    const TemplateParameterList* TemplateParamsList,
     ArrayRef<TemplateArgument> DeducedArgs) {
   struct TemplateParamsReferencedFinder
       : public RecursiveASTVisitor<TemplateParamsReferencedFinder> {
+    const TemplateParameterList* TemplateParamList;
     llvm::DenseSet<NamedDecl *> TemplateParams;
     llvm::DenseSet<const NamedDecl *> ReferencedTemplateParams;
 
-    TemplateParamsReferencedFinder(ArrayRef<NamedDecl *> TemplateParams)
-        : TemplateParams(TemplateParams.begin(), TemplateParams.end()) {}
+    TemplateParamsReferencedFinder(
+        const TemplateParameterList *TemplateParamList)
+        : TemplateParamList(TemplateParamList),
+          TemplateParams(TemplateParamList->begin(), TemplateParamList->end()) {
+    }
 
     bool VisitTemplateTypeParmType(TemplateTypeParmType *TTP) {
-      MarkAppeared(TTP->getDecl());
+      // We use the index and depth to retrieve the corresponding template
+      // parameter from the parameter list.
+      // Note that Clang may not preserve type sugar during template argument
+      // deduction. In such cases, the TTP is a canonical TemplateTypeParamType,
+      // which only retains its index and depth information.
+      if (TTP->getDepth() == TemplateParamList->getDepth() &&
+          TTP->getIndex() < TemplateParamList->size()) {
+        ReferencedTemplateParams.insert(
+            TemplateParamList->getParam(TTP->getIndex()));
+      }
       return true;
     }
+
     bool VisitDeclRefExpr(DeclRefExpr *DRE) {
       MarkAppeared(DRE->getFoundDecl());
       return true;
@@ -2683,12 +2697,13 @@ SmallVector<unsigned> TemplateParamsReferencedInTemplateArgumentList(
         ReferencedTemplateParams.insert(ND);
     }
   };
-  TemplateParamsReferencedFinder Finder(TemplateParams);
+  TemplateParamsReferencedFinder Finder(TemplateParamsList);
   Finder.TraverseTemplateArguments(DeducedArgs);
 
   SmallVector<unsigned> Results;
-  for (unsigned Index = 0; Index < TemplateParams.size(); ++Index) {
-    if (Finder.ReferencedTemplateParams.contains(TemplateParams[Index]))
+  for (unsigned Index = 0; Index < TemplateParamsList->size(); ++Index) {
+    if (Finder.ReferencedTemplateParams.contains(
+            TemplateParamsList->getParam(Index)))
       Results.push_back(Index);
   }
   return Results;
@@ -3047,7 +3062,7 @@ BuildDeductionGuideForTypeAlias(Sema &SemaRef,
   }
   auto DeducedAliasTemplateParams =
       TemplateParamsReferencedInTemplateArgumentList(
-          AliasTemplate->getTemplateParameters()->asArray(), DeducedArgs);
+          AliasTemplate->getTemplateParameters(), DeducedArgs);
   // All template arguments null by default.
   SmallVector<TemplateArgument> TemplateArgsForBuildingFPrime(
       F->getTemplateParameters()->size());
diff --git a/clang/test/AST/ast-dump-ctad-alias.cpp b/clang/test/AST/ast-dump-ctad-alias.cpp
index 6f07a62e9a069..adccad97a205b 100644
--- a/clang/test/AST/ast-dump-ctad-alias.cpp
+++ b/clang/test/AST/ast-dump-ctad-alias.cpp
@@ -99,3 +99,28 @@ BFoo b2(1.0, 2.0);
 // CHECK-NEXT: | | |-ParmVarDecl {{.*}} 'type-parameter-0-0'
 // CHECK-NEXT: | | `-ParmVarDecl {{.*}} 'type-parameter-0-0'
 // CHECK-NEXT: | `-CXXDeductionGuideDecl {{.*}} implicit used <deduction guide for BFoo> 'auto (double, double) -> Foo<double, double>' implicit_instantiation
+
+namespace GH90209 {
+template <class Ts>
+struct List {
+  List(int);
+};
+
+template <class T1>
+struct TemplatedClass {
+  TemplatedClass(T1);
+};
+
+template <class T1>
+TemplatedClass(T1) -> TemplatedClass<List<T1>>;
+
+template <class T2>
+using ATemplatedClass = TemplatedClass<List<T2>>;
+
+ATemplatedClass test(1);
+// Verify that we have a correct template parameter list for the deduction guide.
+//
+// CHECK:      FunctionTemplateDecl {{.*}} <deduction guide for ATemplatedClass>
+// CHECK-NEXT: |-TemplateTypeParmDecl {{.*}} class depth 0 index 0 T2
+// CHECK-NEXT: |-TypeTraitExpr {{.*}} 'bool' __is_deducible
+} // namespace GH90209
\ No newline at end of file

github-actions · 2024-07-08T11:29:12Z

✅ With the latest revision this PR passed the C/C++ code formatter.

mizvekov · 2024-07-08T18:13:48Z

clang/test/AST/ast-dump-ctad-alias.cpp

+// CHECK:      FunctionTemplateDecl {{.*}} <deduction guide for ATemplatedClass>
+// CHECK-NEXT: |-TemplateTypeParmDecl {{.*}} class depth 0 index 0 T2
+// CHECK-NEXT: |-TypeTraitExpr {{.*}} 'bool' __is_deducible
+} // namespace GH90209


Minor nit: there is now a missing newline at end of file

The GitHub editor still shows the missing newline at end of file marker.

mizvekov · 2024-07-08T18:23:21Z

clang/lib/Sema/SemaTemplate.cpp

+      // Note that Clang may not preserve type sugar during template argument
+      // deduction. In such cases, the TTP is a canonical TemplateTypeParamType,
+      // which only retains its index and depth information.


I would remove this section of the comment, as I think this sort of implies it would be ok to use the type sugar if it were available, but if we rely on this for correct compilation of the program, it doesn't make sense to call it type sugar anymore.

Suggested change

// Note that Clang may not preserve type sugar during template argument

// deduction. In such cases, the TTP is a canonical TemplateTypeParamType,

// which only retains its index and depth information.

clang/lib/Sema/SemaTemplate.cpp

…TemplateParamsReferencedInTemplateArgumentList.

hokein

Thanks for the review.

hokein · 2024-07-11T09:02:35Z

clang/lib/Sema/SemaTemplate.cpp

+      // Note that Clang may not preserve type sugar during template argument
+      // deduction. In such cases, the TTP is a canonical TemplateTypeParamType,
+      // which only retains its index and depth information.


hokein · 2024-07-11T09:03:10Z

clang/test/AST/ast-dump-ctad-alias.cpp

+// CHECK:      FunctionTemplateDecl {{.*}} <deduction guide for ATemplatedClass>
+// CHECK-NEXT: |-TemplateTypeParmDecl {{.*}} class depth 0 index 0 T2
+// CHECK-NEXT: |-TypeTraitExpr {{.*}} 'bool' __is_deducible
+} // namespace GH90209


clang/lib/Sema/SemaTemplate.cpp

mizvekov

LGTM, sans:

missing newline at end of test file
A similar test for template template parameter.

…TemplateParamsReferencedInTemplateArgumentList (llvm#98013) As described in llvm#90209 (comment), Clang may not preserve enough information during template argument deduction. This can result in a merely canonical `TemplateTypeParmType` with a null `Decl`, leading to an incomplete template parameter list for the synthesized deduction guide. This patch addresses the issue by using the index and depth information to retrieve the corresponding template parameter, rather than relying on `TTP->getDecl()`. Fixes llvm#90209

hokein requested review from mizvekov and zyn0217 July 8, 2024 11:25

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

mizvekov reviewed Jul 8, 2024

View reviewed changes

hokein added 2 commits July 11, 2024 09:36

[clang] CTAD: use index and depth to retrieve template parameters in …

c9297f8

…TemplateParamsReferencedInTemplateArgumentList.

Address review comment.

7626982

hokein force-pushed the fix-ctad-template-ref branch from 1b4a711 to 7626982 Compare July 11, 2024 09:10

hokein commented Jul 11, 2024

View reviewed changes

clang-format

71b589b

mizvekov approved these changes Jul 11, 2024

View reviewed changes

hokein added 2 commits July 11, 2024 14:24

Add testcase for template template parameter

3f4d58e

clang-format

e0d2289

hokein merged commit 8b7263b into llvm:main Jul 11, 2024
5 of 6 checks passed

hokein deleted the fix-ctad-template-ref branch July 11, 2024 14:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[clang] CTAD: use index and depth to retrieve template parameter for TemplateParamsReferencedInTemplateArgumentList #98013

[clang] CTAD: use index and depth to retrieve template parameter for TemplateParamsReferencedInTemplateArgumentList #98013

Uh oh!

hokein commented Jul 8, 2024

Uh oh!

llvmbot commented Jul 8, 2024

Uh oh!

github-actions bot commented Jul 8, 2024 •

edited

Loading

Uh oh!

mizvekov Jul 8, 2024

Uh oh!

hokein Jul 11, 2024

Uh oh!

mizvekov Jul 11, 2024

Uh oh!

mizvekov Jul 8, 2024

Uh oh!

hokein Jul 11, 2024

Uh oh!

Uh oh!

Uh oh!

hokein left a comment

Uh oh!

hokein Jul 11, 2024

Uh oh!

hokein Jul 11, 2024

Uh oh!

Uh oh!

Uh oh!

mizvekov left a comment

Uh oh!

Uh oh!

Uh oh!

	// Note that Clang may not preserve type sugar during template argument
	// deduction. In such cases, the TTP is a canonical TemplateTypeParamType,
	// which only retains its index and depth information.

[clang] CTAD: use index and depth to retrieve template parameter for TemplateParamsReferencedInTemplateArgumentList #98013

[clang] CTAD: use index and depth to retrieve template parameter for TemplateParamsReferencedInTemplateArgumentList #98013

Uh oh!

Conversation

hokein commented Jul 8, 2024

Uh oh!

llvmbot commented Jul 8, 2024

Uh oh!

github-actions bot commented Jul 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mizvekov Jul 8, 2024

Choose a reason for hiding this comment

Uh oh!

hokein Jul 11, 2024

Choose a reason for hiding this comment

Uh oh!

mizvekov Jul 11, 2024

Choose a reason for hiding this comment

Uh oh!

mizvekov Jul 8, 2024

Choose a reason for hiding this comment

Uh oh!

hokein Jul 11, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

hokein left a comment

Choose a reason for hiding this comment

Uh oh!

hokein Jul 11, 2024

Choose a reason for hiding this comment

Uh oh!

hokein Jul 11, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mizvekov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Jul 8, 2024 •

edited

Loading