Просмотр исходного кода

Add support for importing access from C++ to Carbon (#5858)

Better access control with inheritance should come with better
inheritance support (actually importing inheritance).

C++ Interop Demo:

```c++
// hello_world.h

class HelloWorld {
 public:
  static auto Pub() -> void;

 protected:
  static auto Pro() -> void;

 private:
  static auto Pri() -> void;
};
```

```c++
// hello_world.cpp

#include "hello_world.h"

#include <cstdio>

auto HelloWorld::Pub() -> void { printf("Public!\n"); }
auto HelloWorld::Pro() -> void { printf("Protected!\n"); }
auto HelloWorld::Pri() -> void { printf("Private!\n"); }
```

```carbon
// main.carbon

library "Main";

import Cpp library "hello_world.h";

fn Run() -> i32 {
  Cpp.HelloWorld.Pub();
  Cpp.HelloWorld.Pro();
  Cpp.HelloWorld.Pri();
  return 0;
}
```

```shell
$ clang -c hello_world.cpp
$ bazel-bin/toolchain/carbon compile main.carbon
main.carbon:9:3: error: cannot access protected member `Pro` of type `Cpp.HelloWorld`
  Cpp.HelloWorld.Pro();
  ^~~~~~~~~~~~~~~~~~
main.carbon: note: declared here

main.carbon:10:3: error: cannot access private member `Pri` of type `Cpp.HelloWorld`
  Cpp.HelloWorld.Pri();
  ^~~~~~~~~~~~~~~~~~
main.carbon: note: declared here
```

Before this change (no access checks):
```shell
$ bazel-bin/toolchain/carbon compile main.carbon
$ bazel-bin/toolchain/carbon link hello_world.o main.o --output=demo
$ ./demo
Public!
Protected!
Private!
```

Part of #5859.
Boaz Brickner 9 месяцев назад
Родитель
Сommit
30f0ddab71

+ 33 - 17
toolchain/check/import_cpp.cpp

@@ -74,14 +74,15 @@ static auto GenerateCppIncludesHeaderCode(
   return code;
 }
 
-// Adds the name to the scope with the given `inst_id`, if the `inst_id` is not
-// `None`.
+// Adds the name to the scope with the given `access_kind` and `inst_id`.
+// `inst_id` must have a value.
 static auto AddNameToScope(Context& context, SemIR::NameScopeId scope_id,
-                           SemIR::NameId name_id, SemIR::InstId inst_id)
-    -> void {
-  if (inst_id.has_value()) {
-    context.name_scopes().AddRequiredName(scope_id, name_id, inst_id);
-  }
+                           SemIR::NameId name_id, SemIR::AccessKind access_kind,
+                           SemIR::InstId inst_id) -> void {
+  CARBON_CHECK(inst_id.has_value());
+  context.name_scopes().Get(scope_id).AddRequired(
+      {.name_id = name_id,
+       .result = SemIR::ScopeLookupResult::MakeFound(inst_id, access_kind)});
 }
 
 // Maps a Clang name to a Carbon `NameId`.
@@ -355,10 +356,6 @@ static auto ClangLookup(Context& context, SemIR::NameScopeId scope_id,
               sema.getPreprocessor().getIdentifierInfo(*name)),
           clang::SourceLocation()),
       clang::Sema::LookupNameKind::LookupOrdinaryName);
-  // TODO: Diagnose on access and return the `AccessKind` for storage. We'll
-  // probably need a dedicated `DiagnosticConsumer` because
-  // `TextDiagnosticPrinter` assumes we're processing a C++ source file.
-  lookup.suppressDiagnostics();
 
   auto scope_clang_decl_context_id =
       context.name_scopes().Get(scope_id).clang_decl_context_id();
@@ -1364,22 +1361,41 @@ static auto ImportDeclAndDependencies(Context& context, SemIR::LocId loc_id,
   return inst_id;
 }
 
+// Maps `clang::AccessSpecifier` to `SemIR::AccessKind`.
+static auto MapAccess(clang::AccessSpecifier access_specifier)
+    -> SemIR::AccessKind {
+  switch (access_specifier) {
+    case clang::AS_public:
+    case clang::AS_none:
+      return SemIR::AccessKind::Public;
+    case clang::AS_protected:
+      return SemIR::AccessKind::Protected;
+    case clang::AS_private:
+      return SemIR::AccessKind::Private;
+  }
+}
+
 // Imports a `clang::NamedDecl` into Carbon and adds that name into the
 // `NameScope`.
 static auto ImportNameDeclIntoScope(Context& context, SemIR::LocId loc_id,
                                     SemIR::NameScopeId scope_id,
                                     SemIR::NameId name_id,
                                     clang::NamedDecl* clang_decl)
-    -> SemIR::InstId {
+    -> SemIR::ScopeLookupResult {
   SemIR::InstId inst_id =
       ImportDeclAndDependencies(context, loc_id, clang_decl);
-  AddNameToScope(context, scope_id, name_id, inst_id);
-  return inst_id;
+  if (!inst_id.has_value()) {
+    return SemIR::ScopeLookupResult::MakeNotFound();
+  }
+  SemIR::AccessKind access_kind = MapAccess(clang_decl->getAccess());
+  AddNameToScope(context, scope_id, name_id, access_kind, inst_id);
+  return SemIR::ScopeLookupResult::MakeWrappedLookupResult(inst_id,
+                                                           access_kind);
 }
 
 auto ImportNameFromCpp(Context& context, SemIR::LocId loc_id,
                        SemIR::NameScopeId scope_id, SemIR::NameId name_id)
-    -> SemIR::InstId {
+    -> SemIR::ScopeLookupResult {
   Diagnostics::AnnotationScope annotate_diagnostics(
       &context.emitter(), [&](auto& builder) {
         CARBON_DIAGNOSTIC(InCppNameLookup, Note,
@@ -1389,7 +1405,7 @@ auto ImportNameFromCpp(Context& context, SemIR::LocId loc_id,
 
   auto lookup = ClangLookup(context, scope_id, name_id);
   if (!lookup) {
-    return SemIR::InstId::None;
+    return SemIR::ScopeLookupResult::MakeNotFound();
   }
 
   if (!lookup->isSingleResult()) {
@@ -1400,7 +1416,7 @@ auto ImportNameFromCpp(Context& context, SemIR::LocId loc_id,
                      .str());
     context.name_scopes().AddRequiredName(scope_id, name_id,
                                           SemIR::ErrorInst::InstId);
-    return SemIR::ErrorInst::InstId;
+    return SemIR::ScopeLookupResult::MakeError();
   }
 
   return ImportNameDeclIntoScope(context, loc_id, scope_id, name_id,

+ 3 - 3
toolchain/check/import_cpp.h

@@ -24,11 +24,11 @@ auto ImportCppFiles(Context& context,
                     std::shared_ptr<clang::CompilerInvocation> invocation)
     -> std::unique_ptr<clang::ASTUnit>;
 
-// Looks up the given name in the Clang AST generated when importing C++ code.
-// If successful, generates the instruction and returns the new `InstId`.
+// Looks up the given name in the Clang AST generated when importing C++ code
+// and returns a lookup result.
 auto ImportNameFromCpp(Context& context, SemIR::LocId loc_id,
                        SemIR::NameScopeId scope_id, SemIR::NameId name_id)
-    -> SemIR::InstId;
+    -> SemIR::ScopeLookupResult;
 
 }  // namespace Carbon::Check
 

+ 1 - 6
toolchain/check/name_lookup.cpp

@@ -187,12 +187,7 @@ auto LookupNameInExactScope(Context& context, SemIR::LocId loc_id,
   }
 
   if (scope.is_cpp_scope()) {
-    SemIR::InstId imported_inst_id =
-        ImportNameFromCpp(context, loc_id, scope_id, name_id);
-    if (imported_inst_id.has_value()) {
-      return SemIR::ScopeLookupResult::MakeFound(imported_inst_id,
-                                                 SemIR::AccessKind::Public);
-    }
+    return ImportNameFromCpp(context, loc_id, scope_id, name_id);
   }
 
   return SemIR::ScopeLookupResult::MakeNotFound();

+ 383 - 0
toolchain/check/testdata/interop/cpp/class/access.carbon

@@ -0,0 +1,383 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// INCLUDE-FILE: toolchain/testing/testdata/min_prelude/int.carbon
+//
+// AUTOUPDATE
+// TIP: To test this file alone, run:
+// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/interop/cpp/class/access.carbon
+// TIP: To dump output, run:
+// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/interop/cpp/class/access.carbon
+
+// ============================================================================
+// Public non-function member
+// ============================================================================
+
+// --- non_function_member_public.h
+
+struct S {
+  int x;
+};
+
+// --- import_non_function_member_public.carbon
+
+library "[[@TEST_NAME]]";
+
+import Cpp library "non_function_member_public.h";
+
+fn F(s: Cpp.S) {
+  //@dump-sem-ir-begin
+  let x: i32 = s.x;
+  //@dump-sem-ir-end
+}
+
+// ============================================================================
+// Protected non-function member
+// ============================================================================
+
+// --- non_function_member_protected.h
+
+class C {
+ protected:
+  int x;
+};
+
+// --- fail_import_non_function_member_protected.carbon
+
+library "[[@TEST_NAME]]";
+
+import Cpp library "non_function_member_protected.h";
+
+fn F(c: Cpp.C) {
+  // CHECK:STDERR: fail_import_non_function_member_protected.carbon:[[@LINE+6]]:16: error: cannot access protected member `x` of type `Cpp.C` [ClassInvalidMemberAccess]
+  // CHECK:STDERR:   let x: i32 = c.x;
+  // CHECK:STDERR:                ^~~
+  // CHECK:STDERR: fail_import_non_function_member_protected.carbon:[[@LINE-6]]:1: in import [InImport]
+  // CHECK:STDERR: ./non_function_member_protected.h:2: note: declared here [ClassMemberDeclaration]
+  // CHECK:STDERR:
+  let x: i32 = c.x;
+}
+
+// ============================================================================
+// Private non-function member
+// ============================================================================
+
+// --- non_function_member_private.h
+
+class C {
+  int x;
+};
+
+// --- fail_import_non_function_member_private.carbon
+
+library "[[@TEST_NAME]]";
+
+import Cpp library "non_function_member_private.h";
+
+fn F(c: Cpp.C) {
+  // CHECK:STDERR: fail_import_non_function_member_private.carbon:[[@LINE+6]]:16: error: cannot access private member `x` of type `Cpp.C` [ClassInvalidMemberAccess]
+  // CHECK:STDERR:   let x: i32 = c.x;
+  // CHECK:STDERR:                ^~~
+  // CHECK:STDERR: fail_import_non_function_member_private.carbon:[[@LINE-6]]:1: in import [InImport]
+  // CHECK:STDERR: ./non_function_member_private.h:2: note: declared here [ClassMemberDeclaration]
+  // CHECK:STDERR:
+  let x: i32 = c.x;
+}
+
+// ============================================================================
+// Public function member
+// ============================================================================
+
+// --- function_member_public.h
+
+struct S {
+  auto foo() -> void;
+};
+
+// --- import_function_member_public.carbon
+
+library "[[@TEST_NAME]]";
+
+import Cpp library "function_member_public.h";
+
+fn F(s: Cpp.S*) {
+  //@dump-sem-ir-begin
+  s->foo();
+  //@dump-sem-ir-end
+}
+
+// ============================================================================
+// Protected function member
+// ============================================================================
+
+// --- function_member_protected.h
+
+class C {
+ protected:
+  auto foo() -> void;
+};
+
+// --- fail_import_function_member_protected.carbon
+
+library "[[@TEST_NAME]]";
+
+import Cpp library "function_member_protected.h";
+
+fn F(c: Cpp.C*) {
+  // CHECK:STDERR: fail_import_function_member_protected.carbon:[[@LINE+5]]:3: error: cannot access protected member `foo` of type `Cpp.C` [ClassInvalidMemberAccess]
+  // CHECK:STDERR:   c->foo();
+  // CHECK:STDERR:   ^~~~~~
+  // CHECK:STDERR: fail_import_function_member_protected.carbon: note: declared here [ClassMemberDeclaration]
+  // CHECK:STDERR:
+  c->foo();
+}
+
+// ============================================================================
+// Private function member
+// ============================================================================
+
+// --- function_member_private.h
+
+class C {
+ private:
+  auto foo() -> void;
+};
+
+// --- fail_import_function_member_private.carbon
+
+library "[[@TEST_NAME]]";
+
+import Cpp library "function_member_private.h";
+
+fn F(c: Cpp.C*) {
+  // CHECK:STDERR: fail_import_function_member_private.carbon:[[@LINE+5]]:3: error: cannot access private member `foo` of type `Cpp.C` [ClassInvalidMemberAccess]
+  // CHECK:STDERR:   c->foo();
+  // CHECK:STDERR:   ^~~~~~
+  // CHECK:STDERR: fail_import_function_member_private.carbon: note: declared here [ClassMemberDeclaration]
+  // CHECK:STDERR:
+  c->foo();
+}
+
+// ============================================================================
+// Overload set
+// ============================================================================
+
+// --- overload_set.h
+
+class C {
+ public:
+  static auto foo() -> void;
+ protected:
+  static auto foo(int a) -> void;
+ private:
+  static auto foo(int a, int b) -> void;
+};
+
+// --- fail_todo_import_overload_set_public.carbon
+
+library "[[@TEST_NAME]]";
+
+import Cpp library "overload_set.h";
+
+fn F() {
+  //@dump-sem-ir-begin
+  // CHECK:STDERR: fail_todo_import_overload_set_public.carbon:[[@LINE+7]]:3: error: semantics TODO: `Unsupported: Lookup succeeded but couldn't find a single result; LookupResultKind: 3` [SemanticsTodo]
+  // CHECK:STDERR:   Cpp.C.foo();
+  // CHECK:STDERR:   ^~~~~~~~~
+  // CHECK:STDERR: fail_todo_import_overload_set_public.carbon:[[@LINE+4]]:3: note: in `Cpp` name lookup for `foo` [InCppNameLookup]
+  // CHECK:STDERR:   Cpp.C.foo();
+  // CHECK:STDERR:   ^~~~~~~~~
+  // CHECK:STDERR:
+  Cpp.C.foo();
+  //@dump-sem-ir-end
+}
+
+// --- fail_todo_import_overload_set_protected.carbon
+
+library "[[@TEST_NAME]]";
+
+import Cpp library "overload_set.h";
+
+fn F() {
+  // CHECK:STDERR: fail_todo_import_overload_set_protected.carbon:[[@LINE+7]]:3: error: semantics TODO: `Unsupported: Lookup succeeded but couldn't find a single result; LookupResultKind: 3` [SemanticsTodo]
+  // CHECK:STDERR:   Cpp.C.foo(1 as i32);
+  // CHECK:STDERR:   ^~~~~~~~~
+  // CHECK:STDERR: fail_todo_import_overload_set_protected.carbon:[[@LINE+4]]:3: note: in `Cpp` name lookup for `foo` [InCppNameLookup]
+  // CHECK:STDERR:   Cpp.C.foo(1 as i32);
+  // CHECK:STDERR:   ^~~~~~~~~
+  // CHECK:STDERR:
+  Cpp.C.foo(1 as i32);
+}
+
+// --- fail_todo_import_overload_set_private.carbon
+
+library "[[@TEST_NAME]]";
+
+import Cpp library "overload_set.h";
+
+fn F() {
+  // CHECK:STDERR: fail_todo_import_overload_set_private.carbon:[[@LINE+7]]:3: error: semantics TODO: `Unsupported: Lookup succeeded but couldn't find a single result; LookupResultKind: 3` [SemanticsTodo]
+  // CHECK:STDERR:   Cpp.C.foo(1 as i32, 2 as i32);
+  // CHECK:STDERR:   ^~~~~~~~~
+  // CHECK:STDERR: fail_todo_import_overload_set_private.carbon:[[@LINE+4]]:3: note: in `Cpp` name lookup for `foo` [InCppNameLookup]
+  // CHECK:STDERR:   Cpp.C.foo(1 as i32, 2 as i32);
+  // CHECK:STDERR:   ^~~~~~~~~
+  // CHECK:STDERR:
+  Cpp.C.foo(1 as i32, 2 as i32);
+}
+
+// ============================================================================
+// Base class
+// ============================================================================
+
+// --- base_class.h
+
+struct Base {
+  static auto foo() -> void;
+};
+
+class DerivedPublic : public Base {};
+class DerivedProtected : protected Base {};
+class DerivedPrivate : private Base {};
+
+// --- import_base_class_public.carbon
+
+library "[[@TEST_NAME]]";
+
+import Cpp library "base_class.h";
+
+fn F() {
+  //@dump-sem-ir-begin
+  Cpp.DerivedPublic.foo();
+  //@dump-sem-ir-end
+}
+
+// --- todo_fail_import_base_class_protected.carbon
+
+library "[[@TEST_NAME]]";
+
+import Cpp library "base_class.h";
+
+fn F() {
+  Cpp.DerivedProtected.foo();
+}
+
+// --- todo_fail_import_base_class_private.carbon
+
+library "[[@TEST_NAME]]";
+
+import Cpp library "base_class.h";
+
+fn F() {
+  Cpp.DerivedPrivate.foo();
+}
+
+// CHECK:STDOUT: --- import_non_function_member_public.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %S: type = class_type @S [concrete]
+// CHECK:STDOUT:   %int_32: Core.IntLiteral = int_value 32 [concrete]
+// CHECK:STDOUT:   %i32: type = class_type @Int, @Int(%int_32) [concrete]
+// CHECK:STDOUT:   %S.elem: type = unbound_element_type %S, %i32 [concrete]
+// CHECK:STDOUT:   %pattern_type.7ce: type = pattern_type %i32 [concrete]
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: imports {
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @F(%s.param: %S) {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   name_binding_decl {
+// CHECK:STDOUT:     %x.patt: %pattern_type.7ce = binding_pattern x [concrete]
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %s.ref: %S = name_ref s, %s
+// CHECK:STDOUT:   %x.ref: %S.elem = name_ref x, @S.%.1 [concrete = @S.%.1]
+// CHECK:STDOUT:   %.loc8_17.1: ref %i32 = class_element_access %s.ref, element0
+// CHECK:STDOUT:   %.loc8_17.2: %i32 = bind_value %.loc8_17.1
+// CHECK:STDOUT:   %.loc8_10: type = splice_block %i32 [concrete = constants.%i32] {
+// CHECK:STDOUT:     %int_32: Core.IntLiteral = int_value 32 [concrete = constants.%int_32]
+// CHECK:STDOUT:     %i32: type = class_type @Int, @Int(constants.%int_32) [concrete = constants.%i32]
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %x: %i32 = bind_name x, %.loc8_17.2
+// CHECK:STDOUT:   <elided>
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: --- import_function_member_public.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %S: type = class_type @S [concrete]
+// CHECK:STDOUT:   %ptr.5c7: type = ptr_type %S [concrete]
+// CHECK:STDOUT:   %empty_tuple.type: type = tuple_type () [concrete]
+// CHECK:STDOUT:   %S.foo.type: type = fn_type @S.foo [concrete]
+// CHECK:STDOUT:   %S.foo: %S.foo.type = struct_value () [concrete]
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: imports {
+// CHECK:STDOUT:   %S.foo.decl: %S.foo.type = fn_decl @S.foo [concrete = constants.%S.foo] {
+// CHECK:STDOUT:     <elided>
+// CHECK:STDOUT:   } {
+// CHECK:STDOUT:     <elided>
+// CHECK:STDOUT:   }
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @F(%s.param: %ptr.5c7) {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   %s.ref: %ptr.5c7 = name_ref s, %s
+// CHECK:STDOUT:   %.loc8: ref %S = deref %s.ref
+// CHECK:STDOUT:   %foo.ref: %S.foo.type = name_ref foo, imports.%S.foo.decl [concrete = constants.%S.foo]
+// CHECK:STDOUT:   %S.foo.bound: <bound method> = bound_method %.loc8, %foo.ref
+// CHECK:STDOUT:   %addr: %ptr.5c7 = addr_of %.loc8
+// CHECK:STDOUT:   %S.foo.call: init %empty_tuple.type = call %S.foo.bound(%addr)
+// CHECK:STDOUT:   <elided>
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: --- fail_todo_import_overload_set_public.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %C: type = class_type @C [concrete]
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: imports {
+// CHECK:STDOUT:   %Cpp: <namespace> = namespace file.%Cpp.import_cpp, [concrete] {
+// CHECK:STDOUT:     .C = %C.decl
+// CHECK:STDOUT:     import Cpp//...
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %C.decl: type = class_decl @C [concrete = constants.%C] {} {}
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @F() {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   %Cpp.ref: <namespace> = name_ref Cpp, imports.%Cpp [concrete = imports.%Cpp]
+// CHECK:STDOUT:   %C.ref: type = name_ref C, imports.%C.decl [concrete = constants.%C]
+// CHECK:STDOUT:   %foo.ref: <error> = name_ref foo, <error> [concrete = <error>]
+// CHECK:STDOUT:   <elided>
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: --- import_base_class_public.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %empty_tuple.type: type = tuple_type () [concrete]
+// CHECK:STDOUT:   %DerivedPublic: type = class_type @DerivedPublic [concrete]
+// CHECK:STDOUT:   %Base.foo.type: type = fn_type @Base.foo [concrete]
+// CHECK:STDOUT:   %Base.foo: %Base.foo.type = struct_value () [concrete]
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: imports {
+// CHECK:STDOUT:   %Cpp: <namespace> = namespace file.%Cpp.import_cpp, [concrete] {
+// CHECK:STDOUT:     .DerivedPublic = %DerivedPublic.decl
+// CHECK:STDOUT:     import Cpp//...
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %DerivedPublic.decl: type = class_decl @DerivedPublic [concrete = constants.%DerivedPublic] {} {}
+// CHECK:STDOUT:   %Base.foo.decl: %Base.foo.type = fn_decl @Base.foo [concrete = constants.%Base.foo] {} {}
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @F() {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   %Cpp.ref: <namespace> = name_ref Cpp, imports.%Cpp [concrete = imports.%Cpp]
+// CHECK:STDOUT:   %DerivedPublic.ref: type = name_ref DerivedPublic, imports.%DerivedPublic.decl [concrete = constants.%DerivedPublic]
+// CHECK:STDOUT:   %foo.ref: %Base.foo.type = name_ref foo, imports.%Base.foo.decl [concrete = constants.%Base.foo]
+// CHECK:STDOUT:   %Base.foo.call: init %empty_tuple.type = call %foo.ref()
+// CHECK:STDOUT:   <elided>
+// CHECK:STDOUT: }
+// CHECK:STDOUT:

+ 24 - 40
toolchain/check/testdata/interop/cpp/class/class.carbon

@@ -82,21 +82,21 @@ fn MyF(bar: Cpp.Bar*);
 //@dump-sem-ir-end
 
 // ============================================================================
-// Public static member function
+// Static member function
 // ============================================================================
 
-// --- public_static_member_function.h
+// --- static_member_function.h
 
 class Bar {
  public:
   static auto foo() -> void;
 };
 
-// --- import_public_static_member_function.carbon
+// --- import_static_member_function.carbon
 
 library "[[@TEST_NAME]]";
 
-import Cpp library "public_static_member_function.h";
+import Cpp library "static_member_function.h";
 
 fn MyF() {
   //@dump-sem-ir-begin
@@ -105,53 +105,32 @@ fn MyF() {
 }
 
 // ============================================================================
-// Private static member function
+// Static data member
 // ============================================================================
 
-// --- private_static_member_function.h
-
-class Bar {
- private:
-  static auto foo() -> void;
-};
-
-// --- todo_fail_import_private_static_member_function.carbon
-
-library "[[@TEST_NAME]]";
-
-import Cpp library "private_static_member_function.h";
-
-fn MyF() {
-  Cpp.Bar.foo();
-}
-
-// ============================================================================
-// Public static data member
-// ============================================================================
-
-// --- public_static_data_member.h
+// --- static_data_member.h
 
 class Bar {
  public:
   static Bar* foo;
 };
 
-// --- fail_todo_import_public_static_data_member.carbon
+// --- fail_todo_import_static_data_member.carbon
 
 library "[[@TEST_NAME]]";
 
-import Cpp library "public_static_data_member.h";
+import Cpp library "static_data_member.h";
 
 fn MyF() {
   //@dump-sem-ir-begin
-  // CHECK:STDERR: fail_todo_import_public_static_data_member.carbon:[[@LINE+11]]:23: error: semantics TODO: `Unsupported: Declaration type Var` [SemanticsTodo]
+  // CHECK:STDERR: fail_todo_import_static_data_member.carbon:[[@LINE+11]]:23: error: semantics TODO: `Unsupported: Declaration type Var` [SemanticsTodo]
   // CHECK:STDERR:   let bar: Cpp.Bar* = Cpp.Bar.foo();
   // CHECK:STDERR:                       ^~~~~~~~~~~
-  // CHECK:STDERR: fail_todo_import_public_static_data_member.carbon:[[@LINE+8]]:23: note: in `Cpp` name lookup for `foo` [InCppNameLookup]
+  // CHECK:STDERR: fail_todo_import_static_data_member.carbon:[[@LINE+8]]:23: note: in `Cpp` name lookup for `foo` [InCppNameLookup]
   // CHECK:STDERR:   let bar: Cpp.Bar* = Cpp.Bar.foo();
   // CHECK:STDERR:                       ^~~~~~~~~~~
   // CHECK:STDERR:
-  // CHECK:STDERR: fail_todo_import_public_static_data_member.carbon:[[@LINE+4]]:23: error: member name `foo` not found in `Cpp.Bar` [MemberNameNotFoundInInstScope]
+  // CHECK:STDERR: fail_todo_import_static_data_member.carbon:[[@LINE+4]]:23: error: member name `foo` not found in `Cpp.Bar` [MemberNameNotFoundInInstScope]
   // CHECK:STDERR:   let bar: Cpp.Bar* = Cpp.Bar.foo();
   // CHECK:STDERR:                       ^~~~~~~~~~~
   // CHECK:STDERR:
@@ -160,21 +139,21 @@ fn MyF() {
 }
 
 // ============================================================================
-// Public data member
+// Data member
 // ============================================================================
 
-// --- public_data_member.h
+// --- data_member.h
 
 class Bar {
  public:
   Bar* _Nonnull foo;
 };
 
-// --- import_public_data_member.carbon
+// --- import_data_member.carbon
 
 library "[[@TEST_NAME]]";
 
-import Cpp library "public_data_member.h";
+import Cpp library "data_member.h";
 
 //@dump-sem-ir-begin
 fn MyF(bar : Cpp.Bar*) {
@@ -241,7 +220,7 @@ class Bar {
   static auto foo() -> void;
 };
 
-// --- todo_fail_to_inherit_private.carbon
+// --- fail_to_inherit_private.carbon
 
 library "[[@TEST_NAME]]";
 
@@ -252,6 +231,11 @@ class Derived {
 }
 
 fn MyF() {
+  // CHECK:STDERR: fail_to_inherit_private.carbon:[[@LINE+5]]:3: error: cannot access private member `foo` of type `Cpp.Bar` [ClassInvalidMemberAccess]
+  // CHECK:STDERR:   Derived.foo();
+  // CHECK:STDERR:   ^~~~~~~~~~~
+  // CHECK:STDERR: fail_to_inherit_private.carbon: note: declared here [ClassMemberDeclaration]
+  // CHECK:STDERR:
   Derived.foo();
 }
 
@@ -390,7 +374,7 @@ fn MyF(bar: Cpp.Bar*);
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @MyF(%bar.param: %ptr);
 // CHECK:STDOUT:
-// CHECK:STDOUT: --- import_public_static_member_function.carbon
+// CHECK:STDOUT: --- import_static_member_function.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {
 // CHECK:STDOUT:   %empty_tuple.type: type = tuple_type () [concrete]
@@ -417,7 +401,7 @@ fn MyF(bar: Cpp.Bar*);
 // CHECK:STDOUT:   <elided>
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: --- fail_todo_import_public_static_data_member.carbon
+// CHECK:STDOUT: --- fail_todo_import_static_data_member.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {
 // CHECK:STDOUT:   %Bar: type = class_type @Bar [concrete]
@@ -450,7 +434,7 @@ fn MyF(bar: Cpp.Bar*);
 // CHECK:STDOUT:   <elided>
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: --- import_public_data_member.carbon
+// CHECK:STDOUT: --- import_data_member.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {
 // CHECK:STDOUT:   %Bar: type = class_type @Bar [concrete]

+ 24 - 42
toolchain/check/testdata/interop/cpp/class/struct.carbon

@@ -82,20 +82,20 @@ fn MyF(bar: Cpp.Bar*);
 //@dump-sem-ir-end
 
 // ============================================================================
-// Public static member function
+// Static member function
 // ============================================================================
 
-// --- public_static_member_function.h
+// --- static_member_function.h
 
 struct Bar {
   static auto foo() -> void;
 };
 
-// --- import_public_static_member_function.carbon
+// --- import_static_member_function.carbon
 
 library "[[@TEST_NAME]]";
 
-import Cpp library "public_static_member_function.h";
+import Cpp library "static_member_function.h";
 
 fn MyF() {
   //@dump-sem-ir-begin
@@ -104,53 +104,31 @@ fn MyF() {
 }
 
 // ============================================================================
-// Private static member function
+// Static data member
 // ============================================================================
 
-// --- private_static_member_function.h
-
-struct Bar {
- private:
-  static auto foo() -> void;
-};
-
-// --- todo_fail_import_private_static_member_function.carbon
-
-library "[[@TEST_NAME]]";
-
-import Cpp library "private_static_member_function.h";
-
-fn MyF() {
-  // TODO: Should fail because `foo` is inaccessible.
-  Cpp.Bar.foo();
-}
-
-// ============================================================================
-// Public static data member
-// ============================================================================
-
-// --- public_static_data_member.h
+// --- static_data_member.h
 
 struct Bar {
   static Bar* _Nonnull foo;
 };
 
-// --- fail_todo_import_public_static_data_member.carbon
+// --- fail_todo_import_static_data_member.carbon
 
 library "[[@TEST_NAME]]";
 
-import Cpp library "public_static_data_member.h";
+import Cpp library "static_data_member.h";
 
 fn MyF() {
   //@dump-sem-ir-begin
-  // CHECK:STDERR: fail_todo_import_public_static_data_member.carbon:[[@LINE+11]]:23: error: semantics TODO: `Unsupported: Declaration type Var` [SemanticsTodo]
+  // CHECK:STDERR: fail_todo_import_static_data_member.carbon:[[@LINE+11]]:23: error: semantics TODO: `Unsupported: Declaration type Var` [SemanticsTodo]
   // CHECK:STDERR:   let bar: Cpp.Bar* = Cpp.Bar.foo();
   // CHECK:STDERR:                       ^~~~~~~~~~~
-  // CHECK:STDERR: fail_todo_import_public_static_data_member.carbon:[[@LINE+8]]:23: note: in `Cpp` name lookup for `foo` [InCppNameLookup]
+  // CHECK:STDERR: fail_todo_import_static_data_member.carbon:[[@LINE+8]]:23: note: in `Cpp` name lookup for `foo` [InCppNameLookup]
   // CHECK:STDERR:   let bar: Cpp.Bar* = Cpp.Bar.foo();
   // CHECK:STDERR:                       ^~~~~~~~~~~
   // CHECK:STDERR:
-  // CHECK:STDERR: fail_todo_import_public_static_data_member.carbon:[[@LINE+4]]:23: error: member name `foo` not found in `Cpp.Bar` [MemberNameNotFoundInInstScope]
+  // CHECK:STDERR: fail_todo_import_static_data_member.carbon:[[@LINE+4]]:23: error: member name `foo` not found in `Cpp.Bar` [MemberNameNotFoundInInstScope]
   // CHECK:STDERR:   let bar: Cpp.Bar* = Cpp.Bar.foo();
   // CHECK:STDERR:                       ^~~~~~~~~~~
   // CHECK:STDERR:
@@ -159,20 +137,20 @@ fn MyF() {
 }
 
 // ============================================================================
-// Public data member
+// Data member
 // ============================================================================
 
-// --- public_data_member.h
+// --- data_member.h
 
 struct Bar {
   Bar* _Nonnull foo;
 };
 
-// --- import_public_data_member.carbon
+// --- import_data_member.carbon
 
 library "[[@TEST_NAME]]";
 
-import Cpp library "public_data_member.h";
+import Cpp library "data_member.h";
 
 //@dump-sem-ir-begin
 fn MyF(bar : Cpp.Bar*) {
@@ -254,7 +232,7 @@ struct Bar {
   static auto foo() -> void;
 };
 
-// --- todo_fail_to_inherit_private.carbon
+// --- fail_to_inherit_private.carbon
 
 library "[[@TEST_NAME]]";
 
@@ -265,7 +243,11 @@ class Derived {
 }
 
 fn MyF() {
-  // TODO: Should fail because `foo` is inaccessible.
+  // CHECK:STDERR: fail_to_inherit_private.carbon:[[@LINE+5]]:3: error: cannot access private member `foo` of type `Cpp.Bar` [ClassInvalidMemberAccess]
+  // CHECK:STDERR:   Derived.foo();
+  // CHECK:STDERR:   ^~~~~~~~~~~
+  // CHECK:STDERR: fail_to_inherit_private.carbon: note: declared here [ClassMemberDeclaration]
+  // CHECK:STDERR:
   Derived.foo();
 }
 
@@ -404,7 +386,7 @@ fn MyF(bar: Cpp.Bar*);
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @MyF(%bar.param: %ptr);
 // CHECK:STDOUT:
-// CHECK:STDOUT: --- import_public_static_member_function.carbon
+// CHECK:STDOUT: --- import_static_member_function.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {
 // CHECK:STDOUT:   %empty_tuple.type: type = tuple_type () [concrete]
@@ -431,7 +413,7 @@ fn MyF(bar: Cpp.Bar*);
 // CHECK:STDOUT:   <elided>
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: --- fail_todo_import_public_static_data_member.carbon
+// CHECK:STDOUT: --- fail_todo_import_static_data_member.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {
 // CHECK:STDOUT:   %Bar: type = class_type @Bar [concrete]
@@ -464,7 +446,7 @@ fn MyF(bar: Cpp.Bar*);
 // CHECK:STDOUT:   <elided>
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: --- import_public_data_member.carbon
+// CHECK:STDOUT: --- import_data_member.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {
 // CHECK:STDOUT:   %Bar: type = class_type @Bar [concrete]

+ 18 - 40
toolchain/check/testdata/interop/cpp/class/union.carbon

@@ -82,21 +82,21 @@ fn MyF(bar: Cpp.Bar*);
 //@dump-sem-ir-end
 
 // ============================================================================
-// Public static member function
+// Static member function
 // ============================================================================
 
-// --- public_static_member_function.h
+// --- static_member_function.h
 
 union Bar {
  public:
   static auto foo() -> void;
 };
 
-// --- import_public_static_member_function.carbon
+// --- import_static_member_function.carbon
 
 library "[[@TEST_NAME]]";
 
-import Cpp library "public_static_member_function.h";
+import Cpp library "static_member_function.h";
 
 fn MyF() {
   //@dump-sem-ir-begin
@@ -105,54 +105,32 @@ fn MyF() {
 }
 
 // ============================================================================
-// Private static member function
+// Static data member
 // ============================================================================
 
-// --- private_static_member_function.h
-
-union Bar {
- private:
-  static auto foo() -> void;
-};
-
-// --- todo_fail_import_private_static_member_function.carbon
-
-library "[[@TEST_NAME]]";
-
-import Cpp library "private_static_member_function.h";
-
-fn MyF() {
-  // TODO: Should fail because `foo` is inaccessible.
-  Cpp.Bar.foo();
-}
-
-// ============================================================================
-// Public static data member
-// ============================================================================
-
-// --- public_static_data_member.h
+// --- static_data_member.h
 
 union Bar {
  public:
   static Bar* _Nonnull foo;
 };
 
-// --- fail_todo_import_public_static_data_member.carbon
+// --- fail_todo_import_static_data_member.carbon
 
 library "[[@TEST_NAME]]";
 
-import Cpp library "public_static_data_member.h";
+import Cpp library "static_data_member.h";
 
 fn MyF() {
   //@dump-sem-ir-begin
-  // CHECK:STDERR: fail_todo_import_public_static_data_member.carbon:[[@LINE+11]]:23: error: semantics TODO: `Unsupported: Declaration type Var` [SemanticsTodo]
+  // CHECK:STDERR: fail_todo_import_static_data_member.carbon:[[@LINE+11]]:23: error: semantics TODO: `Unsupported: Declaration type Var` [SemanticsTodo]
   // CHECK:STDERR:   let bar: Cpp.Bar* = Cpp.Bar.foo();
   // CHECK:STDERR:                       ^~~~~~~~~~~
-  // CHECK:STDERR: fail_todo_import_public_static_data_member.carbon:[[@LINE+8]]:23: note: in `Cpp` name lookup for `foo` [InCppNameLookup]
+  // CHECK:STDERR: fail_todo_import_static_data_member.carbon:[[@LINE+8]]:23: note: in `Cpp` name lookup for `foo` [InCppNameLookup]
   // CHECK:STDERR:   let bar: Cpp.Bar* = Cpp.Bar.foo();
   // CHECK:STDERR:                       ^~~~~~~~~~~
   // CHECK:STDERR:
-  // CHECK:STDERR: fail_todo_import_public_static_data_member.carbon:[[@LINE+4]]:23: error: member name `foo` not found in `Cpp.Bar` [MemberNameNotFoundInInstScope]
+  // CHECK:STDERR: fail_todo_import_static_data_member.carbon:[[@LINE+4]]:23: error: member name `foo` not found in `Cpp.Bar` [MemberNameNotFoundInInstScope]
   // CHECK:STDERR:   let bar: Cpp.Bar* = Cpp.Bar.foo();
   // CHECK:STDERR:                       ^~~~~~~~~~~
   // CHECK:STDERR:
@@ -161,21 +139,21 @@ fn MyF() {
 }
 
 // ============================================================================
-// Public data member
+// Data member
 // ============================================================================
 
-// --- public_data_member.h
+// --- data_member.h
 
 union Bar {
  public:
   Bar* _Nonnull foo;
 };
 
-// --- import_public_data_member.carbon
+// --- import_data_member.carbon
 
 library "[[@TEST_NAME]]";
 
-import Cpp library "public_data_member.h";
+import Cpp library "data_member.h";
 
 //@dump-sem-ir-begin
 fn MyF(bar : Cpp.Bar*) {
@@ -344,7 +322,7 @@ fn MyF(bar: Cpp.Bar*);
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @MyF(%bar.param: %ptr);
 // CHECK:STDOUT:
-// CHECK:STDOUT: --- import_public_static_member_function.carbon
+// CHECK:STDOUT: --- import_static_member_function.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {
 // CHECK:STDOUT:   %empty_tuple.type: type = tuple_type () [concrete]
@@ -371,7 +349,7 @@ fn MyF(bar: Cpp.Bar*);
 // CHECK:STDOUT:   <elided>
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: --- fail_todo_import_public_static_data_member.carbon
+// CHECK:STDOUT: --- fail_todo_import_static_data_member.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {
 // CHECK:STDOUT:   %Bar: type = class_type @Bar [concrete]
@@ -404,7 +382,7 @@ fn MyF(bar: Cpp.Bar*);
 // CHECK:STDOUT:   <elided>
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: --- import_public_data_member.carbon
+// CHECK:STDOUT: --- import_data_member.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {
 // CHECK:STDOUT:   %Bar: type = class_type @Bar [concrete]

+ 1 - 0
toolchain/check/testdata/interop/cpp/function/class.carbon

@@ -266,6 +266,7 @@ fn F() {
 // --- definition_with_static_method.h
 
 class C {
+ public:
   static void bar();
 };