From 44b17ff436258548d14594e3327f57d0bb22f10a Mon Sep 17 00:00:00 2001 From: Nicholas Bishop Date: Fri, 5 Jun 2026 16:07:27 -0400 Subject: [PATCH] Set correct C++ access type for fields and static vars (#7312) --- toolchain/check/cpp/export.cpp | 30 ++++++++++++++----- .../interop/cpp/class/export/field.carbon | 26 ++++++++++++++++ .../interop/cpp/class/export/static.carbon | 21 +++++++++++++ 3 files changed, 69 insertions(+), 8 deletions(-) diff --git a/toolchain/check/cpp/export.cpp b/toolchain/check/cpp/export.cpp index d69ab9718075..3161c8cdc5e1 100644 --- a/toolchain/check/cpp/export.cpp +++ b/toolchain/check/cpp/export.cpp @@ -7,11 +7,13 @@ #include #include "llvm/Support/Casting.h" +#include "toolchain/check/cpp/access.h" #include "toolchain/check/cpp/import.h" #include "toolchain/check/cpp/location.h" #include "toolchain/check/cpp/type_mapping.h" #include "toolchain/check/function.h" #include "toolchain/check/import_ref.h" +#include "toolchain/check/name_lookup.h" #include "toolchain/check/pattern.h" #include "toolchain/check/thunk.h" #include "toolchain/check/type.h" @@ -208,9 +210,19 @@ static auto LookupClassFieldByStructField( return std::nullopt; } +static auto SetCppClassMemberAccess(const SemIR::NameScope& class_scope, + SemIR::NameId member_name_id, + clang::Decl* member) -> void { + auto entry_id = class_scope.Lookup(member_name_id); + CARBON_CHECK(entry_id.has_value()); + const auto& entry = class_scope.GetEntry(*entry_id); + member->setAccess(MapToCppAccess(entry.result.access_kind())); +} + // Creates a `clang::FieldDecl` for a Carbon class field. Returns // nullptr if an error occurs. static auto CreateCppFieldDecl(Context& context, + const SemIR::NameScope& class_scope, clang::CXXRecordDecl* record_decl, SemIR::InstId field_inst_id, const SemIR::FieldDecl& field_decl) @@ -238,7 +250,9 @@ static auto CreateCppFieldDecl(Context& context, /*IdLoc=*/clang_loc, identifier_info, cpp_type, /*TInfo=*/nullptr, /*BW=*/nullptr, /*Mutable=*/true, clang::ICIS_NoInit); - cpp_field_decl->setAccess(clang::AS_public); + + SetCppClassMemberAccess(class_scope, field_decl.name_id, cpp_field_decl); + record_decl->addHiddenDecl(cpp_field_decl); return cpp_field_decl; @@ -265,9 +279,9 @@ auto ExportAllFieldsToCpp(Context& context, SemIR::Class& class_info) -> void { continue; } - auto* cpp_field_decl = - CreateCppFieldDecl(context, cast(decl_context), - class_field->inst_id, class_field->inst); + auto* cpp_field_decl = CreateCppFieldDecl( + context, class_scope, cast(decl_context), + class_field->inst_id, class_field->inst); if (!cpp_field_decl) { continue; } @@ -953,8 +967,9 @@ auto ExportVarToCpp(Context& context, SemIR::InstId inst_id, auto entity_name_id = GetFirstBindingNameFromPatternId( context.sem_ir(), var_storage.pattern_id); const auto& entity_name = context.entity_names().Get(entity_name_id); - auto scope_inst = context.insts().Get( - context.name_scopes().Get(entity_name.parent_scope_id).inst_id()); + const auto& name_scope = + context.name_scopes().Get(entity_name.parent_scope_id); + auto scope_inst = context.insts().Get(name_scope.inst_id()); CARBON_CHECK(scope_inst.Is() || scope_inst.Is()); @@ -986,8 +1001,7 @@ auto ExportVarToCpp(Context& context, SemIR::InstId inst_id, var_storage.pattern_id); if (scope_inst.Is()) { - // TODO: Map Carbon access to C++ access. - var_decl->setAccess(clang::AS_public); + SetCppClassMemberAccess(name_scope, entity_name.name_id, var_decl); } // Set the Carbon mangled variable name. diff --git a/toolchain/check/testdata/interop/cpp/class/export/field.carbon b/toolchain/check/testdata/interop/cpp/class/export/field.carbon index ddf6e256d5a1..449bd7f92696 100644 --- a/toolchain/check/testdata/interop/cpp/class/export/field.carbon +++ b/toolchain/check/testdata/interop/cpp/class/export/field.carbon @@ -138,3 +138,29 @@ class A { inline Cpp ''' Carbon::A x; '''; + +// --- fail_private_field.carbon +library "[[@TEST_NAME]]"; +import Cpp; + +class C { + fn Make() -> C { + return {.x = 123}; + } + + private var x: i32; +} + +inline Cpp ''' +int F() { + auto c = Carbon::C::Make(); + // CHECK:STDERR: fail_private_field.carbon:[[@LINE+7]]:12: error: 'x' is a private member of 'Carbon::C' [CppInteropParseError] + // CHECK:STDERR: 22 | return c.x; + // CHECK:STDERR: | ^ + // CHECK:STDERR: fail_private_field.carbon:[[@LINE-9]]:16: note: implicitly declared private here [CppInteropParseNote] + // CHECK:STDERR: 9 | private var x: i32; + // CHECK:STDERR: | ^ + // CHECK:STDERR: + return c.x; +} +'''; diff --git a/toolchain/check/testdata/interop/cpp/class/export/static.carbon b/toolchain/check/testdata/interop/cpp/class/export/static.carbon index 9efa7a51e639..8b65ef6eed88 100644 --- a/toolchain/check/testdata/interop/cpp/class/export/static.carbon +++ b/toolchain/check/testdata/interop/cpp/class/export/static.carbon @@ -23,3 +23,24 @@ int F() { return Carbon::C::x; } '''; + +// --- fail_static_private.carbon +library "[[@TEST_NAME]]"; +import Cpp; + +class C { + private static var x: i32 = 123; +} + +inline Cpp ''' +int F() { + // CHECK:STDERR: fail_static_private.carbon:[[@LINE+7]]:21: error: 'x' is a private member of 'Carbon::C' [CppInteropParseError] + // CHECK:STDERR: 17 | return Carbon::C::x; + // CHECK:STDERR: | ^ + // CHECK:STDERR: fail_static_private.carbon:[[@LINE-8]]:23: note: implicitly declared private here [CppInteropParseNote] + // CHECK:STDERR: 5 | private static var x: i32 = 123; + // CHECK:STDERR: | ^ + // CHECK:STDERR: + return Carbon::C::x; +} +''';