-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[Clang] Fix another parameter mapping substitution bug #162155
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Clang] Fix another parameter mapping substitution bug #162155
Conversation
When a template parameter pack is named before it can be substituted, it might not have a corresponding mapping. Fixes llvm#162125
@llvm/pr-subscribers-clang Author: Corentin Jabot (cor3ntin) ChangesWhen a template parameter pack is named before Fixes #162125 Full diff: https://github.com/llvm/llvm-project/pull/162155.diff 2 Files Affected:
diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index 999e302c02535..cb42d651a6586 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -277,7 +277,8 @@ class HashParameterMapping : public RecursiveASTVisitor<HashParameterMapping> {
bool VisitTemplateTypeParmType(TemplateTypeParmType *T) {
// A lambda expression can introduce template parameters that don't have
// corresponding template arguments yet.
- if (T->getDepth() >= TemplateArgs.getNumLevels())
+ if (T->getDepth() >= TemplateArgs.getNumLevels()
+ || !TemplateArgs.hasTemplateArgument(T->getDepth(), T->getIndex()))
return true;
TemplateArgument Arg = TemplateArgs(T->getDepth(), T->getIndex());
diff --git a/clang/test/SemaTemplate/concepts.cpp b/clang/test/SemaTemplate/concepts.cpp
index e5e081ffb9d0f..3671dc6ff84de 100644
--- a/clang/test/SemaTemplate/concepts.cpp
+++ b/clang/test/SemaTemplate/concepts.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -std=c++20 -ferror-limit 0 -verify %s
+// RUN: %clang_cc1 -std=c++20 -ferror-limit 0 -verify=expected,cxx20 %s
+// RUN: %clang_cc1 -std=c++2c -ferror-limit 0 -verify=expected %s
namespace PR47043 {
template<typename T> concept True = true;
@@ -1405,3 +1406,41 @@ static_assert(!std::is_constructible_v<span<4>, array<int, 3>>);
}
}
+
+
+namespace GH162125 {
+template<typename, int size>
+concept true_int = (size, true);
+
+template<typename, typename... Ts>
+concept true_types = true_int<void, sizeof...(Ts)>;
+
+template<typename, typename... Ts>
+concept true_types2 = true_int<void, Ts...[0]{1}>; // cxx20-warning {{pack indexing is a C++2c extension}}
+
+template<typename... Ts>
+struct s {
+ template<typename T> requires true_types<T, Ts...> && true_types2<T, Ts...>
+ static void f(T);
+};
+void(*test)(int) = &s<bool>::f<int>;
+}
+
+namespace GH162125_reversed {
+template<int size, typename>
+concept true_int = (size, true);
+
+template<typename, typename... Ts>
+concept true_types = true_int<sizeof...(Ts), void>;
+
+template<typename, typename... Ts>
+concept true_types2 = true_int<Ts...[0]{1}, void>;
+
+template<typename... Ts>
+struct s {
+ template<typename T> requires true_types<T, Ts...> && true_types2<T, Ts...>
+ static void f(T);
+};
+
+void(*test)(int) = &s<bool>::f<int>;
+}
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
The comment for the changed condition talks about lambdas, but the failing test case doesn't use lambdas. It seems like that comment does not match the new code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
clang/lib/Sema/SemaConcept.cpp
Outdated
if (T->getDepth() >= TemplateArgs.getNumLevels() || | ||
!TemplateArgs.hasTemplateArgument(T->getDepth(), T->getIndex())) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we
if (T->getDepth() >= TemplateArgs.getNumLevels() || | |
!TemplateArgs.hasTemplateArgument(T->getDepth(), T->getIndex())) | |
// There might not be a corresponding template argument before substituting into the parameter mapping, e.g. a lambda expression ... | |
if (!TemplateArgs.hasTemplateArgument(T->getDepth(), T->getIndex())) |
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, they are slightly different scenarios. I split the condition in 2 and added a comment, it's probably clearer that way. Thanks!
clang/test/SemaTemplate/concepts.cpp
Outdated
template<typename, typename... Ts> | ||
concept true_types2 = true_int<Ts...[0]{1}, void>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
template<typename, typename... Ts> | |
concept true_types2 = true_int<Ts...[0]{1}, void>; | |
template<typename, typename... Ts> | |
concept true_types2 = true_int<Ts...[0]{1}, void>; // cxx20-warning {{pack indexing is a C++2c extension}} |
When a template parameter pack is named before
it can be substituted, it might not have a corresponding mapping.
Fixes #162125