From 1aa34d7642411f4598e076b9e6a63c8a4ce24ce4 Mon Sep 17 00:00:00 2001 From: Nicholas Bishop Date: Thu, 4 Jun 2026 22:07:46 -0400 Subject: [PATCH] Remove CppGlobalVarStore (#7309) Replace all uses of CppGlobalVarStore store with ClangDeclStore. Adding a VarStorage->VarDecl mapping to ClangDeclStore is now done with the `AddVar` method, which takes an extra `pattern_id` arg. While the corresponding `ClangDecl` is unchanged from before, the reverse mapping in `inst_id_to_clang_decl_id_` now uses the `pattern_id` as the key. This is necessary because in some places the original VarStorage instructions gets replaced (e.g. by a call to `Convert`). The `pattern_id` remains stable in those cases. --------- Co-authored-by: Richard Smith --- toolchain/check/context.h | 3 - toolchain/check/cpp/export.cpp | 12 ++-- toolchain/check/cpp/export.h | 2 +- toolchain/check/cpp/generate_ast.cpp | 3 +- toolchain/check/cpp/import.cpp | 24 ++----- .../basics/raw_sem_ir/builtins.carbon | 1 - .../testdata/basics/raw_sem_ir/bundle.carbon | 1 - .../basics/raw_sem_ir/cpp_interop.carbon | 2 - .../basics/raw_sem_ir/multifile.carbon | 2 - .../multifile_with_textual_ir.carbon | 2 - .../raw_sem_ir/non_core_interfaces.carbon | 2 - .../basics/raw_sem_ir/one_file.carbon | 1 - .../one_file_with_textual_ir.carbon | 1 - toolchain/driver/testdata/stdin.carbon | 1 - toolchain/lower/file_context.cpp | 23 ++----- toolchain/sem_ir/BUILD | 2 - toolchain/sem_ir/clang_decl.cpp | 8 +++ toolchain/sem_ir/clang_decl.h | 12 ++++ toolchain/sem_ir/cpp_global_var.cpp | 16 ----- toolchain/sem_ir/cpp_global_var.h | 65 ------------------- toolchain/sem_ir/file.cpp | 4 -- toolchain/sem_ir/file.h | 8 --- toolchain/sem_ir/ids.h | 7 -- toolchain/sem_ir/mangler.cpp | 10 +-- toolchain/sem_ir/yaml_test.cpp | 1 - 25 files changed, 45 insertions(+), 168 deletions(-) delete mode 100644 toolchain/sem_ir/cpp_global_var.cpp delete mode 100644 toolchain/sem_ir/cpp_global_var.h diff --git a/toolchain/check/context.h b/toolchain/check/context.h index b57b072f6516..863f5952802f 100644 --- a/toolchain/check/context.h +++ b/toolchain/check/context.h @@ -328,9 +328,6 @@ class Context { auto entity_names() -> SemIR::EntityNameStore& { return sem_ir().entity_names(); } - auto cpp_global_names() -> SemIR::CppGlobalVarStore& { - return sem_ir().cpp_global_vars(); - } auto cpp_overload_sets() -> SemIR::CppOverloadSetStore& { return sem_ir().cpp_overload_sets(); } diff --git a/toolchain/check/cpp/export.cpp b/toolchain/check/cpp/export.cpp index 5dee85ec99af..d69ab9718075 100644 --- a/toolchain/check/cpp/export.cpp +++ b/toolchain/check/cpp/export.cpp @@ -938,13 +938,11 @@ auto ExportDestructorToCpp(Context& context, const SemIR::Class& class_info, return cpp_destructor_decl; } -auto ExportVarToCpp(Context& context, SemIR::LocId loc_id, +auto ExportVarToCpp(Context& context, SemIR::InstId inst_id, SemIR::VarStorage var_storage) -> clang::VarDecl* { // Check if the variable was already exported and return the existing // `VarDecl` if so. Note that the `pattern_id` is used as the key - // rather than the `InstId` for the `VarStorage`. This just makes - // lookup more convenient in places where the `VarStorage` `InstId` is - // not readily accessible. + // rather than the `InstId` for the `VarStorage`. auto clang_decl_id = context.clang_decls().Lookup(var_storage.pattern_id); if (clang_decl_id.has_value()) { return cast( @@ -961,6 +959,7 @@ auto ExportVarToCpp(Context& context, SemIR::LocId loc_id, scope_inst.Is()); // Map the parent scope into the C++ AST. + SemIR::LocId loc_id(inst_id); auto* decl_context = ExportNameScopeToCpp(context, loc_id, entity_name.parent_scope_id); if (!decl_context) { @@ -981,9 +980,10 @@ auto ExportVarToCpp(Context& context, SemIR::LocId loc_id, context.ast_context(), decl_context, /*StartLoc=*/clang_loc, /*IdLoc=*/clang_loc, identifier_info, cpp_type, /*TInfo=*/nullptr, clang::SC_Extern); - context.clang_decls().Add( + context.clang_decls().AddVar( {.key = SemIR::ClangDeclKey::ForNonFunctionDecl(var_decl), - .inst_id = var_storage.pattern_id}); + .inst_id = inst_id}, + var_storage.pattern_id); if (scope_inst.Is()) { // TODO: Map Carbon access to C++ access. diff --git a/toolchain/check/cpp/export.h b/toolchain/check/cpp/export.h index e36ae3146aca..f064430baad3 100644 --- a/toolchain/check/cpp/export.h +++ b/toolchain/check/cpp/export.h @@ -68,7 +68,7 @@ auto ExportDestructorToCpp(Context& context, const SemIR::Class& class_info, // // Returns nullptr if the variable could not be exported an an error was // diagnosed. -auto ExportVarToCpp(Context& context, SemIR::LocId loc_id, +auto ExportVarToCpp(Context& context, SemIR::InstId inst_id, SemIR::VarStorage var_storage) -> clang::VarDecl*; } // namespace Carbon::Check diff --git a/toolchain/check/cpp/generate_ast.cpp b/toolchain/check/cpp/generate_ast.cpp index 2d885caa5ec6..2e21f243dda2 100644 --- a/toolchain/check/cpp/generate_ast.cpp +++ b/toolchain/check/cpp/generate_ast.cpp @@ -436,8 +436,7 @@ auto CarbonExternalASTSource::MapInstIdToClangDeclOrType(LookupResult lookup) return ExportFieldToCpp(*context_, target_inst_id, field_decl); } case CARBON_KIND(SemIR::VarStorage var_storage): { - return ExportVarToCpp(*context_, SemIR::LocId(target_inst_id), - var_storage); + return ExportVarToCpp(*context_, target_inst_id, var_storage); } default: return nullptr; diff --git a/toolchain/check/cpp/import.cpp b/toolchain/check/cpp/import.cpp index d52e3e78fd8b..e7ef6a266949 100644 --- a/toolchain/check/cpp/import.cpp +++ b/toolchain/check/cpp/import.cpp @@ -2132,14 +2132,11 @@ static auto ImportVarDecl(Context& context, SemIR::LocId loc_id, .pattern_id = pattern_id}}); context.imports().push_back(var_storage_inst_id); - // Register the variable so we don't create it again, and track the - // corresponding declaration to use for mangling. - auto clang_decl_id = - context.clang_decls().Add({.key = SemIR::ClangDeclKey(var_decl), - .inst_id = var_storage_inst_id, - .is_imported = true}); - context.cpp_global_names().Add({.key = {.entity_name_id = entity_name_id}, - .clang_decl_id = clang_decl_id}); + // Register the variable so we don't create it again. + context.clang_decls().AddVar({.key = SemIR::ClangDeclKey(var_decl), + .inst_id = var_storage_inst_id, + .is_imported = true}, + pattern_id); // Inform Clang that the variable has been referenced. context.clang_sema().MarkVariableReferenced(GetCppLocation(context, loc_id), @@ -2640,15 +2637,8 @@ auto GetAsClangVarDecl(Context& context, SemIR::InstId inst_id) -> clang::VarDecl* { if (const auto& var_storage = context.insts().TryGetAs(inst_id)) { - auto var_name_id = SemIR::GetFirstBindingNameFromPatternId( - context.sem_ir(), var_storage->pattern_id); - if (auto cpp_global_var_id = context.sem_ir().cpp_global_vars().Lookup( - {.entity_name_id = var_name_id}); - cpp_global_var_id.has_value()) { - SemIR::ClangDeclId clang_decl_id = context.sem_ir() - .cpp_global_vars() - .Get(cpp_global_var_id) - .clang_decl_id; + auto clang_decl_id = context.clang_decls().Lookup(var_storage->pattern_id); + if (clang_decl_id.has_value()) { return cast( context.clang_decls().Get(clang_decl_id).key.decl); } diff --git a/toolchain/check/testdata/basics/raw_sem_ir/builtins.carbon b/toolchain/check/testdata/basics/raw_sem_ir/builtins.carbon index 268103d4e5b2..832a0d3eac3b 100644 --- a/toolchain/check/testdata/basics/raw_sem_ir/builtins.carbon +++ b/toolchain/check/testdata/basics/raw_sem_ir/builtins.carbon @@ -24,7 +24,6 @@ // CHECK:STDOUT: name_scopes: // CHECK:STDOUT: name_scope0: {inst: instF, parent_scope: name_scope, has_error: false, extended_scopes: [], names: {}} // CHECK:STDOUT: entity_names: {} -// CHECK:STDOUT: cpp_global_vars: {} // CHECK:STDOUT: functions: {} // CHECK:STDOUT: classes: {} // CHECK:STDOUT: interfaces: {} diff --git a/toolchain/check/testdata/basics/raw_sem_ir/bundle.carbon b/toolchain/check/testdata/basics/raw_sem_ir/bundle.carbon index 70afd002de46..5416385f5933 100644 --- a/toolchain/check/testdata/basics/raw_sem_ir/bundle.carbon +++ b/toolchain/check/testdata/basics/raw_sem_ir/bundle.carbon @@ -42,7 +42,6 @@ fn F(Form:! Core.Form()) ->? Form; // CHECK:STDOUT: entity_name70000000: {name: name(PeriodSelf), parent_scope: name_scope, index: -1, is_template: 0, is_unused: 0, form: inst} // CHECK:STDOUT: entity_name70000001: {name: name1, parent_scope: name_scope70000001, index: -1, is_template: 0, is_unused: 0, form: inst} // CHECK:STDOUT: entity_name70000002: {name: name1, parent_scope: name_scope, index: 0, is_template: 0, is_unused: 0, form: inst} -// CHECK:STDOUT: cpp_global_vars: {} // CHECK:STDOUT: functions: // CHECK:STDOUT: function70000000: {name: name1, parent_scope: name_scope70000001, call_param_patterns_id: inst_block70000006, return_type_inst_id: inst7000001E, return_form_inst_id: inst7000001F, return_pattern_id: inst7000001C} // CHECK:STDOUT: function70000001: {name: name0, parent_scope: name_scope0, call_param_patterns_id: inst_block70000008, call_params_id: inst_block70000009, return_type_inst_id: inst7000002E, return_form_inst_id: inst7000002B, return_pattern_id: inst70000035} diff --git a/toolchain/check/testdata/basics/raw_sem_ir/cpp_interop.carbon b/toolchain/check/testdata/basics/raw_sem_ir/cpp_interop.carbon index 462b59808ec6..b16ee0af6fea 100644 --- a/toolchain/check/testdata/basics/raw_sem_ir/cpp_interop.carbon +++ b/toolchain/check/testdata/basics/raw_sem_ir/cpp_interop.carbon @@ -74,8 +74,6 @@ fn G(x: Cpp.X) { // CHECK:STDOUT: entity_name50000001: {name: name1, parent_scope: name_scope, index: -1, is_template: 0, is_unused: 0, form: inst} // CHECK:STDOUT: entity_name50000002: {name: name1, parent_scope: name_scope, index: -1, is_template: 0, is_unused: 0, form: inst} // CHECK:STDOUT: entity_name50000003: {name: name4, parent_scope: name_scope50000001, index: -1, is_template: 0, is_unused: 0, form: inst} -// CHECK:STDOUT: cpp_global_vars: -// CHECK:STDOUT: cpp_global_var50000000: {key: {entity_name_id: entity_name50000003}, clang_decl_id: clang_decl_id50000008} // CHECK:STDOUT: functions: // CHECK:STDOUT: function50000000: {name: name0, parent_scope: name_scope0, call_param_patterns_id: inst_block50000007, call_params_id: inst_block50000008, body: [inst_block5000000B]} // CHECK:STDOUT: function50000001: {name: name3, parent_scope: name_scope50000001, call_param_patterns_id: inst_block_empty, call_params_id: inst_block_empty} diff --git a/toolchain/check/testdata/basics/raw_sem_ir/multifile.carbon b/toolchain/check/testdata/basics/raw_sem_ir/multifile.carbon index 3692cdf15a41..35e83b62e864 100644 --- a/toolchain/check/testdata/basics/raw_sem_ir/multifile.carbon +++ b/toolchain/check/testdata/basics/raw_sem_ir/multifile.carbon @@ -41,7 +41,6 @@ fn B() { // CHECK:STDOUT: name_scopes: // CHECK:STDOUT: name_scope0: {inst: instF, parent_scope: name_scope, has_error: false, extended_scopes: [], names: {name0: inst50000010}} // CHECK:STDOUT: entity_names: {} -// CHECK:STDOUT: cpp_global_vars: {} // CHECK:STDOUT: functions: // CHECK:STDOUT: function50000000: {name: name0, parent_scope: name_scope0, call_param_patterns_id: inst_block_empty, call_params_id: inst_block_empty, body: [inst_block50000006]} // CHECK:STDOUT: classes: {} @@ -148,7 +147,6 @@ fn B() { // CHECK:STDOUT: name_scope70000001: {inst: inst70000011, parent_scope: name_scope0, has_error: false, extended_scopes: [], names: {name1: inst70000017}} // CHECK:STDOUT: entity_names: // CHECK:STDOUT: entity_name70000000: {name: name1, parent_scope: name_scope70000001, index: -1, is_template: 0, is_unused: 0, form: inst} -// CHECK:STDOUT: cpp_global_vars: {} // CHECK:STDOUT: functions: // CHECK:STDOUT: function70000000: {name: name0, parent_scope: name_scope0, call_param_patterns_id: inst_block_empty, call_params_id: inst_block_empty, body: [inst_block70000006]} // CHECK:STDOUT: function70000001: {name: name1, parent_scope: name_scope70000001, call_param_patterns_id: inst_block_empty} diff --git a/toolchain/check/testdata/basics/raw_sem_ir/multifile_with_textual_ir.carbon b/toolchain/check/testdata/basics/raw_sem_ir/multifile_with_textual_ir.carbon index dc8029c68537..ccdbcf15d5f8 100644 --- a/toolchain/check/testdata/basics/raw_sem_ir/multifile_with_textual_ir.carbon +++ b/toolchain/check/testdata/basics/raw_sem_ir/multifile_with_textual_ir.carbon @@ -41,7 +41,6 @@ fn B() { // CHECK:STDOUT: name_scopes: // CHECK:STDOUT: name_scope0: {inst: instF, parent_scope: name_scope, has_error: false, extended_scopes: [], names: {name0: inst50000010}} // CHECK:STDOUT: entity_names: {} -// CHECK:STDOUT: cpp_global_vars: {} // CHECK:STDOUT: functions: // CHECK:STDOUT: function50000000: {name: name0, parent_scope: name_scope0, call_param_patterns_id: inst_block_empty, call_params_id: inst_block_empty, body: [inst_block50000006]} // CHECK:STDOUT: classes: {} @@ -167,7 +166,6 @@ fn B() { // CHECK:STDOUT: name_scope70000001: {inst: inst70000011, parent_scope: name_scope0, has_error: false, extended_scopes: [], names: {name1: inst70000017}} // CHECK:STDOUT: entity_names: // CHECK:STDOUT: entity_name70000000: {name: name1, parent_scope: name_scope70000001, index: -1, is_template: 0, is_unused: 0, form: inst} -// CHECK:STDOUT: cpp_global_vars: {} // CHECK:STDOUT: functions: // CHECK:STDOUT: function70000000: {name: name0, parent_scope: name_scope0, call_param_patterns_id: inst_block_empty, call_params_id: inst_block_empty, body: [inst_block70000006]} // CHECK:STDOUT: function70000001: {name: name1, parent_scope: name_scope70000001, call_param_patterns_id: inst_block_empty} diff --git a/toolchain/check/testdata/basics/raw_sem_ir/non_core_interfaces.carbon b/toolchain/check/testdata/basics/raw_sem_ir/non_core_interfaces.carbon index 944181df8f7b..70da44caa46c 100644 --- a/toolchain/check/testdata/basics/raw_sem_ir/non_core_interfaces.carbon +++ b/toolchain/check/testdata/basics/raw_sem_ir/non_core_interfaces.carbon @@ -53,7 +53,6 @@ fn UseLocalCopy[T:! Copy](_: T.T1, _: T.T2) {} // CHECK:STDOUT: name_scope60000002: {inst: inst60000015, parent_scope: name_scope60000001, has_error: false, extended_scopes: [], names: {name2: inst60000019, name3: inst6000001D}} // CHECK:STDOUT: entity_names: // CHECK:STDOUT: entity_name60000000: {name: name(SelfType), parent_scope: name_scope60000001, index: 0, is_template: 0, is_unused: 0, form: inst} -// CHECK:STDOUT: cpp_global_vars: {} // CHECK:STDOUT: functions: {} // CHECK:STDOUT: classes: {} // CHECK:STDOUT: interfaces: @@ -233,7 +232,6 @@ fn UseLocalCopy[T:! Copy](_: T.T1, _: T.T2) {} // CHECK:STDOUT: entity_name5000000A: {name: name5, parent_scope: name_scope, index: 0, is_template: 0, is_unused: 0, form: inst} // CHECK:STDOUT: entity_name5000000B: {name: name(Underscore), parent_scope: name_scope, index: -1, is_template: 0, is_unused: 1, form: inst} // CHECK:STDOUT: entity_name5000000C: {name: name(Underscore), parent_scope: name_scope, index: -1, is_template: 0, is_unused: 1, form: inst} -// CHECK:STDOUT: cpp_global_vars: {} // CHECK:STDOUT: functions: // CHECK:STDOUT: function50000000: {name: name4, parent_scope: name_scope0, call_param_patterns_id: inst_block50000018, call_params_id: inst_block50000019, body: [inst_block5000001F]} // CHECK:STDOUT: function50000001: {name: name7, parent_scope: name_scope0, call_param_patterns_id: inst_block50000028, call_params_id: inst_block50000029, body: [inst_block5000002F]} diff --git a/toolchain/check/testdata/basics/raw_sem_ir/one_file.carbon b/toolchain/check/testdata/basics/raw_sem_ir/one_file.carbon index 84375dee431c..a920855f7bbd 100644 --- a/toolchain/check/testdata/basics/raw_sem_ir/one_file.carbon +++ b/toolchain/check/testdata/basics/raw_sem_ir/one_file.carbon @@ -318,7 +318,6 @@ fn Foo[T:! type](p: T*) -> (T*, ()) { // CHECK:STDOUT: entity_name7800002B: {name: name6, parent_scope: name_scope, index: 2, is_template: 0, is_unused: 0, form: inst} // CHECK:STDOUT: entity_name7800002C: {name: name5, parent_scope: name_scope, index: 1, is_template: 0, is_unused: 0, form: inst} // CHECK:STDOUT: entity_name7800002D: {name: name1, parent_scope: name_scope, index: 0, is_template: 0, is_unused: 0, form: inst} -// CHECK:STDOUT: cpp_global_vars: {} // CHECK:STDOUT: functions: // CHECK:STDOUT: function78000000: {name: name0, parent_scope: name_scope0, call_param_patterns_id: inst_block78000010, call_params_id: inst_block78000011, return_type_inst_id: inst78000030, return_form_inst_id: inst78000032, return_pattern_id: inst78000038, body: [inst_block78000018]} // CHECK:STDOUT: function78000001: {name: name4, parent_scope: name_scope78000003, call_param_patterns_id: inst_block7800001E, return_type_inst_id: inst78000060, return_form_inst_id: inst78000061, return_pattern_id: inst7800005C} diff --git a/toolchain/check/testdata/basics/raw_sem_ir/one_file_with_textual_ir.carbon b/toolchain/check/testdata/basics/raw_sem_ir/one_file_with_textual_ir.carbon index f42749955bf1..60ecd55f6733 100644 --- a/toolchain/check/testdata/basics/raw_sem_ir/one_file_with_textual_ir.carbon +++ b/toolchain/check/testdata/basics/raw_sem_ir/one_file_with_textual_ir.carbon @@ -33,7 +33,6 @@ fn Foo(n: ()) -> ((), ()) { // CHECK:STDOUT: name_scope0: {inst: instF, parent_scope: name_scope, has_error: false, extended_scopes: [], names: {name0: inst5000002A}} // CHECK:STDOUT: entity_names: // CHECK:STDOUT: entity_name50000000: {name: name1, parent_scope: name_scope, index: -1, is_template: 0, is_unused: 0, form: inst} -// CHECK:STDOUT: cpp_global_vars: {} // CHECK:STDOUT: functions: // CHECK:STDOUT: function50000000: {name: name0, parent_scope: name_scope0, call_param_patterns_id: inst_block5000000B, call_params_id: inst_block5000000C, return_type_inst_id: inst50000020, return_form_inst_id: inst50000021, return_pattern_id: inst50000025, body: [inst_block5000000F]} // CHECK:STDOUT: classes: {} diff --git a/toolchain/driver/testdata/stdin.carbon b/toolchain/driver/testdata/stdin.carbon index aada3a4dcfb8..22160319ca72 100644 --- a/toolchain/driver/testdata/stdin.carbon +++ b/toolchain/driver/testdata/stdin.carbon @@ -36,7 +36,6 @@ // CHECK:STDOUT: name_scopes: // CHECK:STDOUT: name_scope0: {inst: instF, parent_scope: name_scope, has_error: false, extended_scopes: [], names: {}} // CHECK:STDOUT: entity_names: {} -// CHECK:STDOUT: cpp_global_vars: {} // CHECK:STDOUT: functions: {} // CHECK:STDOUT: classes: {} // CHECK:STDOUT: interfaces: {} diff --git a/toolchain/lower/file_context.cpp b/toolchain/lower/file_context.cpp index e8dcb0d6daa7..252e5620b7a3 100644 --- a/toolchain/lower/file_context.cpp +++ b/toolchain/lower/file_context.cpp @@ -714,10 +714,12 @@ auto FileContext::BuildDISubprogram(const SemIR::Function& function, auto FileContext::BuildGlobalVariableDecl(SemIR::VarStorage var_storage) -> llvm::Constant* { - // When a Carbon variable is exported and used from C++, code + // Check if an llvm::GlobalVariable already exists and use it if so. + // + // This happens for C++ variables imported into Carbon. It also + // happens when a Carbon variable is exported and used from C++; code // generation for the C++ code may have already created an - // llvm::GlobalVariable. If so, return that global rather than - // creating a new one. + // llvm::GlobalVariable. auto clang_decl_id = sem_ir().clang_decls().Lookup(var_storage.pattern_id); if (clang_decl_id.has_value()) { auto* decl = sem_ir().clang_decls().Get(clang_decl_id).key.decl; @@ -729,21 +731,6 @@ auto FileContext::BuildGlobalVariableDecl(SemIR::VarStorage var_storage) } } - auto var_name_id = - SemIR::GetFirstBindingNameFromPatternId(sem_ir(), var_storage.pattern_id); - if (auto cpp_global_var_id = - sem_ir().cpp_global_vars().Lookup({.entity_name_id = var_name_id}); - cpp_global_var_id.has_value()) { - SemIR::ClangDeclId clang_decl_id = - sem_ir().cpp_global_vars().Get(cpp_global_var_id).clang_decl_id; - CARBON_CHECK(clang_decl_id.has_value(), - "CppGlobalVar should have a clang_decl_id"); - return cpp_code_generator_->GetAddrOfGlobal( - cast( - sem_ir().clang_decls().Get(clang_decl_id).key.decl), - /*isForDefinition=*/false); - } - return BuildNonCppGlobalVariableDecl(var_storage); } diff --git a/toolchain/sem_ir/BUILD b/toolchain/sem_ir/BUILD index 40abe2f60c40..2ac528de5a8b 100644 --- a/toolchain/sem_ir/BUILD +++ b/toolchain/sem_ir/BUILD @@ -88,7 +88,6 @@ cc_library( "class.cpp", "constant.cpp", "core_interface.cpp", - "cpp_global_var.cpp", "cpp_initializer_list.cpp", "cpp_overload_set.cpp", "facet_type_info.cpp", @@ -120,7 +119,6 @@ cc_library( "constant.h", "copy_on_write_block.h", "core_interface.h", - "cpp_global_var.h", "cpp_initializer_list.h", "cpp_overload_set.h", "entity_name.h", diff --git a/toolchain/sem_ir/clang_decl.cpp b/toolchain/sem_ir/clang_decl.cpp index 097e65c672a7..eaa443894dc4 100644 --- a/toolchain/sem_ir/clang_decl.cpp +++ b/toolchain/sem_ir/clang_decl.cpp @@ -81,11 +81,19 @@ auto ClangDecl::Print(llvm::raw_ostream& out) const -> void { ClangDeclStore::ClangDeclStore(CheckIRId check_ir_id) : values_(check_ir_id) {} auto ClangDeclStore::Add(ClangDecl value) -> ClangDeclId { + CARBON_CHECK(!isa(value.key.decl)); auto id = values_.Add(value); inst_id_to_clang_decl_id_.Insert(value.inst_id, id); return id; } +auto ClangDeclStore::AddVar(ClangDecl value, InstId pattern_id) -> ClangDeclId { + CARBON_CHECK(isa(value.key.decl)); + auto id = values_.Add(value); + inst_id_to_clang_decl_id_.Insert(pattern_id, id); + return id; +} + auto ClangDeclStore::Lookup(ClangDeclKey key) const -> ClangDeclId { return values_.Lookup(key); } diff --git a/toolchain/sem_ir/clang_decl.h b/toolchain/sem_ir/clang_decl.h index 6fdc2796810a..29fa6a5835ba 100644 --- a/toolchain/sem_ir/clang_decl.h +++ b/toolchain/sem_ir/clang_decl.h @@ -196,6 +196,18 @@ class ClangDeclStore { // Adds a `ClangDecl`, returning an ID to reference it. auto Add(ClangDecl value) -> ClangDeclId; + // Same as `Add`, but for `VarStorage` that maps to a `clang::VarDecl`. + // + // When looking up via `InstId`, the pattern's `InstId` must be used + // instead of the `InstId` corresponding to the `VarStorage`. Note however + // that the `value.inst_id` is still the `VarStorage` `InstId`. + // + // The pattern's `InstId` is used because it provides a more stable + // lookup key than the `VarStorage` `InstId`. For example, a call to + // `Convert` may cause a new `VarStorage` instruction to be created, + // but the pattern will remain the same. + auto AddVar(ClangDecl value, InstId pattern_id) -> ClangDeclId; + // Looks up a `ClangDecl` by `ClangDeclId`. auto Get(ClangDeclId id) const -> const ClangDecl& { return values_.Get(id); } diff --git a/toolchain/sem_ir/cpp_global_var.cpp b/toolchain/sem_ir/cpp_global_var.cpp deleted file mode 100644 index b4738ab28011..000000000000 --- a/toolchain/sem_ir/cpp_global_var.cpp +++ /dev/null @@ -1,16 +0,0 @@ -// 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 "toolchain/sem_ir/cpp_global_var.h" - -#include "toolchain/base/canonical_value_store_impl.h" -#include "toolchain/base/value_store_impl.h" - -namespace Carbon { -template class CanonicalValueStore, SemIR::CppGlobalVar>; -template class ValueStore>; -} // namespace Carbon diff --git a/toolchain/sem_ir/cpp_global_var.h b/toolchain/sem_ir/cpp_global_var.h deleted file mode 100644 index 078db0a27261..000000000000 --- a/toolchain/sem_ir/cpp_global_var.h +++ /dev/null @@ -1,65 +0,0 @@ -// 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 - -#ifndef CARBON_TOOLCHAIN_SEM_IR_CPP_GLOBAL_VAR_H_ -#define CARBON_TOOLCHAIN_SEM_IR_CPP_GLOBAL_VAR_H_ - -#include "common/hashing.h" -#include "common/ostream.h" -#include "toolchain/sem_ir/clang_decl.h" -#include "toolchain/sem_ir/ids.h" - -namespace Carbon::SemIR { - -// A key describing a C++ global variable imported into Carbon, identified by -// its entity name. -struct CppGlobalVarKey : public Printable { - auto Print(llvm::raw_ostream& out) const -> void { - out << "{entity_name_id: " << entity_name_id << "}"; - } - - // TODO: Use default when `Printable` supports it. - friend auto operator==(const CppGlobalVarKey& lhs, const CppGlobalVarKey& rhs) - -> bool { - return lhs.entity_name_id == rhs.entity_name_id; - } - - // The name of the variable. - EntityNameId entity_name_id; -}; - -// A C++ global variable imported into Carbon. This is used to map the entity -// name to the Clang declaration so we can use Clang mangling. -struct CppGlobalVar : public Printable { - auto Print(llvm::raw_ostream& out) const -> void { - out << "{key: " << key << ", clang_decl_id: " << clang_decl_id << "}"; - } - - // The key by which this variable can be looked up. - CppGlobalVarKey key; - - // The Clang declaration for this variable, if any. - // This is ignored for equality and hashing, since it's always unique for a - // given key, in order to store it in `CanonicalValueStore` and allow lookup - // by `CppGlobalVarKey`. - ClangDeclId clang_decl_id; - - auto GetAsKey() const -> CppGlobalVarKey { return key; } -}; - -// Use the name of a C++ global variable when doing `Lookup` to find an ID. -using CppGlobalVarStore = CanonicalValueStore, CppGlobalVar>; - -} // namespace Carbon::SemIR - -namespace Carbon { -extern template class CanonicalValueStore< - SemIR::CppGlobalVarId, SemIR::CppGlobalVarKey, Tag, - SemIR::CppGlobalVar>; -extern template class ValueStore>; -} // namespace Carbon - -#endif // CARBON_TOOLCHAIN_SEM_IR_CPP_GLOBAL_VAR_H_ diff --git a/toolchain/sem_ir/file.cpp b/toolchain/sem_ir/file.cpp index 948664d8fea3..1d00e3c68d4f 100644 --- a/toolchain/sem_ir/file.cpp +++ b/toolchain/sem_ir/file.cpp @@ -40,7 +40,6 @@ File::File(const Parse::Tree* parse_tree, CheckIRId check_ir_id, value_stores_(&value_stores), filename_(std::move(filename)), entity_names_(check_ir_id), - cpp_global_vars_(check_ir_id), functions_(check_ir_id), cpp_overload_sets_(check_ir_id), thunks_(check_ir_id), @@ -159,7 +158,6 @@ auto File::OutputYaml(bool include_singletons) const -> Yaml::OutputMapping { map.Add("clang_decl_signatures", clang_decl_signatures_.OutputYaml()); map.Add("name_scopes", name_scopes_.OutputYaml()); map.Add("entity_names", entity_names_.OutputYaml()); - map.Add("cpp_global_vars", cpp_global_vars_.OutputYaml()); map.Add("functions", functions_.OutputYaml()); map.Add("classes", classes_.OutputYaml()); map.Add("interfaces", interfaces_.OutputYaml()); @@ -195,8 +193,6 @@ auto File::CollectMemUsage(MemUsage& mem_usage, llvm::StringRef label) const mem_usage.Collect(MemUsage::ConcatLabel(label, "allocator_"), allocator_); mem_usage.Collect(MemUsage::ConcatLabel(label, "entity_names_"), entity_names_); - mem_usage.Collect(MemUsage::ConcatLabel(label, "cpp_global_vars_"), - cpp_global_vars_); mem_usage.Collect(MemUsage::ConcatLabel(label, "functions_"), functions_); mem_usage.Collect(MemUsage::ConcatLabel(label, "thunks_"), thunks_); mem_usage.Collect(MemUsage::ConcatLabel(label, "classes_"), classes_); diff --git a/toolchain/sem_ir/file.h b/toolchain/sem_ir/file.h index 0db52bfc333c..c6a23e0298af 100644 --- a/toolchain/sem_ir/file.h +++ b/toolchain/sem_ir/file.h @@ -23,7 +23,6 @@ #include "toolchain/sem_ir/class.h" #include "toolchain/sem_ir/constant.h" #include "toolchain/sem_ir/cpp_file.h" -#include "toolchain/sem_ir/cpp_global_var.h" #include "toolchain/sem_ir/cpp_overload_set.h" #include "toolchain/sem_ir/entity_name.h" #include "toolchain/sem_ir/facet_type_info.h" @@ -165,10 +164,6 @@ class File : public Printable { auto entity_names() -> EntityNameStore& { return entity_names_; } auto entity_names() const -> const EntityNameStore& { return entity_names_; } - auto cpp_global_vars() -> CppGlobalVarStore& { return cpp_global_vars_; } - auto cpp_global_vars() const -> const CppGlobalVarStore& { - return cpp_global_vars_; - } auto functions() -> FunctionStore& { return functions_; } auto functions() const -> const FunctionStore& { return functions_; } auto cpp_overload_sets() -> CppOverloadSetStore& { @@ -353,9 +348,6 @@ class File : public Printable { // Storage for EntityNames. EntityNameStore entity_names_; - // For imported C++ global variables, the Clang decl to use for mangling. - CppGlobalVarStore cpp_global_vars_; - // Storage for callable objects. FunctionStore functions_; diff --git a/toolchain/sem_ir/ids.h b/toolchain/sem_ir/ids.h index 6862641899ae..7dc70ed38821 100644 --- a/toolchain/sem_ir/ids.h +++ b/toolchain/sem_ir/ids.h @@ -266,13 +266,6 @@ struct EntityNameId : public IdBase { using IdBase::IdBase; }; -// The ID of a C++ global variable. -struct CppGlobalVarId : public IdBase { - static constexpr llvm::StringLiteral Label = "cpp_global_var"; - - using IdBase::IdBase; -}; - // The index of a compile-time binding. This is the de Bruijn level for the // binding -- that is, this is the number of other compile time bindings whose // scope encloses this binding. diff --git a/toolchain/sem_ir/mangler.cpp b/toolchain/sem_ir/mangler.cpp index 2de04cf23422..1058ecd69bc6 100644 --- a/toolchain/sem_ir/mangler.cpp +++ b/toolchain/sem_ir/mangler.cpp @@ -276,11 +276,11 @@ auto Mangler::MangleGlobalVariable(SemIR::InstId pattern_id) -> std::string { return std::string(); } - CARBON_CHECK(!sem_ir() - .cpp_global_vars() - .Lookup({.entity_name_id = var_name_id}) - .has_value(), - "Mangling a C++ variable"); + auto clang_decl_id = sem_ir().clang_decls().Lookup(pattern_id); + if (clang_decl_id.has_value()) { + CARBON_CHECK(!sem_ir().clang_decls().Get(clang_decl_id).is_imported, + "Mangling a C++ variable"); + } RawStringOstream os; os << "_C"; diff --git a/toolchain/sem_ir/yaml_test.cpp b/toolchain/sem_ir/yaml_test.cpp index f777848edd83..0b399d9b6c81 100644 --- a/toolchain/sem_ir/yaml_test.cpp +++ b/toolchain/sem_ir/yaml_test.cpp @@ -64,7 +64,6 @@ TEST(SemIRTest, Yaml) { Pair("clang_decl_signatures", Yaml::Mapping(SizeIs(0))), Pair("name_scopes", Yaml::Mapping(SizeIs(1))), Pair("entity_names", Yaml::Mapping(SizeIs(1))), - Pair("cpp_global_vars", Yaml::Mapping(SizeIs(0))), Pair("functions", Yaml::Mapping(SizeIs(1))), Pair("classes", Yaml::Mapping(SizeIs(0))), Pair("interfaces", Yaml::Mapping(SizeIs(0))),