Fix C++ access to Carbon friend members from an unrelated class (#8097)

See access_private_parent_member_in_unrelated_class_with_friend.carbon
for an example of this fix.

The problem with that example, prior to this fix, is that when
`Derived::F` is accessed, clang tries to find `F` via
`CarbonExternalASTSource::FindExternalVisibleDeclsByName`, but
`LookupQualifiedName` doesn't find anything because it (correctly) sees
that `F` is a private member of `Base`, so it's not accessible. What it
doesn't know is that it will be accessed from the context of `A`, which
is a friend of `Base`, and so privacy doesn't matter.

`FindExternalVisibleDeclsByName` is really supposed to report visible
items, regardless of whether they can be accessed. But changing it to
simply ignore access (e.g. by modifying `LookupQualifiedName` to
optionally ignore access) doesn't quite work here. This can make a
private member of a base class appear within the `DeclContext` of a
derived class with access set to private, when it should be set to none.
This causes an assert in clang (this case is tested in
fail_access_private_parent_member_in_unrelated_fn.carbon).

Instead, change `FindExternalVisibleDeclsByName` to call
`LookupNameInExactScope` instead of `LookupQualifiedName`. This prevents
extended scopes from being searched, so members of a base class are not
found when filling in the `DeclContext` of a derived class. Clang will
call `FindExternalVisibleDeclsByName` in the context of the base class,
and the base class members are added at that time.

toolchain/check/cpp/generate_ast.cpp

@@ -369,11 +369,24 @@ auto CarbonExternalASTSource::FindExternalVisibleDeclsByName(
 
   auto name_id = AddIdentifierName(*context_, identifier->getName());
 
-  // `required=false` so Carbon doesn't diagnose a failure, let Clang diagnose
-  // it or even SFINAE.
-  LookupResult result = LookupQualifiedName(*context_, GetCurrentCppLocId(),
-                                            name_id, lookup_scopes,
-                                            /*required=*/false);
+  LookupResult result{
+      .specific_id = SemIR::SpecificId::None,
+      .scope_result = SemIR::ScopeLookupResult::MakeNotFound(),
+  };
+  for (auto lookup_scope : lookup_scopes) {
+    auto scope_id = lookup_scope.name_scope_id;
+    result.scope_result =
+        LookupNameInExactScope(*context_, GetCurrentCppLocId(), name_id,
+                               scope_id, context_->name_scopes().Get(scope_id));
+    result.specific_id = lookup_scope.specific_id;
+    // TODO: If the name is found in multiple scopes, it should be treated as
+    // ambiguous, possibly by adding all the results to the name lookup table
+    // so Clang reports the ambiguity.
+    if (result.scope_result.is_found()) {
+      break;
+    }
+  }
+
   if (!result.scope_result.is_found()) {
     // Avoid caching a negative lookup, as a declaration with this name may be
     // added in Carbon later.

toolchain/check/testdata/class/access/inheritance_access.carbon

@@ -332,27 +332,7 @@ struct B : private A {};
 
 template<typename T>
 auto F(T* _Nonnull t) -> void {
-    t->X();
-}
-''';
-
-class C(T: type) {
-  extend base: Cpp.B;
-};
-
-fn G() {
-    var d: C(i32) = {
-        .base = Cpp.B.B(),
-    };
-    // CHECK:STDERR:        ╭─┤ fail_access_private_parent_in_generic.carbon:[[@LINE+29]]:13
-    // CHECK:STDERR: ->     │     Cpp.F(&d);
-    // CHECK:STDERR:        ·             ┳
-    // CHECK:STDERR:        ·             ┗━━┫ error: cannot access private member `X` of type `Cpp.B`
-    // CHECK:STDERR:        ·             │  ┃        [ClassInvalidMemberAccess]
-    // CHECK:STDERR:        ·             ╰────┤ declared here [ClassMemberDeclaration]
-    // CHECK:STDERR:        ╰────
-    // CHECK:STDERR:
-    // CHECK:STDERR:        ╭─┤ fail_access_private_parent_in_generic.carbon:[[@LINE-20]]:8
+    // CHECK:STDERR:        ╭─┤ fail_access_private_parent_in_generic.carbon:[[@LINE+21]]:8
     // CHECK:STDERR:        │   bool X() const;
     // CHECK:STDERR:        ·        ┬
     // CHECK:STDERR:        ·        ╰──┤ member is declared here [CppInteropParseNote]
@@ -373,5 +353,17 @@ fn G() {
     // CHECK:STDERR:        ·                │ [CppInteropParseNote]
     // CHECK:STDERR:        ╰────
     // CHECK:STDERR:
+    t->X();
+}
+''';
+
+class C(T: type) {
+  extend base: Cpp.B;
+};
+
+fn G() {
+    var d: C(i32) = {
+        .base = Cpp.B.B(),
+    };
     Cpp.F(&d);
 }

toolchain/check/testdata/interop/cpp/class/import/access.carbon

@@ -2087,6 +2087,114 @@ class B {
   }
 }
 
+// --- fail_access_private_parent_member_in_unrelated_class.carbon
+library "[[@TEST_NAME]]";
+import Cpp;
+
+class Derived;
+
+inline Cpp '''
+class Base {
+private:
+  static void F() {}
+};
+
+template <class T>
+struct A {
+  static void G() {
+    // CHECK:STDERR:        ╭─┤ fail_access_private_parent_member_in_unrelated_class.carbon:[[@LINE+16]]:8
+    // CHECK:STDERR:        │   static void F() {}
+    // CHECK:STDERR:        ·               ┬
+    // CHECK:STDERR:        ·               ╰──┤ declared private here [CppInteropParseNote]
+    // CHECK:STDERR:        ┆
+    // CHECK:STDERR: ->     │     T::F();
+    // CHECK:STDERR:        ·        ┳
+    // CHECK:STDERR:        ·        ┗━━┫ error: 'F' is a private member of 'Base' [CppInteropParseError]
+    // CHECK:STDERR:        ┆
+    // CHECK:STDERR:        │   a.G();
+    // CHECK:STDERR:        ·     ┬
+    // CHECK:STDERR:        ·     ╰──┤ in instantiation of member function 'A<Carbon::Main::Derived>::G'
+    // CHECK:STDERR:        ·        │ requested here
+    // CHECK:STDERR:        ·        │ [CppInteropParseNote]
+    // CHECK:STDERR:        ╰────
+    // CHECK:STDERR:
+    T::F();
+  }
+};
+
+void H() {
+  A<Carbon::Derived> a;
+  a.G();
+}
+''';
+
+class Derived {
+  extend base: Cpp.Base;
+};
+
+// --- access_private_parent_member_in_unrelated_class_with_friend.carbon
+library "[[@TEST_NAME]]";
+import Cpp;
+
+class Derived;
+
+inline Cpp '''
+class Base {
+private:
+  // This test is identical to
+  // fail_access_private_parent_member_in_unrelated_class except for this
+  // friend decl:
+  template <typename> friend class A;
+
+  static void F() {}
+};
+
+template <class T>
+struct A {
+  static void G() {
+    T::F();
+  }
+};
+
+void H() {
+  A<Carbon::Derived> a;
+  a.G();
+}
+''';
+
+class Derived {
+  extend base: Cpp.Base;
+};
+
+// --- fail_access_private_parent_member_in_unrelated_fn.carbon
+library "[[@TEST_NAME]]";
+import Cpp;
+
+base class A {
+  private fn F();
+}
+
+class B {
+  extend base: A;
+}
+
+inline Cpp '''
+int G() {
+  // CHECK:STDERR:        ╭─┤ fail_access_private_parent_member_in_unrelated_fn.carbon:[[@LINE+11]]:14
+  // CHECK:STDERR:        │   private fn F();
+  // CHECK:STDERR:        ·                 ┬
+  // CHECK:STDERR:        ·                 ╰──┤ implicitly declared private here [CppInteropParseNote]
+  // CHECK:STDERR:        ┆
+  // CHECK:STDERR: ->     │   Carbon::B::F();
+  // CHECK:STDERR:        ·              ┳
+  // CHECK:STDERR:        ·              ┗━━┫ error: 'F' is a private member of 'Carbon::Main::A'
+  // CHECK:STDERR:        ·                 ┃        [CppInteropParseError]
+  // CHECK:STDERR:        ╰────
+  // CHECK:STDERR:
+  Carbon::B::F();
+}
+''';
+
 // CHECK:STDOUT: --- import_non_function_member_public.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {