[llvm-branch-commits] [clang] release/18.x: [Clang][Sema] Revise the transformation of CTAD parameters of nested class templates (#91628) (PR #91890)
tstellar wrote: @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. https://github.com/llvm/llvm-project/pull/91890 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [Clang][Sema] Revise the transformation of CTAD parameters of nested class templates (#91628) (PR #91890)
https://github.com/tstellar closed https://github.com/llvm/llvm-project/pull/91890 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [Clang][Sema] Revise the transformation of CTAD parameters of nested class templates (#91628) (PR #91890)
https://github.com/tstellar updated
https://github.com/llvm/llvm-project/pull/91890
>From 8d65f14cb6ff87536433b934fea3730c63bd466a Mon Sep 17 00:00:00 2001
From: Younan Zhang
Date: Fri, 10 May 2024 20:47:15 +0800
Subject: [PATCH] [Clang][Sema] Revise the transformation of CTAD parameters of
nested class templates (#91628)
This fixes a regression introduced by bee78b88f.
When we form a deduction guide for a constructor, basically, we do the
following work:
- Collect template parameters from the constructor's surrounding class
template, if present.
- Collect template parameters from the constructor.
- Splice these template parameters together into a new template
parameter list.
- Turn all the references (e.g. the function parameter list) to the
invented parameter list by applying a `TreeTransform` to the function
type.
In the previous fix, we handled cases of nested class templates by
substituting the "outer" template parameters (i.e. those not declared at
the surrounding class template or the constructor) with the
instantiating template arguments. The approach per se makes sense, but
there was a flaw in the following case:
```cpp
template struct X {
template struct Y {
template Y(T) {}
};
template Y(T) -> Y;
};
X::Y y(42);
```
While we're transforming the parameters for `Y(T)`, we first attempt to
transform all references to `V` and `T`; then, we handle the references
to outer parameters `U` and `Us` using the template arguments from
`X` by transforming the same `ParamDecl`. However, the first step
results in the reference `T` being `` because the
invented `T` is the last of the parameter list of the deduction guide,
and what we're substituting with is a corresponding parameter pack
(which is `Us`, though empty). Hence we're messing up the substitution.
I think we can resolve it by reversing the substitution order, which
means handling outer template parameters first and then the inner
parameters.
There's no release note because this is a regression in 18, and I hope
we can catch up with the last release.
Fixes https://github.com/llvm/llvm-project/issues/88142
(cherry picked from commit 8c852ab57932a5cd954cb0d050c3d2ab486428df)
---
clang/lib/Sema/SemaTemplate.cpp | 25 ++-
.../nested-implicit-deduction-guides.cpp | 14 +++
2 files changed, 33 insertions(+), 6 deletions(-)
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index b619f5d729e86..a12a64939c464 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -2404,9 +2404,6 @@ struct ConvertConstructorToDeductionGuideTransform {
Args.addOuterRetainedLevel();
}
-if (NestedPattern)
- Args.addOuterRetainedLevels(NestedPattern->getTemplateDepth());
-
FunctionProtoTypeLoc FPTL = CD->getTypeSourceInfo()->getTypeLoc()
.getAsAdjusted();
assert(FPTL && "no prototype for constructor declaration");
@@ -2526,11 +2523,27 @@ struct ConvertConstructorToDeductionGuideTransform {
//-- The types of the function parameters are those of the constructor.
for (auto *OldParam : TL.getParams()) {
- ParmVarDecl *NewParam =
- transformFunctionTypeParam(OldParam, Args, MaterializedTypedefs);
- if (NestedPattern && NewParam)
+ ParmVarDecl *NewParam = OldParam;
+ // Given
+ // template struct C {
+ // template struct D {
+ // template D(U, V);
+ // };
+ // };
+ // First, transform all the references to template parameters that are
+ // defined outside of the surrounding class template. That is T in the
+ // above example.
+ if (NestedPattern) {
NewParam = transformFunctionTypeParam(NewParam, OuterInstantiationArgs,
MaterializedTypedefs);
+if (!NewParam)
+ return QualType();
+ }
+ // Then, transform all the references to template parameters that are
+ // defined at the class template and the constructor. In this example,
+ // they're U and V, respectively.
+ NewParam =
+ transformFunctionTypeParam(NewParam, Args, MaterializedTypedefs);
if (!NewParam)
return QualType();
ParamTypes.push_back(NewParam->getType());
diff --git a/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp
b/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp
index 38b6706595a11..f289dc0452868 100644
--- a/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp
+++ b/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp
@@ -84,3 +84,17 @@ nested_init_list::concept_fail nil_invalid{1, ""};
// expected-note@#INIT_LIST_INNER_INVALID {{candidate template ignored:
substitution failure [with F = const char *]: constraints not satisfied for
class template 'concept_fail' [with F = const char *]}}
// expected-note@#INIT_LIST_INNER_INVALID {{candidate
[llvm-branch-commits] [clang] release/18.x: [Clang][Sema] Revise the transformation of CTAD parameters of nested class templates (#91628) (PR #91890)
https://github.com/erichkeane approved this pull request. This seems useful enough with little enough impact to add to the release branch. If we've got another one coming, I see no reason not to. https://github.com/llvm/llvm-project/pull/91890 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [Clang][Sema] Revise the transformation of CTAD parameters of nested class templates (#91628) (PR #91890)
https://github.com/llvmbot updated
https://github.com/llvm/llvm-project/pull/91890
>From 7dcca34943097af1863f5a8718a41c888bedb682 Mon Sep 17 00:00:00 2001
From: Younan Zhang
Date: Fri, 10 May 2024 20:47:15 +0800
Subject: [PATCH] [Clang][Sema] Revise the transformation of CTAD parameters of
nested class templates (#91628)
This fixes a regression introduced by bee78b88f.
When we form a deduction guide for a constructor, basically, we do the
following work:
- Collect template parameters from the constructor's surrounding class
template, if present.
- Collect template parameters from the constructor.
- Splice these template parameters together into a new template
parameter list.
- Turn all the references (e.g. the function parameter list) to the
invented parameter list by applying a `TreeTransform` to the function
type.
In the previous fix, we handled cases of nested class templates by
substituting the "outer" template parameters (i.e. those not declared at
the surrounding class template or the constructor) with the
instantiating template arguments. The approach per se makes sense, but
there was a flaw in the following case:
```cpp
template struct X {
template struct Y {
template Y(T) {}
};
template Y(T) -> Y;
};
X::Y y(42);
```
While we're transforming the parameters for `Y(T)`, we first attempt to
transform all references to `V` and `T`; then, we handle the references
to outer parameters `U` and `Us` using the template arguments from
`X` by transforming the same `ParamDecl`. However, the first step
results in the reference `T` being `` because the
invented `T` is the last of the parameter list of the deduction guide,
and what we're substituting with is a corresponding parameter pack
(which is `Us`, though empty). Hence we're messing up the substitution.
I think we can resolve it by reversing the substitution order, which
means handling outer template parameters first and then the inner
parameters.
There's no release note because this is a regression in 18, and I hope
we can catch up with the last release.
Fixes https://github.com/llvm/llvm-project/issues/88142
(cherry picked from commit 8c852ab57932a5cd954cb0d050c3d2ab486428df)
---
clang/lib/Sema/SemaTemplate.cpp | 25 ++-
.../nested-implicit-deduction-guides.cpp | 14 +++
2 files changed, 33 insertions(+), 6 deletions(-)
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index b619f5d729e86..a12a64939c464 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -2404,9 +2404,6 @@ struct ConvertConstructorToDeductionGuideTransform {
Args.addOuterRetainedLevel();
}
-if (NestedPattern)
- Args.addOuterRetainedLevels(NestedPattern->getTemplateDepth());
-
FunctionProtoTypeLoc FPTL = CD->getTypeSourceInfo()->getTypeLoc()
.getAsAdjusted();
assert(FPTL && "no prototype for constructor declaration");
@@ -2526,11 +2523,27 @@ struct ConvertConstructorToDeductionGuideTransform {
//-- The types of the function parameters are those of the constructor.
for (auto *OldParam : TL.getParams()) {
- ParmVarDecl *NewParam =
- transformFunctionTypeParam(OldParam, Args, MaterializedTypedefs);
- if (NestedPattern && NewParam)
+ ParmVarDecl *NewParam = OldParam;
+ // Given
+ // template struct C {
+ // template struct D {
+ // template D(U, V);
+ // };
+ // };
+ // First, transform all the references to template parameters that are
+ // defined outside of the surrounding class template. That is T in the
+ // above example.
+ if (NestedPattern) {
NewParam = transformFunctionTypeParam(NewParam, OuterInstantiationArgs,
MaterializedTypedefs);
+if (!NewParam)
+ return QualType();
+ }
+ // Then, transform all the references to template parameters that are
+ // defined at the class template and the constructor. In this example,
+ // they're U and V, respectively.
+ NewParam =
+ transformFunctionTypeParam(NewParam, Args, MaterializedTypedefs);
if (!NewParam)
return QualType();
ParamTypes.push_back(NewParam->getType());
diff --git a/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp
b/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp
index 38b6706595a11..f289dc0452868 100644
--- a/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp
+++ b/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp
@@ -84,3 +84,17 @@ nested_init_list::concept_fail nil_invalid{1, ""};
// expected-note@#INIT_LIST_INNER_INVALID {{candidate template ignored:
substitution failure [with F = const char *]: constraints not satisfied for
class template 'concept_fail' [with F = const char *]}}
// expected-note@#INIT_LIST_INNER_INVALID {{candidate
[llvm-branch-commits] [clang] release/18.x: [Clang][Sema] Revise the transformation of CTAD parameters of nested class templates (#91628) (PR #91890)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: None (llvmbot)
Changes
Backport 8c852ab57932
Requested by: @zyn0217
---
Full diff: https://github.com/llvm/llvm-project/pull/91890.diff
2 Files Affected:
- (modified) clang/lib/Sema/SemaTemplate.cpp (+19-6)
- (modified) clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp (+14)
``diff
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index b619f5d729e86..a12a64939c464 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -2404,9 +2404,6 @@ struct ConvertConstructorToDeductionGuideTransform {
Args.addOuterRetainedLevel();
}
-if (NestedPattern)
- Args.addOuterRetainedLevels(NestedPattern->getTemplateDepth());
-
FunctionProtoTypeLoc FPTL = CD->getTypeSourceInfo()->getTypeLoc()
.getAsAdjusted();
assert(FPTL && "no prototype for constructor declaration");
@@ -2526,11 +2523,27 @@ struct ConvertConstructorToDeductionGuideTransform {
//-- The types of the function parameters are those of the constructor.
for (auto *OldParam : TL.getParams()) {
- ParmVarDecl *NewParam =
- transformFunctionTypeParam(OldParam, Args, MaterializedTypedefs);
- if (NestedPattern && NewParam)
+ ParmVarDecl *NewParam = OldParam;
+ // Given
+ // template struct C {
+ // template struct D {
+ // template D(U, V);
+ // };
+ // };
+ // First, transform all the references to template parameters that are
+ // defined outside of the surrounding class template. That is T in the
+ // above example.
+ if (NestedPattern) {
NewParam = transformFunctionTypeParam(NewParam, OuterInstantiationArgs,
MaterializedTypedefs);
+if (!NewParam)
+ return QualType();
+ }
+ // Then, transform all the references to template parameters that are
+ // defined at the class template and the constructor. In this example,
+ // they're U and V, respectively.
+ NewParam =
+ transformFunctionTypeParam(NewParam, Args, MaterializedTypedefs);
if (!NewParam)
return QualType();
ParamTypes.push_back(NewParam->getType());
diff --git a/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp
b/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp
index 38b6706595a11..f289dc0452868 100644
--- a/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp
+++ b/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp
@@ -84,3 +84,17 @@ nested_init_list::concept_fail nil_invalid{1, ""};
// expected-note@#INIT_LIST_INNER_INVALID {{candidate template ignored:
substitution failure [with F = const char *]: constraints not satisfied for
class template 'concept_fail' [with F = const char *]}}
// expected-note@#INIT_LIST_INNER_INVALID {{candidate function template not
viable: requires 1 argument, but 2 were provided}}
// expected-note@#INIT_LIST_INNER_INVALID {{candidate function template not
viable: requires 0 arguments, but 2 were provided}}
+
+namespace GH88142 {
+
+template struct X {
+ template struct Y {
+template Y(T) {}
+ };
+
+ template Y(T) -> Y;
+};
+
+X::Y y(42);
+
+} // namespace PR88142
``
https://github.com/llvm/llvm-project/pull/91890
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [Clang][Sema] Revise the transformation of CTAD parameters of nested class templates (#91628) (PR #91890)
llvmbot wrote: @zyn0217 What do you think about merging this PR to the release branch? https://github.com/llvm/llvm-project/pull/91890 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [Clang][Sema] Revise the transformation of CTAD parameters of nested class templates (#91628) (PR #91890)
https://github.com/llvmbot milestoned https://github.com/llvm/llvm-project/pull/91890 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [Clang][Sema] Revise the transformation of CTAD parameters of nested class templates (#91628) (PR #91890)
https://github.com/llvmbot created
https://github.com/llvm/llvm-project/pull/91890
Backport 8c852ab57932
Requested by: @zyn0217
>From e45b88f445fdbc90c0ba40f31ef264ba8a20baa9 Mon Sep 17 00:00:00 2001
From: Younan Zhang
Date: Fri, 10 May 2024 20:47:15 +0800
Subject: [PATCH] [Clang][Sema] Revise the transformation of CTAD parameters of
nested class templates (#91628)
This fixes a regression introduced by bee78b88f.
When we form a deduction guide for a constructor, basically, we do the
following work:
- Collect template parameters from the constructor's surrounding class
template, if present.
- Collect template parameters from the constructor.
- Splice these template parameters together into a new template
parameter list.
- Turn all the references (e.g. the function parameter list) to the
invented parameter list by applying a `TreeTransform` to the function
type.
In the previous fix, we handled cases of nested class templates by
substituting the "outer" template parameters (i.e. those not declared at
the surrounding class template or the constructor) with the
instantiating template arguments. The approach per se makes sense, but
there was a flaw in the following case:
```cpp
template struct X {
template struct Y {
template Y(T) {}
};
template Y(T) -> Y;
};
X::Y y(42);
```
While we're transforming the parameters for `Y(T)`, we first attempt to
transform all references to `V` and `T`; then, we handle the references
to outer parameters `U` and `Us` using the template arguments from
`X` by transforming the same `ParamDecl`. However, the first step
results in the reference `T` being `` because the
invented `T` is the last of the parameter list of the deduction guide,
and what we're substituting with is a corresponding parameter pack
(which is `Us`, though empty). Hence we're messing up the substitution.
I think we can resolve it by reversing the substitution order, which
means handling outer template parameters first and then the inner
parameters.
There's no release note because this is a regression in 18, and I hope
we can catch up with the last release.
Fixes https://github.com/llvm/llvm-project/issues/88142
(cherry picked from commit 8c852ab57932a5cd954cb0d050c3d2ab486428df)
---
clang/lib/Sema/SemaTemplate.cpp | 25 ++-
.../nested-implicit-deduction-guides.cpp | 14 +++
2 files changed, 33 insertions(+), 6 deletions(-)
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index b619f5d729e86..a12a64939c464 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -2404,9 +2404,6 @@ struct ConvertConstructorToDeductionGuideTransform {
Args.addOuterRetainedLevel();
}
-if (NestedPattern)
- Args.addOuterRetainedLevels(NestedPattern->getTemplateDepth());
-
FunctionProtoTypeLoc FPTL = CD->getTypeSourceInfo()->getTypeLoc()
.getAsAdjusted();
assert(FPTL && "no prototype for constructor declaration");
@@ -2526,11 +2523,27 @@ struct ConvertConstructorToDeductionGuideTransform {
//-- The types of the function parameters are those of the constructor.
for (auto *OldParam : TL.getParams()) {
- ParmVarDecl *NewParam =
- transformFunctionTypeParam(OldParam, Args, MaterializedTypedefs);
- if (NestedPattern && NewParam)
+ ParmVarDecl *NewParam = OldParam;
+ // Given
+ // template struct C {
+ // template struct D {
+ // template D(U, V);
+ // };
+ // };
+ // First, transform all the references to template parameters that are
+ // defined outside of the surrounding class template. That is T in the
+ // above example.
+ if (NestedPattern) {
NewParam = transformFunctionTypeParam(NewParam, OuterInstantiationArgs,
MaterializedTypedefs);
+if (!NewParam)
+ return QualType();
+ }
+ // Then, transform all the references to template parameters that are
+ // defined at the class template and the constructor. In this example,
+ // they're U and V, respectively.
+ NewParam =
+ transformFunctionTypeParam(NewParam, Args, MaterializedTypedefs);
if (!NewParam)
return QualType();
ParamTypes.push_back(NewParam->getType());
diff --git a/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp
b/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp
index 38b6706595a11..f289dc0452868 100644
--- a/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp
+++ b/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp
@@ -84,3 +84,17 @@ nested_init_list::concept_fail nil_invalid{1, ""};
// expected-note@#INIT_LIST_INNER_INVALID {{candidate template ignored:
substitution failure [with F = const char *]: constraints not satisfied for
class template 'concept_fail' [with F = const char *]}}
//
