[clang] [Clang][Sema] fix a bug on constraint check with template friend function (PR #90646)

2024-05-02 Thread Qizhi Hu via cfe-commits 

jcsxky wrote:

> This still doesn't handle cases such as:
> 
> ```c++
> template
> concept E = sizeof(T) == sizeof(U) && sizeof(V) == sizeof(W);
> 
> template // T = char
> struct A
> {
> template // U = signed char
> struct B
> {
> template // V = int, W = int, X = 
> short
> requires E // incorrectly substitutes T = int, U = short, 
> V = int, V = int
> static void f();
> };
> };
> 
> 
> template // T = int, U = int
> struct C
> {
> template // V = short
> struct D
> {
> friend void A::B::f();
> };
> };
> 
> template struct C::D;
> 
> extern template void A::B::f();
> ```
> 
> (which is handled by 
> [be79079](https://github.com/llvm/llvm-project/commit/be79079507ffbd9b29683498f405dc2c32dd8ba7))

Thanks for your patience and testcases! They really make me understand these 
related issues more clearly.

https://github.com/llvm/llvm-project/pull/90646
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] fix a bug on constraint check with template friend function (PR #90646)

2024-05-02 Thread Krystian Stasiowski via cfe-commits 

sdkrystian wrote:

This still doesn't handle cases such as:
```cpp
template
concept E = sizeof(T) == sizeof(U) && sizeof(V) == sizeof(W);

template // T = char
struct A
{
template // U = signed char
struct B
{
template // V = int, W = int, X = 
short
requires E // incorrectly substitutes T = int, U = short, V 
= int, V = int
static void f();
};
};


template // T = int, U = int
struct C
{
template // V = short
struct D
{
friend void A::B::f();
};
};

template struct C::D;

extern template void A::B::f();
```
(which is handled by be79079507ffbd9b29683498f405dc2c32dd8ba7)

https://github.com/llvm/llvm-project/pull/90646
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] fix a bug on constraint check with template friend function (PR #90646)

2024-05-02 Thread Qizhi Hu via cfe-commits 

jcsxky wrote:

> 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](https://github.com/llvm/llvm-project/pull/90646/commits/3d27f11d79b00608bf9135bcc4b4cb859027933a)
 demonstrates the comparison works fine.

https://github.com/llvm/llvm-project/pull/90646
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] fix a bug on constraint check with template friend function (PR #90646)

2024-05-02 Thread Qizhi Hu via cfe-commits 

https://github.com/jcsxky updated 
https://github.com/llvm/llvm-project/pull/90646

>From 50aa5b64f6d2cf98706bfb4792f274bd071e3b9c Mon Sep 17 00:00:00 2001
From: Qizhi Hu <836744...@qq.com>
Date: Wed, 1 May 2024 02:25:04 +0800
Subject: [PATCH 1/2] [Clang][Sema] fix a bug on constraint check with template
 friend function

---
 clang/docs/ReleaseNotes.rst|  1 +
 clang/lib/Sema/SemaTemplateInstantiate.cpp | 14 +++
 clang/test/SemaCXX/PR90349.cpp | 43 ++
 3 files changed, 58 insertions(+)
 create mode 100644 clang/test/SemaCXX/PR90349.cpp

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 2c5308fbcb319a..ac90e3933798cb 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -630,6 +630,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 , const FunctionDecl 
*Function,
 if (Function->getPrimaryTemplate()->isMemberSpecialization())
   return Response::Done();
 
+if (Function->getFriendObjectKind())
+  if (const ClassTemplateSpecializationDecl *TD =
+  dyn_cast(
+  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 00..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
+concept floating_point = __is_same(T,double) || __is_same(T,float);
+
+template
+concept integral = __is_same(T,int);
+
+}
+
+template
+class Blob;
+
+template
+Blob MakeBlob();
+
+template
+class Blob {
+private:
+Blob() {}
+
+friend Blob MakeBlob();
+};
+
+template
+Blob MakeBlob()
+{
+return Blob();
+}
+
+template
+Blob FindBlobs()
+{
+return MakeBlob();
+}
+
+int main(int argc, const char * argv[]) {
+FindBlobs();
+return 0;
+}

>From 3d27f11d79b00608bf9135bcc4b4cb859027933a Mon Sep 17 00:00:00 2001
From: Qizhi Hu <836744...@qq.com>
Date: Thu, 2 May 2024 20:05:10 +0800
Subject: [PATCH 2/2] continue to handle context instead of finish

---
 clang/lib/Sema/SemaTemplateInstantiate.cpp |  2 +-
 clang/test/SemaCXX/PR90349.cpp | 24 ++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp 
b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 1805f8f6e5ad90..22789c72b2c90f 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -292,7 +292,7 @@ Response HandleFunction(Sema , const FunctionDecl 
*Function,
 if (TemplatePattern && FunctionPattern &&
 TemplatePattern->getTemplateDepth() ==
 FunctionPattern->getTemplateDepth())
-  return Response::Done();
+  return Response::UseNextDecl(Function);
   }
 
 // If this function is a generic lambda specialization, we are done.
diff --git a/clang/test/SemaCXX/PR90349.cpp b/clang/test/SemaCXX/PR90349.cpp
index 6a4b5c21e88f3b..570a49fd2073bb 100644
--- a/clang/test/SemaCXX/PR90349.cpp
+++ b/clang/test/SemaCXX/PR90349.cpp
@@ -41,3 +41,27 @@ int main(int argc, const char * argv[]) {
 FindBlobs();
 return 0;
 }
+
+template
+concept D = sizeof(T) == sizeof(U);
+
+template
+struct A
+{
+template requires D
+static void f();
+};
+
+template
+struct B
+{
+template
+struct C
+{
+friend void A::f();
+};
+};
+
+template struct B::C;
+
+extern template void A::f(); // crash here

___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[clang] [Clang][Sema] fix a bug on constraint check with template friend function (PR #90646)

2024-05-01 Thread Krystian Stasiowski via cfe-commits 

sdkrystian wrote:

FWIW, the changes in be79079507ffbd9b29683498f405dc2c32dd8ba7 fix the crash

https://github.com/llvm/llvm-project/pull/90646
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] fix a bug on constraint check with template friend function (PR #90646)

2024-05-01 Thread Matheus Izvekov via cfe-commits 

mizvekov wrote:

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.

https://github.com/llvm/llvm-project/pull/90646
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] fix a bug on constraint check with template friend function (PR #90646)

2024-05-01 Thread Qizhi Hu via cfe-commits 

jcsxky wrote:

> ```c++
> template
> concept D = sizeof(T) == sizeof(U);
> 
> template
> struct A
> {
> template requires D
> static void f();
> };
> 
> template
> struct B
> {
> template
> struct C
> {
> friend void A::f();
> };
> };
> 
> template struct B::C;
> 
> extern template void A::f(); // 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. 

https://github.com/llvm/llvm-project/pull/90646
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] fix a bug on constraint check with template friend function (PR #90646)

2024-05-01 Thread Krystian Stasiowski via cfe-commits 

sdkrystian wrote:

```cpp
template
concept D = sizeof(T) == sizeof(U);

template
struct A
{
template requires D
static void f();
};

template
struct B
{
template
struct C
{
friend void A::f();
};
};

template struct B::C;

extern template void A::f(); // crash here
```


https://github.com/llvm/llvm-project/pull/90646
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] fix a bug on constraint check with template friend function (PR #90646)

2024-05-01 Thread Erich Keane via cfe-commits 

https://github.com/erichkeane commented:

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.

https://github.com/llvm/llvm-project/pull/90646
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] fix a bug on constraint check with template friend function (PR #90646)

2024-05-01 Thread Qizhi Hu via cfe-commits 

jcsxky wrote:

Windows CI failed with some unrelated files.

https://github.com/llvm/llvm-project/pull/90646
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] fix a bug on constraint check with template friend function (PR #90646)

2024-04-30 Thread Qizhi Hu via cfe-commits 

jcsxky wrote:

> 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:
> 
> ```c++
> 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](https://github.com/llvm/llvm-project/commit/be79079507ffbd9b29683498f405dc2c32dd8ba7))
> 
> 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?

https://github.com/llvm/llvm-project/pull/90646
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] fix a bug on constraint check with template friend function (PR #90646)

2024-04-30 Thread Krystian Stasiowski via cfe-commits 

sdkrystian wrote:

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:
```cpp
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 
(be79079507ffbd9b29683498f405dc2c32dd8ba7)

https://github.com/llvm/llvm-project/pull/90646
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] fix a bug on constraint check with template friend function (PR #90646)

2024-04-30 Thread via cfe-commits 

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: Qizhi Hu (jcsxky)


Changes

attempt to fix https://github.com/llvm/llvm-project/issues/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
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 , const FunctionDecl 
*Function,
 if (Function->getPrimaryTemplate()->isMemberSpecialization())
   return Response::Done();
 
+if (Function->getFriendObjectKind())
+  if (const ClassTemplateSpecializationDecl *TD =
+  dyn_cast(
+  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 00..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
+concept floating_point = __is_same(T,double) || __is_same(T,float);
+
+template
+concept integral = __is_same(T,int);
+
+}
+
+template
+class Blob;
+
+template
+Blob MakeBlob();
+
+template
+class Blob {
+private:
+Blob() {}
+
+friend Blob MakeBlob();
+};
+
+template
+Blob MakeBlob()
+{
+return Blob();
+}
+
+template
+Blob FindBlobs()
+{
+return MakeBlob();
+}
+
+int main(int argc, const char * argv[]) {
+FindBlobs();
+return 0;
+}

``




https://github.com/llvm/llvm-project/pull/90646
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] fix a bug on constraint check with template friend function (PR #90646)

2024-04-30 Thread Qizhi Hu via cfe-commits 

https://github.com/jcsxky created 
https://github.com/llvm/llvm-project/pull/90646

attempt to fix https://github.com/llvm/llvm-project/issues/90349
Skip to add outer class template arguments to `MTAL` when the friend function 
has the same depth with its lexical context(`CXXRecordDecl`).

>From e2785999ed38cd3b0addcf28d9122ecf8100d908 Mon Sep 17 00:00:00 2001
From: Qizhi Hu <836744...@qq.com>
Date: Wed, 1 May 2024 02:25:04 +0800
Subject: [PATCH] [Clang][Sema] fix a bug on constraint check with template
 friend function

---
 clang/docs/ReleaseNotes.rst|  1 +
 clang/lib/Sema/SemaTemplateInstantiate.cpp | 14 +++
 clang/test/SemaCXX/PR90349.cpp | 43 ++
 3 files changed, 58 insertions(+)
 create mode 100644 clang/test/SemaCXX/PR90349.cpp

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 , const FunctionDecl 
*Function,
 if (Function->getPrimaryTemplate()->isMemberSpecialization())
   return Response::Done();
 
+if (Function->getFriendObjectKind())
+  if (const ClassTemplateSpecializationDecl *TD =
+  dyn_cast(
+  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 00..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
+concept floating_point = __is_same(T,double) || __is_same(T,float);
+
+template
+concept integral = __is_same(T,int);
+
+}
+
+template
+class Blob;
+
+template
+Blob MakeBlob();
+
+template
+class Blob {
+private:
+Blob() {}
+
+friend Blob MakeBlob();
+};
+
+template
+Blob MakeBlob()
+{
+return Blob();
+}
+
+template
+Blob FindBlobs()
+{
+return MakeBlob();
+}
+
+int main(int argc, const char * argv[]) {
+FindBlobs();
+return 0;
+}

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits