[clang] CTAD alias: fix transformation for require-clause expr Part2. #93533
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Haojian Wu (hokein) ChangesIn the #90961 fix, we miss a case where the undeduced template parameters of the underlying deduction guide are not transformed, which leaves incorrect depth/index information, and causes crashes when evaluating constraints. This patch fix this missing case. Fixes #92596 Full diff: https://github.com/llvm/llvm-project/pull/93533.diff 3 Files Affected:
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 39e9dbed0c3e0..62d7097f68785 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -2780,6 +2780,7 @@ Expr *
buildAssociatedConstraints(Sema &SemaRef, FunctionTemplateDecl *F,
TypeAliasTemplateDecl *AliasTemplate,
ArrayRef<DeducedTemplateArgument> DeduceResults,
+ unsigned UndeducedTemplateParameterStartIndex,
Expr *IsDeducible) {
Expr *RC = F->getTemplateParameters()->getRequiresClause();
if (!RC)
@@ -2840,8 +2841,22 @@ buildAssociatedConstraints(Sema &SemaRef, FunctionTemplateDecl *F,
for (unsigned Index = 0; Index < DeduceResults.size(); ++Index) {
const auto &D = DeduceResults[Index];
- if (D.isNull())
+ if (D.isNull()) { // non-deduced template parameters of f
+ auto TP = F->getTemplateParameters()->getParam(Index);
+ MultiLevelTemplateArgumentList Args;
+ Args.setKind(TemplateSubstitutionKind::Rewrite);
+ Args.addOuterTemplateArguments(TemplateArgsForBuildingRC);
+ // Rebuild the template parameter with updated depth and index.
+ NamedDecl *NewParam = transformTemplateParameter(
+ SemaRef, F->getDeclContext(), TP, Args,
+ /*NewIndex=*/UndeducedTemplateParameterStartIndex++,
+ getTemplateParameterDepth(TP) + AdjustDepth);
+
+ assert(TemplateArgsForBuildingRC[Index].isNull());
+ TemplateArgsForBuildingRC[Index] = Context.getCanonicalTemplateArgument(
+ Context.getInjectedTemplateArg(NewParam));
continue;
+ }
TemplateArgumentLoc Input =
SemaRef.getTrivialTemplateArgumentLoc(D, QualType(), SourceLocation{});
TemplateArgumentLoc Output;
@@ -2857,9 +2872,11 @@ buildAssociatedConstraints(Sema &SemaRef, FunctionTemplateDecl *F,
MultiLevelTemplateArgumentList ArgsForBuildingRC;
ArgsForBuildingRC.setKind(clang::TemplateSubstitutionKind::Rewrite);
ArgsForBuildingRC.addOuterTemplateArguments(TemplateArgsForBuildingRC);
- // For 2), if the underlying F is instantiated from a member template, we need
- // the entire template argument list, as the constraint AST in the
- // require-clause of F remains completely uninstantiated.
+ // For 2), if the underlying function template F is nested in a class template
+ // (either instantiated from an explicitly-written deduction guide, or
+ // synthesized from a constructor), we need the entire template argument list,
+ // as the constraint AST in the require-clause of F remains completely
+ // uninstantiated.
//
// For example:
// template <typename T> // depth 0
@@ -2882,7 +2899,8 @@ buildAssociatedConstraints(Sema &SemaRef, FunctionTemplateDecl *F,
// We add the outer template arguments which is [int] to the multi-level arg
// list to ensure that the occurrence U in `C<U>` will be replaced with int
// during the substitution.
- if (F->getInstantiatedFromMemberTemplate()) {
+ if (F->getLexicalDeclContext()->getDeclKind() ==
+ clang::Decl::ClassTemplateSpecialization) {
auto OuterLevelArgs = SemaRef.getTemplateInstantiationArgs(
F, F->getLexicalDeclContext(),
/*Final=*/false, /*Innermost=*/std::nullopt,
@@ -3100,6 +3118,7 @@ BuildDeductionGuideForTypeAlias(Sema &SemaRef,
Context.getInjectedTemplateArg(NewParam));
TransformedDeducedAliasArgs[AliasTemplateParamIdx] = NewTemplateArgument;
}
+ unsigned UndeducedTemplateParameterStartIndex = FPrimeTemplateParams.size();
// ...followed by the template parameters of f that were not deduced
// (including their default template arguments)
for (unsigned FTemplateParamIdx : NonDeducedTemplateParamsInFIndex) {
@@ -3169,7 +3188,8 @@ BuildDeductionGuideForTypeAlias(Sema &SemaRef,
Expr *IsDeducible = buildIsDeducibleConstraint(
SemaRef, AliasTemplate, FPrime->getReturnType(), FPrimeTemplateParams);
Expr *RequiresClause = buildAssociatedConstraints(
- SemaRef, F, AliasTemplate, DeduceResults, IsDeducible);
+ SemaRef, F, AliasTemplate, DeduceResults,
+ UndeducedTemplateParameterStartIndex, IsDeducible);
// FIXME: implement the is_deducible constraint per C++
// [over.match.class.deduct]p3.3:
diff --git a/clang/test/AST/ast-dump-ctad-alias.cpp b/clang/test/AST/ast-dump-ctad-alias.cpp
index 9382558393e4c..357ff8a244652 100644
--- a/clang/test/AST/ast-dump-ctad-alias.cpp
+++ b/clang/test/AST/ast-dump-ctad-alias.cpp
@@ -49,6 +49,31 @@ Out2<double>::AInner t(1.0);
// CHECK-NEXT: | | `-BuiltinType {{.*}} 'double'
// CHECK-NEXT: | `-ParmVarDecl {{.*}} 'double'
+// GH92596
+template <typename T0>
+struct Out3 {
+ template<class T1, typename T2>
+ struct Foo {
+ // Deduction guide: Foo(T1, T2, V) -> Foo<T1, T2, V>;
+ template<class V> requires Concept<T0, V> // V in require clause of Foo deduction guide: depth 1, index: 2
+ Foo(V, T1);
+ };
+};
+template<class T3>
+using AFoo3 = Out3<int>::Foo<T3, T3>;
+AFoo3 afoo3{0, 1};
+// Verify occurrence V in the require-clause is transformed (depth: 1 => 0, index: 2 => 1) correctly.
+
+// CHECK: FunctionTemplateDecl {{.*}} implicit <deduction guide for AFoo3>
+// CHECK-NEXT: |-TemplateTypeParmDecl {{.*}} class depth 0 index 0 T3
+// CHECK-NEXT: |-TemplateTypeParmDecl {{.*}} class depth 0 index 1 V
+// CHECK-NEXT: |-BinaryOperator {{.*}} '<dependent type>' '&&'
+// CHECK-NEXT: | |-UnresolvedLookupExpr {{.*}} '<dependent type>' lvalue (no ADL) = 'Concept'
+// CHECK-NEXT: | | |-TemplateArgument type 'int'
+// CHECK-NEXT: | | | `-BuiltinType {{.*}} 'int'
+// CHECK-NEXT: | | `-TemplateArgument type 'type-parameter-0-1'
+// CHECK-NEXT: | | `-TemplateTypeParmType {{.*}} 'type-parameter-0-1' dependent depth 0 index 1
+
template <typename... T1>
struct Foo {
Foo(T1...);
diff --git a/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp b/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp
index b71dfc6ccaf4f..224eebd21f29d 100644
--- a/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp
+++ b/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp
@@ -401,4 +401,29 @@ struct A1 {
template <typename U>
using AFoo = A1<int>::A2<int>::Foo<U>;
AFoo case3(1);
+
+// Case4: crashes on the constexpr evaluator due to the mixed-up index for the
+// template parameters `V`.
+template<class T, typename T2>
+struct Case4 {
+ template<class V> requires C<V>
+ Case4(V, T);
+};
+
+template<class T2>
+using ACase4 = Case4<T2, T2>;
+ACase4 case4{0, 1};
+
} // namespace test24
+
+namespace GH92212 {
+template<typename T, typename...Us>
+struct A{
+ template<typename V> requires __is_same(V, int)
+ A(V);
+};
+
+template<typename...TS>
+using AA = A<int, TS...>;
+AA a{0};
+}
|
mizvekov
reviewed
May 28, 2024
clang/lib/Sema/SemaTemplate.cpp
Outdated
| @@ -2840,8 +2841,22 @@ buildAssociatedConstraints(Sema &SemaRef, FunctionTemplateDecl *F, | |||
|
|
|||
| for (unsigned Index = 0; Index < DeduceResults.size(); ++Index) { | |||
| const auto &D = DeduceResults[Index]; | |||
| if (D.isNull()) | |||
| if (D.isNull()) { // non-deduced template parameters of f | |||
| auto TP = F->getTemplateParameters()->getParam(Index); | |||
There was a problem hiding this comment.
Small nit:
Suggested change
| auto TP = F->getTemplateParameters()->getParam(Index); | |
| NamedDecl *TP = F->getTemplateParameters()->getParam(Index); |
clang/lib/Sema/SemaTemplate.cpp
Outdated
Comment on lines
2850
to
2816
| NamedDecl *NewParam = transformTemplateParameter( | ||
| SemaRef, F->getDeclContext(), TP, Args, | ||
| /*NewIndex=*/UndeducedTemplateParameterStartIndex++, | ||
| getTemplateParameterDepth(TP) + AdjustDepth); |
There was a problem hiding this comment.
Took me a while to spot that one:
Suggested change
| NamedDecl *NewParam = transformTemplateParameter( | |
| SemaRef, F->getDeclContext(), TP, Args, | |
| /*NewIndex=*/UndeducedTemplateParameterStartIndex++, | |
| getTemplateParameterDepth(TP) + AdjustDepth); | |
| NamedDecl *NewParam = transformTemplateParameter( | |
| SemaRef, F->getDeclContext(), TP, Args, | |
| /*NewIndex=*/UndeducedTemplateParameterStartIndex, | |
| getTemplateParameterDepth(TP) + AdjustDepth); | |
| UndeducedTemplateParameterStartIndex += 1; |
| @@ -2882,7 +2899,8 @@ buildAssociatedConstraints(Sema &SemaRef, FunctionTemplateDecl *F, | |||
| // We add the outer template arguments which is [int] to the multi-level arg | |||
| // list to ensure that the occurrence U in `C<U>` will be replaced with int | |||
| // during the substitution. | |||
| if (F->getInstantiatedFromMemberTemplate()) { | |||
| if (F->getLexicalDeclContext()->getDeclKind() == | |||
| clang::Decl::ClassTemplateSpecialization) { | |||
There was a problem hiding this comment.
Should this apply to a ClassTemplatePartialSpecialization as well?
There was a problem hiding this comment.
Not needed. The F here is an instantiated template (either from an explicit deduction guide within a class or a constructor), so its DeclContext cannot be ClassTemplatePartialSpecialization. I made a code comment to clarify it.
clang/lib/Sema/SemaTemplate.cpp
Outdated
| @@ -3100,6 +3118,7 @@ BuildDeductionGuideForTypeAlias(Sema &SemaRef, | |||
| Context.getInjectedTemplateArg(NewParam)); | |||
| TransformedDeducedAliasArgs[AliasTemplateParamIdx] = NewTemplateArgument; | |||
| } | |||
| unsigned UndeducedTemplateParameterStartIndex = FPrimeTemplateParams.size(); | |||
There was a problem hiding this comment.
Nit, this is a bit of a mouth full:
Suggested change
| unsigned UndeducedTemplateParameterStartIndex = FPrimeTemplateParams.size(); | |
| unsigned FirstUndeducedParamIdx = FPrimeTemplateParams.size(); |
hokein
commented
Jun 27, 2024
| @@ -2882,7 +2899,8 @@ buildAssociatedConstraints(Sema &SemaRef, FunctionTemplateDecl *F, | |||
| // We add the outer template arguments which is [int] to the multi-level arg | |||
| // list to ensure that the occurrence U in `C<U>` will be replaced with int | |||
| // during the substitution. | |||
| if (F->getInstantiatedFromMemberTemplate()) { | |||
| if (F->getLexicalDeclContext()->getDeclKind() == | |||
| clang::Decl::ClassTemplateSpecialization) { | |||
There was a problem hiding this comment.
Not needed. The F here is an instantiated template (either from an explicit deduction guide within a class or a constructor), so its DeclContext cannot be ClassTemplatePartialSpecialization. I made a code comment to clarify it.
clang/lib/Sema/SemaTemplate.cpp
Outdated
| @@ -3100,6 +3118,7 @@ BuildDeductionGuideForTypeAlias(Sema &SemaRef, | |||
| Context.getInjectedTemplateArg(NewParam)); | |||
| TransformedDeducedAliasArgs[AliasTemplateParamIdx] = NewTemplateArgument; | |||
| } | |||
| unsigned UndeducedTemplateParameterStartIndex = FPrimeTemplateParams.size(); | |||
clang/lib/Sema/SemaTemplate.cpp
Outdated
| @@ -2840,8 +2841,22 @@ buildAssociatedConstraints(Sema &SemaRef, FunctionTemplateDecl *F, | |||
|
|
|||
| for (unsigned Index = 0; Index < DeduceResults.size(); ++Index) { | |||
| const auto &D = DeduceResults[Index]; | |||
| if (D.isNull()) | |||
| if (D.isNull()) { // non-deduced template parameters of f | |||
| auto TP = F->getTemplateParameters()->getParam(Index); | |||
clang/lib/Sema/SemaTemplate.cpp
Outdated
Comment on lines
2850
to
2816
| NamedDecl *NewParam = transformTemplateParameter( | ||
| SemaRef, F->getDeclContext(), TP, Args, | ||
| /*NewIndex=*/UndeducedTemplateParameterStartIndex++, | ||
| getTemplateParameterDepth(TP) + AdjustDepth); |
|
Friendly ping, I'd like to get it landed before the release cut. |
mizvekov
approved these changes
Jul 4, 2024
In the llvm#90961 fix, we miss a case where the undeduced template parameters of the underlying deduction guide is not transformed, which leaves incorrect depth/index information, and causes crash when evaluating the constraints. This patch fix this missing case. Fixes llvm#92596 Fixes llvm#92212
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
kbluck
pushed a commit
to kbluck/llvm-project
that referenced
this pull request
Jul 6, 2024
…llvm#93533) In the llvm#90961 fix, we miss a case where the undeduced template parameters of the underlying deduction guide are not transformed, which leaves incorrect depth/index information, and causes crashes when evaluating constraints. This patch fix this missing case. Fixes llvm#92596 Fixes llvm#92212
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In the #90961 fix, we miss a case where the undeduced template parameters of the underlying deduction guide are not transformed, which leaves incorrect depth/index information, and causes crashes when evaluating constraints.
This patch fix this missing case.
Fixes #92596
Fixes #92212