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 <richard@metafoo.co.uk>

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();
   }

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<clang::VarDecl>(
@@ -961,6 +959,7 @@ auto ExportVarToCpp(Context& context, SemIR::LocId loc_id,
                scope_inst.Is<SemIR::ClassDecl>());
 
   // 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<SemIR::ClassDecl>()) {
     // TODO: Map Carbon access to C++ access.

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

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;

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<SemIR::VarStorage>(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<clang::VarDecl>(
           context.clang_decls().Get(clang_decl_id).key.decl);
     }

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<none>, has_error: false, extended_scopes: [], names: {}}
 // CHECK:STDOUT:   entity_names:    {}
-// CHECK:STDOUT:   cpp_global_vars: {}
 // CHECK:STDOUT:   functions:       {}
 // CHECK:STDOUT:   classes:         {}
 // CHECK:STDOUT:   interfaces:      {}

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<none>, index: -1, is_template: 0, is_unused: 0, form: inst<none>}
 // CHECK:STDOUT:     entity_name70000001: {name: name1, parent_scope: name_scope70000001, index: -1, is_template: 0, is_unused: 0, form: inst<none>}
 // CHECK:STDOUT:     entity_name70000002: {name: name1, parent_scope: name_scope<none>, index: 0, is_template: 0, is_unused: 0, form: inst<none>}
-// 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}

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<none>, index: -1, is_template: 0, is_unused: 0, form: inst<none>}
 // CHECK:STDOUT:     entity_name50000002: {name: name1, parent_scope: name_scope<none>, index: -1, is_template: 0, is_unused: 0, form: inst<none>}
 // CHECK:STDOUT:     entity_name50000003: {name: name4, parent_scope: name_scope50000001, index: -1, is_template: 0, is_unused: 0, form: inst<none>}
-// 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}

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<none>, 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<none>}
-// 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}

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<none>, 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<none>}
-// 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}

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<none>}
-// 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<none>, index: 0, is_template: 0, is_unused: 0, form: inst<none>}
 // CHECK:STDOUT:     entity_name5000000B: {name: name(Underscore), parent_scope: name_scope<none>, index: -1, is_template: 0, is_unused: 1, form: inst<none>}
 // CHECK:STDOUT:     entity_name5000000C: {name: name(Underscore), parent_scope: name_scope<none>, index: -1, is_template: 0, is_unused: 1, form: inst<none>}
-// 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]}

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<none>, index: 2, is_template: 0, is_unused: 0, form: inst<none>}
 // CHECK:STDOUT:     entity_name7800002C: {name: name5, parent_scope: name_scope<none>, index: 1, is_template: 0, is_unused: 0, form: inst<none>}
 // CHECK:STDOUT:     entity_name7800002D: {name: name1, parent_scope: name_scope<none>, index: 0, is_template: 0, is_unused: 0, form: inst<none>}
-// 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}

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<none>, has_error: false, extended_scopes: [], names: {name0: inst5000002A}}
 // CHECK:STDOUT:   entity_names:
 // CHECK:STDOUT:     entity_name50000000: {name: name1, parent_scope: name_scope<none>, index: -1, is_template: 0, is_unused: 0, form: inst<none>}
-// 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:         {}

toolchain/driver/testdata/stdin.carbon

@@ -36,7 +36,6 @@
 // CHECK:STDOUT:   name_scopes:
 // CHECK:STDOUT:     name_scope0:     {inst: instF, parent_scope: name_scope<none>, has_error: false, extended_scopes: [], names: {}}
 // CHECK:STDOUT:   entity_names:    {}
-// CHECK:STDOUT:   cpp_global_vars: {}
 // CHECK:STDOUT:   functions:       {}
 // CHECK:STDOUT:   classes:         {}
 // CHECK:STDOUT:   interfaces:      {}

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<clang::VarDecl>(
-            sem_ir().clang_decls().Get(clang_decl_id).key.decl),
-        /*isForDefinition=*/false);
-  }
-
   return BuildNonCppGlobalVariableDecl(var_storage);
 }
 

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",

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<clang::VarDecl>(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<clang::VarDecl>(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);
 }

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); }
 

toolchain/sem_ir/cpp_global_var.cpp

@@ -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::CppGlobalVarId,
-                                   SemIR::CppGlobalVarKey,
-                                   Tag<SemIR::CheckIRId>, SemIR::CppGlobalVar>;
-template class ValueStore<SemIR::CppGlobalVarId, SemIR::CppGlobalVar,
-                          Tag<SemIR::CheckIRId>>;
-}  // namespace Carbon

toolchain/sem_ir/cpp_global_var.h

@@ -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<CppGlobalVarKey> {
-  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<CppGlobalVar> {
-  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<CppGlobalVarId, CppGlobalVarKey,
-                                              Tag<CheckIRId>, CppGlobalVar>;
-
-}  // namespace Carbon::SemIR
-
-namespace Carbon {
-extern template class CanonicalValueStore<
-    SemIR::CppGlobalVarId, SemIR::CppGlobalVarKey, Tag<SemIR::CheckIRId>,
-    SemIR::CppGlobalVar>;
-extern template class ValueStore<SemIR::CppGlobalVarId, SemIR::CppGlobalVar,
-                                 Tag<SemIR::CheckIRId>>;
-}  // namespace Carbon
-
-#endif  // CARBON_TOOLCHAIN_SEM_IR_CPP_GLOBAL_VAR_H_

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_);

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<File> {
 
   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<File> {
   // 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_;
 

toolchain/sem_ir/ids.h

@@ -266,13 +266,6 @@ struct EntityNameId : public IdBase<EntityNameId> {
   using IdBase::IdBase;
 };
 
-// The ID of a C++ global variable.
-struct CppGlobalVarId : public IdBase<CppGlobalVarId> {
-  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.

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";

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))),