[clang] fix access checking about function overloading (PR #107768)

2025-06-10 Thread Erich Keane via cfe-commits 

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

[clang] fix access checking about function overloading (PR #107768)

2025-01-21 Thread Erich Keane via cfe-commits 

https://github.com/erichkeane approved this pull request.

This seems reasonable to me?  And passes tests?  

The `AS_none` has 'different meanings in different contexts, which is confusing 
here, so I have no idea what it is supposed to mean here.  But unless someone 
has a problem with this, I suspect we should just see what the fallout of this 
is...

THOUGH, because I'm not sure, perhaps we wait another ~week for the branch and 
only do this in Clang21+?

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

[clang] fix access checking about function overloading (PR #107768)

2024-12-01 Thread via cfe-commits 

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: Zhikai Zeng (Backl1ght)


Changes

fix https://github.com/llvm/llvm-project/issues/107629

After some more debugging, I find out that we will check access here at 
https://github.com/llvm/llvm-project/blob/8e010ac5a173c9dee44b44324169a3e100a1a6fc/clang/lib/Sema/SemaInit.cpp#L7807

And for `f()` inside code below, `Found.getAccess()` is `AS_none` hence 
`CheckAddressOfMemberAccess` return `AR_accessible` directly.

```cpp
struct Base {
public:
  int f(int);
private:
  int f();  // expect-note {{declared private here}}
};

struct Derived : public Base {};

void f() {
  int(Derived::* public_f)(int) = &Derived::f;
  int(Derived::* private_f)() = &Derived::f;  // expect-error {{'f' is a 
private member of 'Base'}}
}
```

I think the `Found.getAccess()` is intended to be `AS_none` so I just add one 
more access check for the `UnresolvedLookupExpr` when `Found.getAccess()` is 
`AS_none`. If add the check unconditionally clang will report lots of duplicate 
errors and cause several unit tests to fail.

I also test the UB mentioned in 
https://github.com/llvm/llvm-project/issues/107629 and clang now display 4 
`false` as expecetd.

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


3 Files Affected:

- (modified) clang/docs/ReleaseNotes.rst (+1) 
- (modified) clang/lib/Sema/SemaOverload.cpp (+3) 
- (modified) clang/test/CXX/class.access/p4.cpp (+14-2) 


``diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index e44aefa90ab386..0de249ced8309a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -763,6 +763,7 @@ Bug Fixes to C++ Support
 - Fixed a bug where bounds of partially expanded pack indexing expressions 
were checked too early. (#GH116105)
 - Fixed an assertion failure caused by using ``consteval`` in condition in 
consumed analyses. (#GH117385)
 - Fix a crash caused by incorrect argument position in merging deduced 
template arguments. (#GH113659)
+- Fix a bug where private access specifier of overloaded function not 
respected. (#GH107629)
 
 Bug Fixes to AST Handling
 ^
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 4c9e37bd286dee..35389957adf32d 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -16305,6 +16305,9 @@ ExprResult Sema::FixOverloadedFunctionReference(Expr 
*E, DeclAccessPair Found,
   }
 
   if (UnresolvedLookupExpr *ULE = dyn_cast(E)) {
+if (Found.getAccess() == AS_none) {
+  CheckUnresolvedLookupAccess(ULE, Found);
+}
 // FIXME: avoid copy.
 TemplateArgumentListInfo TemplateArgsBuffer, *TemplateArgs = nullptr;
 if (ULE->hasExplicitTemplateArgs()) {
diff --git a/clang/test/CXX/class.access/p4.cpp 
b/clang/test/CXX/class.access/p4.cpp
index ca98c9f90bd89e..6d4c8c004911db 100644
--- a/clang/test/CXX/class.access/p4.cpp
+++ b/clang/test/CXX/class.access/p4.cpp
@@ -21,11 +21,13 @@ namespace test0 {
   public:
 void foo(Public&);
   protected:
-void foo(Protected&); // expected-note 2 {{declared protected here}}
+void foo(Protected&); // expected-note 4 {{declared protected here}}
   private:
-void foo(Private&); // expected-note 2 {{declared private here}}
+void foo(Private&); // expected-note 4 {{declared private here}}
   };
 
+  class B : public A {};
+
   void test(A *op) {
 op->foo(PublicInst);
 op->foo(ProtectedInst); // expected-error {{'foo' is a protected member}}
@@ -35,6 +37,16 @@ namespace test0 {
 void (A::*b)(Protected&) = &A::foo; // expected-error {{'foo' is a 
protected member}}
 void (A::*c)(Private&) = &A::foo; // expected-error {{'foo' is a private 
member}}
   }
+
+  void test(B *op) {
+op->foo(PublicInst);
+op->foo(ProtectedInst); // expected-error {{'foo' is a protected member}}
+op->foo(PrivateInst); // expected-error {{'foo' is a private member}}
+
+void (B::*a)(Public&) = &B::foo;
+void (B::*b)(Protected&) = &B::foo; // expected-error {{'foo' is a 
protected member}}
+void (B::*c)(Private&) = &B::foo; // expected-error {{'foo' is a private 
member}}
+  }
 }
 
 // Member operators.

``




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

[clang] fix access checking about function overloading (PR #107768)

2024-12-01 Thread Zhikai Zeng via cfe-commits 

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

[clang] fix access checking about function overloading (PR #107768)

2024-12-01 Thread Zhikai Zeng via cfe-commits 

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

[clang] fix access checking about function overloading (PR #107768)

2024-12-01 Thread Zhikai Zeng via cfe-commits 

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

[clang] fix access checking about function overloading (PR #107768)

2024-12-01 Thread Zhikai Zeng via cfe-commits 

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

[clang] fix access checking about function overloading (PR #107768)

2024-12-01 Thread Zhikai Zeng via cfe-commits 

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

[clang] fix access checking about function overloading (PR #107768)

2024-12-01 Thread Zhikai Zeng via cfe-commits 

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

[clang] fix access checking about function overloading (PR #107768)

2024-12-01 Thread Zhikai Zeng via cfe-commits 

https://github.com/Backl1ght updated 
https://github.com/llvm/llvm-project/pull/107768

>From 694b7dd640676c83a25e79b1f4ba1cb8e6cbb87b Mon Sep 17 00:00:00 2001
From: Backl1ght 
Date: Sun, 1 Dec 2024 16:45:28 +0800
Subject: [PATCH] fix

---
 clang/docs/ReleaseNotes.rst|  1 +
 clang/lib/Sema/SemaOverload.cpp|  3 +++
 clang/test/CXX/class.access/p4.cpp | 16 ++--
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index e44aefa90ab386..0de249ced8309a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -763,6 +763,7 @@ Bug Fixes to C++ Support
 - Fixed a bug where bounds of partially expanded pack indexing expressions 
were checked too early. (#GH116105)
 - Fixed an assertion failure caused by using ``consteval`` in condition in 
consumed analyses. (#GH117385)
 - Fix a crash caused by incorrect argument position in merging deduced 
template arguments. (#GH113659)
+- Fix a bug where private access specifier of overloaded function not 
respected. (#GH107629)
 
 Bug Fixes to AST Handling
 ^
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 4c9e37bd286dee..35389957adf32d 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -16305,6 +16305,9 @@ ExprResult Sema::FixOverloadedFunctionReference(Expr 
*E, DeclAccessPair Found,
   }
 
   if (UnresolvedLookupExpr *ULE = dyn_cast(E)) {
+if (Found.getAccess() == AS_none) {
+  CheckUnresolvedLookupAccess(ULE, Found);
+}
 // FIXME: avoid copy.
 TemplateArgumentListInfo TemplateArgsBuffer, *TemplateArgs = nullptr;
 if (ULE->hasExplicitTemplateArgs()) {
diff --git a/clang/test/CXX/class.access/p4.cpp 
b/clang/test/CXX/class.access/p4.cpp
index ca98c9f90bd89e..6d4c8c004911db 100644
--- a/clang/test/CXX/class.access/p4.cpp
+++ b/clang/test/CXX/class.access/p4.cpp
@@ -21,11 +21,13 @@ namespace test0 {
   public:
 void foo(Public&);
   protected:
-void foo(Protected&); // expected-note 2 {{declared protected here}}
+void foo(Protected&); // expected-note 4 {{declared protected here}}
   private:
-void foo(Private&); // expected-note 2 {{declared private here}}
+void foo(Private&); // expected-note 4 {{declared private here}}
   };
 
+  class B : public A {};
+
   void test(A *op) {
 op->foo(PublicInst);
 op->foo(ProtectedInst); // expected-error {{'foo' is a protected member}}
@@ -35,6 +37,16 @@ namespace test0 {
 void (A::*b)(Protected&) = &A::foo; // expected-error {{'foo' is a 
protected member}}
 void (A::*c)(Private&) = &A::foo; // expected-error {{'foo' is a private 
member}}
   }
+
+  void test(B *op) {
+op->foo(PublicInst);
+op->foo(ProtectedInst); // expected-error {{'foo' is a protected member}}
+op->foo(PrivateInst); // expected-error {{'foo' is a private member}}
+
+void (B::*a)(Public&) = &B::foo;
+void (B::*b)(Protected&) = &B::foo; // expected-error {{'foo' is a 
protected member}}
+void (B::*c)(Private&) = &B::foo; // expected-error {{'foo' is a private 
member}}
+  }
 }
 
 // Member operators.

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

[clang] fix access checking about function overloading (PR #107768)

2024-10-13 Thread Zhikai Zeng via cfe-commits 

Backl1ght wrote:

After some more debugging, I find out that we will check access here at 
https://github.com/llvm/llvm-project/blob/8e010ac5a173c9dee44b44324169a3e100a1a6fc/clang/lib/Sema/SemaInit.cpp#L7807

And for `f()` inside code below, `Found.getAccess()` is `AS_none` hence 
`CheckAddressOfMemberAccess` return `AR_accessible` directly.

```cpp
struct Base {
public:
  int f(int);
private:
  int f();  // expect-note {{declared private here}}
};

struct Derived : public Base {};

void f() {
  int(Derived::* public_f)(int) = &Derived::f;
  int(Derived::* private_f)() = &Derived::f;  // expect-error {{'f' is a 
private member of 'Base'}}
}
```

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

[clang] fix access checking about function overloading (PR #107768)

2024-10-11 Thread Zhikai Zeng via cfe-commits 

Backl1ght wrote:

Sorry for the late response, I will continue to look into this issue this 
sunday.

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

[clang] fix access checking about function overloading (PR #107768)

2024-09-30 Thread Younan Zhang via cfe-commits 

zyn0217 wrote:

What's the status here?

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

[clang] fix access checking about function overloading (PR #107768)

2024-09-08 Thread Zhikai Zeng via cfe-commits 

https://github.com/Backl1ght created 
https://github.com/llvm/llvm-project/pull/107768

fix https://github.com/llvm/llvm-project/issues/107629

TODO: add UT.
TODO: test UB mentioned in https://github.com/llvm/llvm-project/issues/107629.

>From 65172d8391cef4c0c7e239c9a09b34e061f4963a Mon Sep 17 00:00:00 2001
From: Backl1ght 
Date: Mon, 9 Sep 2024 00:34:31 +0800
Subject: [PATCH] fix

---
 clang/lib/Sema/SemaOverload.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 861b0a91240b3b..fdfa18eff93037 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -16265,6 +16265,7 @@ ExprResult Sema::FixOverloadedFunctionReference(Expr 
*E, DeclAccessPair Found,
   }
 
   if (UnresolvedLookupExpr *ULE = dyn_cast(E)) {
+CheckUnresolvedLookupAccess(ULE, Found);
 // FIXME: avoid copy.
 TemplateArgumentListInfo TemplateArgsBuffer, *TemplateArgs = nullptr;
 if (ULE->hasExplicitTemplateArgs()) {

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