From 198e447e4accc1aed098b64754598808fb974422 Mon Sep 17 00:00:00 2001 From: Nicholas Bishop Date: Wed, 29 Jul 2026 15:53:12 -0400 Subject: [PATCH] Add specific_id and pattern_inst_id to ClangDecl (#7583) Exporting class fields in class specifics will require looking up `ClangDecl`s by the field's `InstId` and the class's `SpecificId`. Add the `specific_id` to ClangeDecl, and rework the reverse lookup to use a `Set` with a `KeyContext` rather than a `Map`. The `Lookup` method now takes an optional `SpecificId` argument, although currently it is always `None`. For `VarStorage`, reverse lookup is performed by the pattern `InstId` rather than the `InstId` of the `VarStorage` itself, so also add `pattern_inst_id` to `ClangDecl`, and provide a separate `LookupByPatternInstId` method for reverse lookups. For this lookup, the `inst_id` part of the key is set to `None`, so only the pattern's `InstId` is used for lookup. --- toolchain/check/cpp/export.cpp | 6 +-- toolchain/check/cpp/import.cpp | 15 ++++-- .../basics/raw_sem_ir/cpp_interop.carbon | 2 +- toolchain/sem_ir/BUILD | 1 + toolchain/sem_ir/clang_decl.cpp | 49 +++++++++++++------ toolchain/sem_ir/clang_decl.h | 35 +++++++------ 6 files changed, 69 insertions(+), 39 deletions(-) diff --git a/toolchain/check/cpp/export.cpp b/toolchain/check/cpp/export.cpp index f9b27a9776d4..20cc8e51d00c 100644 --- a/toolchain/check/cpp/export.cpp +++ b/toolchain/check/cpp/export.cpp @@ -1439,10 +1439,10 @@ auto ExportVarToCpp(Context& context, SemIR::InstId inst_id, context.ast_context(), decl_context, /*StartLoc=*/clang_loc, /*IdLoc=*/clang_loc, identifier_info, cpp_type, /*TInfo=*/nullptr, clang::SC_Extern); - context.clang_decls().AddVar( + context.clang_decls().Add( {.key = SemIR::ClangDeclKey::ForNonFunctionDecl(var_decl), - .inst_id = inst_id}, - var_storage.pattern_id); + .inst_id = var_storage.pattern_id, + .var_storage_inst_id = inst_id}); if (scope_inst.Is()) { SetCppClassMemberAccess(name_scope, entity_name.name_id, var_decl); diff --git a/toolchain/check/cpp/import.cpp b/toolchain/check/cpp/import.cpp index b24d2761c338..cceed53aa963 100644 --- a/toolchain/check/cpp/import.cpp +++ b/toolchain/check/cpp/import.cpp @@ -441,7 +441,12 @@ static auto LookupClangDeclInstId(Context& context, SemIR::ClangDeclKey key) const auto& clang_decls = context.clang_decls(); if (auto context_clang_decl_id = clang_decls.LookupId(key); context_clang_decl_id.has_value()) { - return clang_decls.Get(context_clang_decl_id).inst_id; + const auto& clang_decl = clang_decls.Get(context_clang_decl_id); + if (clang_decl.var_storage_inst_id.has_value()) { + return clang_decl.var_storage_inst_id; + } else { + return clang_decls.Get(context_clang_decl_id).inst_id; + } } return SemIR::InstId::None; } @@ -2143,10 +2148,10 @@ static auto ImportVarDecl(Context& context, SemIR::LocId loc_id, context.imports().push_back(var_storage_inst_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); + context.clang_decls().Add({.key = SemIR::ClangDeclKey(var_decl), + .inst_id = pattern_id, + .var_storage_inst_id = var_storage_inst_id, + .is_imported = true}); // Inform Clang that the variable has been referenced. context.clang_sema().MarkVariableReferenced(GetCppLocation(context, loc_id), 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 e219faaf7647..703f76bf609a 100644 --- a/toolchain/check/testdata/basics/raw_sem_ir/cpp_interop.carbon +++ b/toolchain/check/testdata/basics/raw_sem_ir/cpp_interop.carbon @@ -61,7 +61,7 @@ fn G(x: Cpp.X) { // CHECK:STDOUT: clang_decl_id50000005: {key: {decl: "inline void f__carbon_thunk()", clang_decl_signature_id: clang_decl_signature_id50000000}, inst_id: inst50000032} // CHECK:STDOUT: clang_decl_id50000006: {key: {decl: "void f(X x = {})", clang_decl_signature_id: clang_decl_signature_id50000001}, inst_id: inst5000003D} // CHECK:STDOUT: clang_decl_id50000007: {key: {decl: "inline void f__carbon_thunk(X * _Nonnull x)", clang_decl_signature_id: clang_decl_signature_id50000001}, inst_id: inst50000047} -// CHECK:STDOUT: clang_decl_id50000008: {key: {decl: "X * _Nonnull global"}, inst_id: inst50000054} +// CHECK:STDOUT: clang_decl_id50000008: {key: {decl: "X * _Nonnull global"}, inst_id: inst50000052} // CHECK:STDOUT: clang_decl_signatures: // CHECK:STDOUT: clang_decl_signature_id50000000: {kind: normal, num_params: 0} // CHECK:STDOUT: clang_decl_signature_id50000001: {kind: normal, num_params: 1, modes: [value]} diff --git a/toolchain/sem_ir/BUILD b/toolchain/sem_ir/BUILD index 06fc7c26373c..f83e2c076a4d 100644 --- a/toolchain/sem_ir/BUILD +++ b/toolchain/sem_ir/BUILD @@ -58,6 +58,7 @@ cc_library( "//common:hashtable_key_context", "//common:ostream", "//common:raw_string_ostream", + "//common:set", "//toolchain/base:canonical_value_store", "//toolchain/base:value_store", "@llvm-project//clang:ast", diff --git a/toolchain/sem_ir/clang_decl.cpp b/toolchain/sem_ir/clang_decl.cpp index 6c96abae11ae..863ec96acc65 100644 --- a/toolchain/sem_ir/clang_decl.cpp +++ b/toolchain/sem_ir/clang_decl.cpp @@ -6,6 +6,7 @@ #include "clang/AST/DeclBase.h" #include "clang/AST/TextNodeDumper.h" +#include "common/hashtable_key_context.h" #include "common/ostream.h" #include "common/raw_string_ostream.h" #include "toolchain/base/canonical_value_store_impl.h" @@ -93,19 +94,36 @@ auto ClangDecl::Print(llvm::raw_ostream& out) const -> void { out << "{key: " << key << ", inst_id: " << inst_id << "}"; } +class ClangDeclStore::KeyContext : public TranslatingKeyContext { + public: + // A lookup key for a clang declaration. + struct Key { + InstId inst_id; + SpecificId specific_id; + + friend auto operator==(const Key&, const Key&) -> bool = default; + }; + + explicit KeyContext(const ClangDeclStore* store) : store_(store) {} + + auto TranslateKey(ClangDeclId id) const -> Key { + const auto& clang_decl = store_->Get(id); + return {.inst_id = clang_decl.inst_id, + .specific_id = clang_decl.specific_id}; + } + + private: + const ClangDeclStore* store_; +}; + ClangDeclStore::ClangDeclStore(CheckIRId check_ir_id) : values_(check_ir_id) {} auto ClangDeclStore::Add(ClangDecl value) -> ClangDeclId { - CARBON_CHECK(!isa(value.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.decl())); - auto id = values_.Add(value); - inst_id_to_clang_decl_id_.Insert(pattern_id, id); + reverse_lookup_.Insert( + KeyContext::Key{.inst_id = value.inst_id, + .specific_id = value.specific_id}, + [&] { return id; }, KeyContext(this)); return id; } @@ -113,9 +131,12 @@ auto ClangDeclStore::LookupId(ClangDeclKey key) const -> ClangDeclId { return values_.Lookup(key); } -auto ClangDeclStore::Lookup(InstId inst_id) const -> const ClangDecl* { - if (auto result = inst_id_to_clang_decl_id_.Lookup(inst_id)) { - return &Get(result.value()); +auto ClangDeclStore::Lookup(InstId inst_id, SpecificId specific_id) const + -> const ClangDecl* { + if (auto result = reverse_lookup_.Lookup( + KeyContext::Key{.inst_id = inst_id, .specific_id = specific_id}, + KeyContext(this))) { + return &Get(result.key()); } return nullptr; } @@ -127,8 +148,8 @@ auto ClangDeclStore::OutputYaml() const -> Yaml::OutputMapping { auto ClangDeclStore::CollectMemUsage(MemUsage& mem_usage, llvm::StringRef label) const -> void { values_.CollectMemUsage(mem_usage, label); - mem_usage.Collect(MemUsage::ConcatLabel(label, "inst_id_to_clang_decl_id_"), - inst_id_to_clang_decl_id_); + mem_usage.Collect(MemUsage::ConcatLabel(label, "reverse_lookup_"), + reverse_lookup_, KeyContext(this)); } } // namespace Carbon::SemIR diff --git a/toolchain/sem_ir/clang_decl.h b/toolchain/sem_ir/clang_decl.h index 7179eccb06ea..da765e34cd44 100644 --- a/toolchain/sem_ir/clang_decl.h +++ b/toolchain/sem_ir/clang_decl.h @@ -9,6 +9,7 @@ #include "common/hashtable_key_context.h" #include "common/ostream.h" +#include "common/set.h" #include "toolchain/base/canonical_value_store.h" #include "toolchain/sem_ir/ids.h" @@ -178,6 +179,16 @@ struct ClangDecl : public Printable { // The instruction the Clang declaration is mapped to. InstId inst_id; + // The specific the Clang declaration is mapped to. + SpecificId specific_id = SpecificId::None; + + // When exporting a `VarStorage`, its `InstId` is needed in some cases, but + // its pattern is used as the primary `inst_id`. The pattern 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. + InstId var_storage_inst_id = InstId::None; + // True if this declaration originated from C++. False if this declaration was // created by exporting some Carbon declaration to C++. bool is_imported = false; @@ -197,26 +208,16 @@ 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); } // Looks up a `ClangDeclId` by `ClangDeclKey`. auto LookupId(ClangDeclKey key) const -> ClangDeclId; - // Looks up a `ClangDecl` by `InstId`. Returns nullptr if not found. - auto Lookup(InstId inst_id) const -> const ClangDecl*; + // Looks up a `ClangDecl` by `InstId` and optional `SpecificId`. Returns + // nullptr if not found. + auto Lookup(InstId inst_id, SpecificId specific_id = SpecificId::None) const + -> const ClangDecl*; auto OutputYaml() const -> Yaml::OutputMapping; @@ -224,13 +225,15 @@ class ClangDeclStore { -> void; private: + class KeyContext; + // Canonical storage for `ClangDecl`s. Allows mapping from a // `ClangDeclId` to an `InstId`. CanonicalValueStore, ClangDecl> values_; - // Map from `InstId` to `ClangDeclId`. - Map inst_id_to_clang_decl_id_; + // Provides lookup by `InstId` and `SpecificId`. + Set reverse_lookup_; }; // A ClangDeclSignature mapped to an ID.