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;} + }; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
