[Clang] Clean up the fix for deferred access checking

200f3bd introduced a parsing scope to avoid deferring access checking
for friend declarations. That turned out to be insufficient because it
only sets up the scope until we see the friend keyword, which can be
bypassed if the declaration occurs before the keyword.

That said, based on the discussion in #12361, we still want to
preserve the fix since a friend function declaration doesn't grant the
access to other redeclarations.

This patch implemented it by not considering the friend declaration
functions as effective contexts. So we don't end up checking the
canonical function declaration when we're supposed to check only
non-function friend declarations.

Author: zyn0217

clang/lib/Sema/SemaAccess.cpp

@@ -1497,9 +1497,37 @@ void Sema::HandleDelayedAccessCheck(DelayedDiagnostic &DD, Decl *D) {
     DC = D->getLexicalDeclContext();
   } else if (FunctionDecl *FN = dyn_cast<FunctionDecl>(D)) {
     DC = FN;
+    // C++ [class.access.general]p4:
+    //   Access control is applied uniformly to declarations and expressions.
+    // C++ [class.access.general]p6:
+    //   All access controls in [class.access] affect the ability to name a
+    //   class member from the declaration, including parts of the declaration
+    //   preceding the name of the entity being declared [...]
+    //
+    // A friend function declaration might name an entity to which it has access
+    // in the particular context, but it doesn't implicitly grant access
+    // to that entity in other declaration contexts. For example:
+    //
+    //   class A {
+    //     using T = int;
+    //     friend void foo(T);     // #1
+    //   };
+    //   class B {
+    //     friend void foo(A::T);  // #2
+    //   };
+    //
+    // The friend declaration at #1 does not give B access to A::T, nor does it
+    // befriend B. Therefore, the friend declaration should not serve
+    // as an effective context.
+    if (FN->getFriendObjectKind())
+      DC = FN->getLexicalDeclContext();
   } else if (TemplateDecl *TD = dyn_cast<TemplateDecl>(D)) {
-    if (auto *D = dyn_cast_if_present<DeclContext>(TD->getTemplatedDecl()))
+    if (auto *D = dyn_cast_if_present<DeclContext>(TD->getTemplatedDecl())) {
       DC = D;
+      if (FunctionDecl *FN = dyn_cast<FunctionDecl>(DC);
+          FN && FN->getFriendObjectKind())
+        DC = FN->getLexicalDeclContext();
+    }
   } else if (auto *RD = dyn_cast<RequiresExprBodyDecl>(D)) {
     DC = RD;
   }

clang/test/SemaCXX/access-control-check.cpp

@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-unused-variable -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-unused-variable -std=c++20 -verify=expected,since-cxx20 %s
 
 class M {
   int iM;
@@ -59,3 +60,40 @@ struct A {
 int i = f<B>();
 
 }
+
+namespace GH12361 {
+class D1 {
+  class E1 {
+    class F1 {}; // #F1
+
+    friend D1::E1::F1 foo1();
+    friend void foo2(D1::E1::F1);
+    friend void foo3() noexcept(sizeof(D1::E1::F1) == 1);
+    friend void foo4();
+#if __cplusplus >= 202002L
+    template <class T>
+    friend void foo5(T) requires (sizeof(D1::E1::F1) == 1);
+#endif
+  };
+
+  D1::E1::F1 friend foo1();     // expected-error{{'F1' is a private member of 'GH12361::D1::E1'}}
+  // expected-note@#F1 {{implicitly declared private}}
+  friend void foo2(D1::E1::F1); // expected-error{{'F1' is a private member of 'GH12361::D1::E1'}}
+  // expected-note@#F1 {{implicitly declared private}}
+
+  // FIXME: This should be diagnosed. We entered the function DC too early.
+  friend void foo3() noexcept(sizeof(D1::E1::F1) == 1);
+  friend void foo4() {
+    D1::E1::F1 V;
+  }
+#if __cplusplus >= 202002L
+  template <class T>
+  friend void foo5(T)
+    requires (sizeof(D1::E1::F1) == 1); // since-cxx20-error {{'F1' is a private member of 'GH12361::D1::E1'}}
+  // since-cxx20-note@#F1 {{implicitly declared private}}
+#endif
+};
+
+D1::E1::F1 foo1() { return D1::E1::F1(); }
+
+}