[Clang][Sema] fix a bug on constraint check with template friend function

[jcsxky]

attempt to fix #90349
Skip to add outer class template arguments to MTAL when the friend function has the same depth with its lexical context(CXXRecordDecl).

[llvmbot]

@llvm/pr-subscribers-clang

Author: Qizhi Hu (jcsxky)

Changes

attempt to fix #90349
Skip to add outer class template arguments to MTAL when the friend function has the same depth with its lexical context(CXXRecordDecl).

---

Full diff: https://github.com/llvm/llvm-project/pull/90646.diff

3 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+1)
• (modified) clang/lib/Sema/SemaTemplateInstantiate.cpp (+14)
• (added) clang/test/SemaCXX/PR90349.cpp (+43)

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; +} 

[sdkrystian]

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

[jcsxky]

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 MTAL.

this will cause a crash if the befriended function is a member of a class template specialization

Could you please provide a testcase?

[jcsxky]

Windows CI failed with some unrelated files.

[erichkeane]

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.

[sdkrystian]

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.

[jcsxky]

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.

[mizvekov]

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.

[sdkrystian]

FWIW, the changes in be79079 fix the crash

[jcsxky]

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.

@mizvekov I missed the case which friend function is declared in an inner class, and 3d27f11 demonstrates the comparison works fine.

[sdkrystian]

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)

[jcsxky]

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.