[Clang][Sema] fix a bug on constraint check with template friend function #90646
Conversation
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Qizhi Hu (jcsxky) Changesattempt to fix #90349 Full diff: https://github.com/llvm/llvm-project/pull/90646.diff 3 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c11f117cd6e8b4..ec10c82a3a5403 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -627,6 +627,7 @@ Bug Fixes to C++ Support
- Fix a bug on template partial specialization with issue on deduction of nontype template parameter
whose type is `decltype(auto)`. Fixes (#GH68885).
- Clang now correctly treats the noexcept-specifier of a friend function to be a complete-class context.
+- Fix a bug on constraint check with template friend function. Fixes (#GH90349).
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 3a9fd906b7af86..1805f8f6e5ad90 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -281,6 +281,20 @@ Response HandleFunction(Sema &SemaRef, const FunctionDecl *Function,
if (Function->getPrimaryTemplate()->isMemberSpecialization())
return Response::Done();
+ if (Function->getFriendObjectKind())
+ if (const ClassTemplateSpecializationDecl *TD =
+ dyn_cast<ClassTemplateSpecializationDecl>(
+ Function->getLexicalDeclContext())) {
+ const CXXRecordDecl *TemplatePattern =
+ TD->getTemplateInstantiationPattern();
+ const FunctionDecl *FunctionPattern =
+ Function->getTemplateInstantiationPattern();
+ if (TemplatePattern && FunctionPattern &&
+ TemplatePattern->getTemplateDepth() ==
+ FunctionPattern->getTemplateDepth())
+ return Response::Done();
+ }
+
// If this function is a generic lambda specialization, we are done.
if (!ForConstraintInstantiation &&
isGenericLambdaCallOperatorOrStaticInvokerSpecialization(Function)) {
diff --git a/clang/test/SemaCXX/PR90349.cpp b/clang/test/SemaCXX/PR90349.cpp
new file mode 100644
index 00000000000000..6a4b5c21e88f3b
--- /dev/null
+++ b/clang/test/SemaCXX/PR90349.cpp
@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 -verify -std=c++20 -fsyntax-only %s
+
+// expected-no-diagnostics
+
+namespace std {
+template<class T>
+concept floating_point = __is_same(T,double) || __is_same(T,float);
+
+template<class T>
+concept integral = __is_same(T,int);
+
+}
+
+template<std::integral T, std::floating_point Float>
+class Blob;
+
+template<std::floating_point Float, std::integral T>
+Blob<T, Float> MakeBlob();
+
+template<std::integral T, std::floating_point Float>
+class Blob {
+private:
+ Blob() {}
+
+ friend Blob<T, Float> MakeBlob<Float, T>();
+};
+
+template<std::floating_point Float, std::integral T>
+Blob<T, Float> MakeBlob()
+{
+ return Blob<T, Float>();
+}
+
+template<std::floating_point Float, std::integral T>
+Blob<T, Float> FindBlobs()
+{
+ return MakeBlob<Float, T>();
+}
+
+int main(int argc, const char * argv[]) {
+ FindBlobs<double, int>();
+ return 0;
+}
|
|
I'm actually working on constraint checking for function template specializations in #88963. I don't think this patch is quite right... this will cause a crash if the befriended function is a member of a class template specialization. Relative to the changes in #88963, I believe the correct fix would be to change line 278 to: if (RelativeToPrimary &&
(Function->getTemplateSpecializationKind() ==
TSK_ExplicitSpecialization ||
(Function->getFriendObjectKind() &&
!Function->getPrimaryTemplate()->getFriendObjectKind())))
return Response::UseNextDecl(Function);I added a commit to #88963 which makes this change (be79079) cc @erichkeane |
Only when the friend function doesn't use any other new template parameters, i.e. their depth is equal can we skip to add the outer arguments to
Could you please provide a testcase? |
|
Windows CI failed with some unrelated files. |
There was a problem hiding this comment.
This seems reasonable, @mizvekov is doing more work in this area, so I'd like him to make sure this isn't something he's fixed elsewhere.
template<typename T, typename U>
concept D = sizeof(T) == sizeof(U);
template<typename T>
struct A
{
template<typename U, typename V> requires D<U, V>
static void f();
};
template<typename T, typename U>
struct B
{
template<typename V>
struct C
{
friend void A<char>::f<T, U>();
};
};
template struct B<int, int>::C<short>;
extern template void A<char>::f<int, int>(); // crash here@jcsxky causes crash with and without this patch applied. |
Thanks for your feedback! This may be another issue. clang trunk crashes with this case. |
|
I agree with @sdkrystian, even though the test crashes for maybe yet another reason, it demonstrates you can friend a function from a different template context, so comparing the depths from different branches is not helpful. |
|
FWIW, the changes in be79079 fix the crash |
@mizvekov I missed the case which friend function is declared in an inner class, and 3d27f11 demonstrates the comparison works fine. |
|
This still doesn't handle cases such as: template<typename T, typename U, typename V, typename W>
concept E = sizeof(T) == sizeof(U) && sizeof(V) == sizeof(W);
template<typename T> // T = char
struct A
{
template<typename U> // U = signed char
struct B
{
template<typename V, typename W, typename X> // V = int, W = int, X = short
requires E<T, U, V, W> // incorrectly substitutes T = int, U = short, V = int, V = int
static void f();
};
};
template<typename T, typename U> // T = int, U = int
struct C
{
template<typename V> // V = short
struct D
{
friend void A<char>::B<signed char>::f<T, U, V>();
};
};
template struct C<int, int>::D<short>;
extern template void A<char>::B<signed char>::f<int, int, short>();(which is handled by be79079) |
Thanks for your patience and testcases! They really make me understand these related issues more clearly. |
attempt to fix #90349
Skip to add outer class template arguments to
MTALwhen the friend function has the same depth with its lexical context(CXXRecordDecl).