llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Qizhi Hu (jcsxky) <details> <summary>Changes</summary> attempt to fix https://github.com/llvm/llvm-project/issues/12361 Consider this example: ```cpp 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); }; 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. --- Full diff: https://github.com/llvm/llvm-project/pull/91430.diff 5 Files Affected: - (modified) clang/include/clang/Sema/Scope.h (+6) - (modified) clang/lib/Parse/ParseDecl.cpp (+5-2) - (modified) clang/lib/Sema/Scope.cpp (+2) - (modified) clang/lib/Sema/SemaAccess.cpp (+7-1) - (added) clang/test/SemaCXX/PR12361.cpp (+30) ``````````diff 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 4e4b05b21383e..78a81c77f48c6 100644 --- a/clang/lib/Parse/ParseDecl.cpp +++ b/clang/lib/Parse/ParseDecl.cpp @@ -4332,9 +4332,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..780aa898b1085 100644 --- a/clang/lib/Sema/Scope.cpp +++ b/clang/lib/Sema/Scope.cpp @@ -229,6 +229,8 @@ void Scope::dumpImpl(raw_ostream &OS) const { {ClassInheritanceScope, "ClassInheritanceScope"}, {CatchScope, "CatchScope"}, {OpenACCComputeConstructScope, "OpenACCComputeConstructScope"}, + {TypeAliasScope, "TypeAliasScope"}, + {FriendScope, "FriendScope"}, }; for (auto Info : FlagInfo) { diff --git a/clang/lib/Sema/SemaAccess.cpp b/clang/lib/Sema/SemaAccess.cpp index 6a707eeb66d01..ded88da510525 100644 --- a/clang/lib/Sema/SemaAccess.cpp +++ b/clang/lib/Sema/SemaAccess.cpp @@ -1476,7 +1476,13 @@ static Sema::AccessResult CheckAccess(Sema &S, SourceLocation Loc, // 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); - if (S.DelayedDiagnostics.shouldDelayDiagnostics()) { + Scope *TS = S.getCurScope(); + bool IsFriendDeclaration = false; + while (TS && !IsFriendDeclaration) { + IsFriendDeclaration = TS->isFriendScope(); + TS = TS->getParent(); + } + if (S.DelayedDiagnostics.shouldDelayDiagnostics() && !IsFriendDeclaration) { S.DelayedDiagnostics.add(DelayedDiagnostic::makeAccess(Loc, Entity)); return Sema::AR_delayed; } 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;} + }; `````````` </details> https://github.com/llvm/llvm-project/pull/91430 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits