[clang] [Clang] Correct the order of substituted arguments in CTAD alias guides (PR #123022)
Younan Zhang via cfe-commits
cfe-commits at lists.llvm.org
Wed Jan 15 23:57:20 PST 2025
https://github.com/zyn0217 updated https://github.com/llvm/llvm-project/pull/123022
>From 549216ee8a58c8b60fcbc757e1b7041b5ef2da54 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Wed, 15 Jan 2025 15:25:20 +0800
Subject: [PATCH 1/2] [Clang] Correct the order of substituted arguments in
CTAD alias guides
We missed a case of type constraints referencing deduced template parameters
when constructing a deduction guide for the type alias. This patch fixes
the issue by swapping the order of constructing 'template arguments not
appearing in the type alias parameters' and 'template arguments that are
not yet deduced'.
---
clang/docs/ReleaseNotes.rst | 1 +
clang/lib/Sema/SemaTemplateDeductionGuide.cpp | 50 +++++++++----------
clang/test/SemaTemplate/deduction-guide.cpp | 50 +++++++++++++++++++
3 files changed, 75 insertions(+), 26 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 07a1a4195427d8..9b87e0f38e5eed 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -907,6 +907,7 @@ Bug Fixes to C++ Support
(`LWG3929 <https://wg21.link/LWG3929>`__.) (#GH121278)
- Clang now identifies unexpanded parameter packs within the type constraint on a non-type template parameter. (#GH88866)
- Fixed an issue while resolving type of expression indexing into a pack of values of non-dependent type (#GH121242)
+- Fixed incorrect construction of template arguments for CTAD alias guides when type constraints are applied. (#GH122134)
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaTemplateDeductionGuide.cpp b/clang/lib/Sema/SemaTemplateDeductionGuide.cpp
index d42c3765aa534f..1eb0abb41d3e9a 100644
--- a/clang/lib/Sema/SemaTemplateDeductionGuide.cpp
+++ b/clang/lib/Sema/SemaTemplateDeductionGuide.cpp
@@ -996,7 +996,7 @@ BuildDeductionGuideForTypeAlias(Sema &SemaRef,
F->getTemplateParameters()->size());
// FIXME: DeduceTemplateArguments stops immediately at the first
- // non-deducible template argument. However, this doesn't seem to casue
+ // non-deducible template argument. However, this doesn't seem to cause
// issues for practice cases, we probably need to extend it to continue
// performing deduction for rest of arguments to align with the C++
// standard.
@@ -1053,25 +1053,6 @@ BuildDeductionGuideForTypeAlias(Sema &SemaRef,
TransformedDeducedAliasArgs[AliasTemplateParamIdx] = NewTemplateArgument;
}
unsigned FirstUndeducedParamIdx = FPrimeTemplateParams.size();
- // ...followed by the template parameters of f that were not deduced
- // (including their default template arguments)
- for (unsigned FTemplateParamIdx : NonDeducedTemplateParamsInFIndex) {
- auto *TP = F->getTemplateParameters()->getParam(FTemplateParamIdx);
- MultiLevelTemplateArgumentList Args;
- Args.setKind(TemplateSubstitutionKind::Rewrite);
- // We take a shortcut here, it is ok to reuse the
- // TemplateArgsForBuildingFPrime.
- Args.addOuterTemplateArguments(TemplateArgsForBuildingFPrime);
- NamedDecl *NewParam = transformTemplateParameter(
- SemaRef, F->getDeclContext(), TP, Args, FPrimeTemplateParams.size(),
- getDepthAndIndex(TP).first);
- FPrimeTemplateParams.push_back(NewParam);
-
- assert(TemplateArgsForBuildingFPrime[FTemplateParamIdx].isNull() &&
- "The argument must be null before setting");
- TemplateArgsForBuildingFPrime[FTemplateParamIdx] =
- Context.getInjectedTemplateArg(NewParam);
- }
// To form a deduction guide f' from f, we leverage clang's instantiation
// mechanism, we construct a template argument list where the template
@@ -1086,18 +1067,14 @@ BuildDeductionGuideForTypeAlias(Sema &SemaRef,
// 2) non-deduced template parameters of f: rebuild a
// template argument;
//
- // 2) has been built already (when rebuilding the new template
- // parameters), we now perform 1).
+ // We now perform 1), as case 2) might refer to substituted 1).
MultiLevelTemplateArgumentList Args;
Args.setKind(TemplateSubstitutionKind::Rewrite);
Args.addOuterTemplateArguments(TransformedDeducedAliasArgs);
for (unsigned Index = 0; Index < DeduceResults.size(); ++Index) {
const auto &D = DeduceResults[Index];
if (D.isNull()) {
- // 2): Non-deduced template parameter has been built already.
- assert(!TemplateArgsForBuildingFPrime[Index].isNull() &&
- "template arguments for non-deduced template parameters should "
- "be been set!");
+ // 2): Non-deduced template parameters would be substituted later.
continue;
}
TemplateArgumentLoc Input =
@@ -1110,6 +1087,27 @@ BuildDeductionGuideForTypeAlias(Sema &SemaRef,
}
}
+ // Case 2)
+ // ...followed by the template parameters of f that were not deduced
+ // (including their default template arguments)
+ for (unsigned FTemplateParamIdx : NonDeducedTemplateParamsInFIndex) {
+ auto *TP = F->getTemplateParameters()->getParam(FTemplateParamIdx);
+ MultiLevelTemplateArgumentList Args;
+ Args.setKind(TemplateSubstitutionKind::Rewrite);
+ // We take a shortcut here, it is ok to reuse the
+ // TemplateArgsForBuildingFPrime.
+ Args.addOuterTemplateArguments(TemplateArgsForBuildingFPrime);
+ NamedDecl *NewParam = transformTemplateParameter(
+ SemaRef, F->getDeclContext(), TP, Args, FPrimeTemplateParams.size(),
+ getDepthAndIndex(TP).first);
+ FPrimeTemplateParams.push_back(NewParam);
+
+ assert(TemplateArgsForBuildingFPrime[FTemplateParamIdx].isNull() &&
+ "The argument must be null before setting");
+ TemplateArgsForBuildingFPrime[FTemplateParamIdx] =
+ Context.getInjectedTemplateArg(NewParam);
+ }
+
auto *TemplateArgListForBuildingFPrime =
TemplateArgumentList::CreateCopy(Context, TemplateArgsForBuildingFPrime);
// Form the f' by substituting the template arguments into f.
diff --git a/clang/test/SemaTemplate/deduction-guide.cpp b/clang/test/SemaTemplate/deduction-guide.cpp
index d03c783313dd71..39250f0617f4b5 100644
--- a/clang/test/SemaTemplate/deduction-guide.cpp
+++ b/clang/test/SemaTemplate/deduction-guide.cpp
@@ -478,3 +478,53 @@ A a{.f1 = {1}};
// CHECK-NEXT: `-DeclRefExpr {{.+}} <col:10> 'int' NonTypeTemplateParm {{.+}} 'N' 'int'
} // namespace GH83368
+
+namespace GH122134 {
+
+template <class, class>
+concept Constraint = true;
+
+template <class T, int> struct Struct {
+ Struct(Constraint<T> auto) {}
+};
+
+template <int N = 0> using Test = Struct<int, N>;
+
+Test test(42);
+
+// CHECK-LABEL: Dumping GH122134::<deduction guide for Test>:
+// CHECK-NEXT: FunctionTemplateDecl {{.*}} implicit <deduction guide for Test>
+// CHECK-NEXT: |-NonTypeTemplateParmDecl {{.*}} 'int' depth 0 index 0 N
+// CHECK-NEXT: | `-TemplateArgument {{.*}} expr '0'
+// CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 0
+// CHECK-NEXT: |-TemplateTypeParmDecl {{.*}} Concept {{.*}} 'Constraint' depth 0 index 1 auto:1
+// CHECK-NEXT: | `-ConceptSpecializationExpr {{.*}} 'bool' Concept {{.*}} 'Constraint'
+// CHECK-NEXT: | |-ImplicitConceptSpecializationDecl {{.*}}
+// CHECK-NEXT: | | |-TemplateArgument type 'type-parameter-0-1'
+// CHECK-NEXT: | | | `-TemplateTypeParmType {{.*}} 'type-parameter-0-1' dependent depth 0 index 1
+// CHECK-NEXT: | | `-TemplateArgument type 'int'
+// CHECK-NEXT: | | `-BuiltinType {{.*}} 'int'
+// CHECK-NEXT: | |-TemplateArgument {{.*}} type 'auto:1':'type-parameter-0-1'
+// CHECK-NEXT: | | `-TemplateTypeParmType {{.*}} 'auto:1' dependent depth 0 index 1
+// CHECK-NEXT: | | `-TemplateTypeParm {{.*}} 'auto:1'
+// CHECK-NEXT: | `-TemplateArgument {{.*}} type 'int'
+// CHECK-NEXT: | `-BuiltinType {{.*}} 'int'
+// CHECK-NEXT: |-TypeTraitExpr {{.*}} 'bool' __is_deducible
+// CHECK-NEXT: | |-DeducedTemplateSpecializationType {{.*}} 'GH122134::Test' dependent
+// CHECK-NEXT: | | `-name: 'GH122134::Test'
+// CHECK-NEXT: | | `-TypeAliasTemplateDecl {{.*}} Test
+// CHECK-NEXT: | `-TemplateSpecializationType {{.*}} 'Struct<int, N>' dependent
+// CHECK-NEXT: | |-name: 'Struct':'GH122134::Struct' qualified
+// CHECK-NEXT: | | `-ClassTemplateDecl {{.*}} Struct
+// CHECK-NEXT: | |-TemplateArgument type 'int'
+// CHECK-NEXT: | | `-SubstTemplateTypeParmType {{.*}} 'int' sugar class depth 0 index 0 T
+// CHECK-NEXT: | | |-FunctionTemplate {{.*}} '<deduction guide for Struct>'
+// CHECK-NEXT: | | `-BuiltinType {{.*}} 'int'
+// CHECK-NEXT: | `-TemplateArgument expr 'N'
+// CHECK-NEXT: | `-SubstNonTypeTemplateParmExpr {{.*}} 'int'
+// CHECK-NEXT: | |-NonTypeTemplateParmDecl {{.*}} 'int' depth 0 index 1
+// CHECK-NEXT: | `-DeclRefExpr {{.*}} 'int' NonTypeTemplateParm {{.*}} 'N' 'int'
+// CHECK-NEXT: |-CXXDeductionGuideDecl {{.*}} implicit <deduction guide for Test> 'auto (auto:1) -> Struct<int, N>'
+// CHECK-NEXT: | `-ParmVarDecl {{.*}} 'auto:1'
+
+} // namespace GH122134
>From 6e3f6a3aef84ac90f3862ff84999e7d05ffb7300 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Thu, 16 Jan 2025 15:56:10 +0800
Subject: [PATCH 2/2] Address feedback from corentin
---
clang/docs/ReleaseNotes.rst | 3 ++-
clang/lib/Sema/SemaTemplateDeductionGuide.cpp | 13 +++++++------
2 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 9b87e0f38e5eed..ea1f7a4ac6b434 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -907,7 +907,8 @@ Bug Fixes to C++ Support
(`LWG3929 <https://wg21.link/LWG3929>`__.) (#GH121278)
- Clang now identifies unexpanded parameter packs within the type constraint on a non-type template parameter. (#GH88866)
- Fixed an issue while resolving type of expression indexing into a pack of values of non-dependent type (#GH121242)
-- Fixed incorrect construction of template arguments for CTAD alias guides when type constraints are applied. (#GH122134)
+- Fixed a crash caused by the incorrect construction of template arguments for CTAD alias guides when type
+ constraints are applied. (#GH122134)
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaTemplateDeductionGuide.cpp b/clang/lib/Sema/SemaTemplateDeductionGuide.cpp
index 1eb0abb41d3e9a..5f813ba3a597a3 100644
--- a/clang/lib/Sema/SemaTemplateDeductionGuide.cpp
+++ b/clang/lib/Sema/SemaTemplateDeductionGuide.cpp
@@ -1061,13 +1061,14 @@ BuildDeductionGuideForTypeAlias(Sema &SemaRef,
// f, this ensures all template parameter occurrences are updated
// correctly.
//
- // The template argument list is formed from the `DeducedArgs`, two parts:
- // 1) appeared template parameters of alias: transfrom the deduced
- // template argument;
- // 2) non-deduced template parameters of f: rebuild a
- // template argument;
+ // The template argument list is formed, in order, from
+ // 1) For the template parameters of the alias, the corresponding deduced
+ // template arguments
+ // 2) For the non-deduced template parameters of f. the
+ // (rebuilt) template arguments corresponding.
//
- // We now perform 1), as case 2) might refer to substituted 1).
+ // Note: the non-deduced template arguments of `f` might refer to arguments
+ // deduced in 1), as in a type constraint.
MultiLevelTemplateArgumentList Args;
Args.setKind(TemplateSubstitutionKind::Rewrite);
Args.addOuterTemplateArguments(TransformedDeducedAliasArgs);
More information about the cfe-commits
mailing list