[clang] 200f3bd - [Clang][Sema] access checking of friend declaration should not be delayed (#91430)
via cfe-commits
cfe-commits at lists.llvm.org
Fri May 10 05:14:11 PDT 2024
Author: Qizhi Hu
Date: 2024-05-10T20:14:08+08:00
New Revision: 200f3bd39562f4d605f13567398025d30fa27d61
URL: https://github.com/llvm/llvm-project/commit/200f3bd39562f4d605f13567398025d30fa27d61
DIFF: https://github.com/llvm/llvm-project/commit/200f3bd39562f4d605f13567398025d30fa27d61.diff
LOG: [Clang][Sema] access checking of friend declaration should not be delayed (#91430)
attempt to fix https://github.com/llvm/llvm-project/issues/12361
Consider this example:
```cpp
class D {
class E{
class F{};
friend void foo(D::E::F& q);
};
friend void foo(D::E::F& q);
};
void foo(D::E::F& q) {}
```
The first friend declaration of foo is correct. After that, the second
friend declaration delayed access checking and set its previous
declaration to be the first one. When doing access checking of `F`(which
is private filed of `E`), we put its canonical declaration(the first
friend declaration) into `EffectiveContext.Functions`. Actually, we are
still checking the first one. This is incorrect due to the delayed
checking.
Creating a new scope to indicate we are parsing a friend declaration and
doing access checking in time.
Added:
clang/test/SemaCXX/PR12361.cpp
Modified:
clang/docs/ReleaseNotes.rst
clang/include/clang/Sema/Scope.h
clang/lib/Parse/ParseDecl.cpp
clang/lib/Sema/Scope.cpp
clang/lib/Sema/SemaAccess.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index eef627ff2e31f..7c5dcc59c7016 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -706,6 +706,7 @@ Bug Fixes to C++ Support
within initializers for variables that are usable in constant expressions or are constant
initialized, rather than evaluating them as a part of the larger manifestly constant evaluated
expression.
+- Fix a bug in access control checking due to dealyed checking of friend declaration. Fixes (#GH12361).
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Sema/Scope.h b/clang/include/clang/Sema/Scope.h
index 1752a25111a77..084db73034219 100644
--- a/clang/include/clang/Sema/Scope.h
+++ b/clang/include/clang/Sema/Scope.h
@@ -159,6 +159,9 @@ class Scope {
/// This is a scope of type alias declaration.
TypeAliasScope = 0x20000000,
+
+ /// This is a scope of friend declaration.
+ FriendScope = 0x40000000,
};
private:
@@ -586,6 +589,9 @@ class Scope {
/// Determine whether this scope is a type alias scope.
bool isTypeAliasScope() const { return getFlags() & Scope::TypeAliasScope; }
+ /// Determine whether this scope is a friend scope.
+ bool isFriendScope() const { return getFlags() & Scope::FriendScope; }
+
/// Returns if rhs has a higher scope depth than this.
///
/// The caller is responsible for calling this only if one of the two scopes
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 2c11ae693c354..5b5fc02ad4023 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -4331,9 +4331,12 @@ void Parser::ParseDeclarationSpecifiers(
// friend
case tok::kw_friend:
- if (DSContext == DeclSpecContext::DSC_class)
+ if (DSContext == DeclSpecContext::DSC_class) {
isInvalid = DS.SetFriendSpec(Loc, PrevSpec, DiagID);
- else {
+ Scope *CurS = getCurScope();
+ if (!isInvalid && CurS)
+ CurS->setFlags(CurS->getFlags() | Scope::FriendScope);
+ } else {
PrevSpec = ""; // not actually used by the diagnostic
DiagID = diag::err_friend_invalid_in_context;
isInvalid = true;
diff --git a/clang/lib/Sema/Scope.cpp b/clang/lib/Sema/Scope.cpp
index 11a41753a1bda..c08073e80ff3d 100644
--- a/clang/lib/Sema/Scope.cpp
+++ b/clang/lib/Sema/Scope.cpp
@@ -229,6 +229,7 @@ void Scope::dumpImpl(raw_ostream &OS) const {
{ClassInheritanceScope, "ClassInheritanceScope"},
{CatchScope, "CatchScope"},
{OpenACCComputeConstructScope, "OpenACCComputeConstructScope"},
+ {FriendScope, "FriendScope"},
};
for (auto Info : FlagInfo) {
diff --git a/clang/lib/Sema/SemaAccess.cpp b/clang/lib/Sema/SemaAccess.cpp
index 6a707eeb66d01..979a64b065f3d 100644
--- a/clang/lib/Sema/SemaAccess.cpp
+++ b/clang/lib/Sema/SemaAccess.cpp
@@ -1473,12 +1473,32 @@ static Sema::AccessResult CheckAccess(Sema &S, SourceLocation Loc,
// specifier, like this:
// A::private_type A::foo() { ... }
//
- // Or we might be parsing something that will turn out to be a friend:
- // void foo(A::private_type);
- // void B::foo(A::private_type);
+ // friend declaration should not be delayed because it may lead to incorrect
+ // redeclaration chain, such as:
+ // class D {
+ // class E{
+ // class F{};
+ // friend void foo(D::E::F& q);
+ // };
+ // friend void foo(D::E::F& q);
+ // };
if (S.DelayedDiagnostics.shouldDelayDiagnostics()) {
- S.DelayedDiagnostics.add(DelayedDiagnostic::makeAccess(Loc, Entity));
- return Sema::AR_delayed;
+ // [class.friend]p9:
+ // A member nominated by a friend declaration shall be accessible in the
+ // class containing the friend declaration. The meaning of the friend
+ // declaration is the same whether the friend declaration appears in the
+ // private, protected, or public ([class.mem]) portion of the class
+ // member-specification.
+ Scope *TS = S.getCurScope();
+ bool IsFriendDeclaration = false;
+ while (TS && !IsFriendDeclaration) {
+ IsFriendDeclaration = TS->isFriendScope();
+ TS = TS->getParent();
+ }
+ if (!IsFriendDeclaration) {
+ S.DelayedDiagnostics.add(DelayedDiagnostic::makeAccess(Loc, Entity));
+ return Sema::AR_delayed;
+ }
}
EffectiveContext EC(S.CurContext);
diff --git a/clang/test/SemaCXX/PR12361.cpp b/clang/test/SemaCXX/PR12361.cpp
new file mode 100644
index 0000000000000..95ceb45b7ba04
--- /dev/null
+++ b/clang/test/SemaCXX/PR12361.cpp
@@ -0,0 +1,30 @@
+ // RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 %s
+ // RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 %s
+
+class D {
+ class E{
+ class F{}; // expected-note{{implicitly declared private here}}
+ friend void foo(D::E::F& q);
+ };
+ friend void foo(D::E::F& q); // expected-error{{'F' is a private member of 'D::E'}}
+ };
+
+void foo(D::E::F& q) {}
+
+class D1 {
+ class E1{
+ class F1{}; // expected-note{{implicitly declared private here}}
+ friend D1::E1::F1 foo1();
+ };
+ friend D1::E1::F1 foo1(); // expected-error{{'F1' is a private member of 'D1::E1'}}
+ };
+
+D1::E1::F1 foo1() { return D1::E1::F1(); }
+
+class D2 {
+ class E2{
+ class F2{};
+ friend void foo2();
+ };
+ friend void foo2(){ D2::E2::F2 c;}
+ };
More information about the cfe-commits
mailing list