[Clang][Sema] access checking of friend declaration should not be delayed (llvm#91430)

attempt to fix llvm#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.

Author: jcsxky

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
 ^^^^^^^^^^^^^^^^^^^^^^^^^

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

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;

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) {

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);

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;}
+    };