[Clang][Sema] access checking of friend declaration should not be delayed

clang/docs/ReleaseNotes.rst

@@ -703,6 +703,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;}
+    };