From 7f11012f5879c5670fdff7fed7efdc13c5f044fd Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Mon, 29 Jan 2024 15:05:47 -0800 Subject: [PATCH] CrossRefIRId -> ImportIRId (#3662) One more (hopefully last) rename on the Import instruction renaming. I was kind of tempted to rename to just "IRId", since the IRs aren't all imports. However, this felt easier to read, and a better choice than CrossRef because it's more consistent with the other ways imports exist in code. (even if IRs aren't all imports, most use-cases are derived from imports) Note though that import_irs may include IRs not just from direct imports. Beyond the builtin IR, I'm thinking that for indirect imports, or the prelude, we may end up adding them. e.g., so that constants can be generated for indirect imports and still correspond to a directly known IR, and for a given IR that's indirectly imported multiple times to be deduplicated locally. I'm not there yet, I'm just mentioning this to help give background for naming thoughts. --- toolchain/check/check.cpp | 2 +- toolchain/check/context.cpp | 16 ++++++++-------- toolchain/check/context.h | 4 ++-- toolchain/check/import.cpp | 6 +++--- .../check/testdata/basics/builtin_insts.carbon | 2 +- .../basics/multifile_raw_and_textual_ir.carbon | 4 ++-- .../testdata/basics/multifile_raw_ir.carbon | 4 ++-- .../testdata/basics/raw_and_textual_ir.carbon | 2 +- toolchain/check/testdata/basics/raw_ir.carbon | 2 +- toolchain/sem_ir/file.cpp | 17 ++++++++--------- toolchain/sem_ir/file.h | 13 ++++++------- toolchain/sem_ir/formatter.cpp | 2 +- toolchain/sem_ir/ids.h | 8 ++++---- toolchain/sem_ir/typed_insts.h | 14 +++++++------- toolchain/sem_ir/yaml_test.cpp | 2 +- 15 files changed, 48 insertions(+), 50 deletions(-) diff --git a/toolchain/check/check.cpp b/toolchain/check/check.cpp index 17fcc5296c9c..5052b9795c8d 100644 --- a/toolchain/check/check.cpp +++ b/toolchain/check/check.cpp @@ -53,7 +53,7 @@ class SemIRLocationTranslator // possible. if (auto import_ref = cursor_ir->insts().TryGetAs( cursor_inst_id)) { - cursor_ir = cursor_ir->cross_ref_irs().Get(import_ref->ir_id); + cursor_ir = cursor_ir->import_irs().Get(import_ref->ir_id); cursor_inst_id = import_ref->inst_id; continue; } diff --git a/toolchain/check/context.cpp b/toolchain/check/context.cpp index 6a13adc4e1a7..db1fd0471fd2 100644 --- a/toolchain/check/context.cpp +++ b/toolchain/check/context.cpp @@ -174,20 +174,20 @@ auto Context::AddPackageImports(Parse::NodeId import_node, auto name_id = SemIR::NameId::ForIdentifier(package_id); - SemIR::CrossRefIRId first_id(cross_ref_irs().size()); + SemIR::ImportIRId first_id(import_irs().size()); for (const auto* sem_ir : sem_irs) { - cross_ref_irs().Add(sem_ir); + import_irs().Add(sem_ir); } if (has_load_error) { - cross_ref_irs().Add(nullptr); + import_irs().Add(nullptr); } - SemIR::CrossRefIRId last_id(cross_ref_irs().size() - 1); + SemIR::ImportIRId last_id(import_irs().size() - 1); auto type_id = GetBuiltinType(SemIR::BuiltinKind::NamespaceType); auto inst_id = AddInst({import_node, SemIR::Import{.type_id = type_id, - .first_cross_ref_ir_id = first_id, - .last_cross_ref_ir_id = last_id}}); + .first_import_ir_id = first_id, + .last_import_ir_id = last_id}}); // Add the import to lookup. Should always succeed because imports will be // uniquely named. @@ -225,7 +225,7 @@ auto Context::ResolveIfImportRefUnused(SemIR::InstId inst_id) -> void { if (!unused_inst) { return; } - const SemIR::File& import_ir = *cross_ref_irs().Get(unused_inst->ir_id); + const SemIR::File& import_ir = *import_irs().Get(unused_inst->ir_id); auto import_inst = import_ir.insts().Get(unused_inst->inst_id); switch (import_inst.kind()) { default: @@ -798,7 +798,7 @@ class TypeCompleter { << "TODO: Handle non-builtin ImportRefUsed cases, such as functions, " "classes, and interfaces"; - const auto& import_ir = context_.cross_ref_irs().Get(import_ref.ir_id); + const auto& import_ir = context_.import_irs().Get(import_ref.ir_id); auto import_inst = import_ir->insts().Get(import_ref.inst_id); switch (import_inst.As().builtin_kind) { diff --git a/toolchain/check/context.h b/toolchain/check/context.h index 22f274f9e60d..10df14b93dfc 100644 --- a/toolchain/check/context.h +++ b/toolchain/check/context.h @@ -425,8 +425,8 @@ class Context { auto interfaces() -> ValueStore& { return sem_ir().interfaces(); } - auto cross_ref_irs() -> ValueStore& { - return sem_ir().cross_ref_irs(); + auto import_irs() -> ValueStore& { + return sem_ir().import_irs(); } auto names() -> SemIR::NameStoreWrapper { return sem_ir().names(); } auto name_scopes() -> SemIR::NameScopeStore& { diff --git a/toolchain/check/import.cpp b/toolchain/check/import.cpp index 2ccebf195dc7..bdb12c6d4353 100644 --- a/toolchain/check/import.cpp +++ b/toolchain/check/import.cpp @@ -85,7 +85,7 @@ static auto CacheCopiedNamespace( static auto CopySingleNameScopeFromImportIR( Context& context, llvm::DenseMap& copied_namespaces, - SemIR::CrossRefIRId ir_id, SemIR::InstId import_inst_id, + SemIR::ImportIRId ir_id, SemIR::InstId import_inst_id, SemIR::NameScopeId import_scope_id, SemIR::NameScopeId enclosing_scope_id, SemIR::NameId name_id, SemIR::TypeId namespace_type_id) -> SemIR::NameScopeId { @@ -132,7 +132,7 @@ static auto CopySingleNameScopeFromImportIR( // import_enclosing_scope_id. static auto CopyEnclosingNameScopesFromImportIR( Context& context, SemIR::TypeId namespace_type_id, - const SemIR::File& import_sem_ir, SemIR::CrossRefIRId ir_id, + const SemIR::File& import_sem_ir, SemIR::ImportIRId ir_id, SemIR::NameScopeId import_enclosing_scope_id, llvm::DenseMap& copied_namespaces) -> SemIR::NameScopeId { @@ -181,7 +181,7 @@ static auto CopyEnclosingNameScopesFromImportIR( auto Import(Context& context, SemIR::TypeId namespace_type_id, const SemIR::File& import_sem_ir) -> void { - auto ir_id = context.cross_ref_irs().Add(&import_sem_ir); + auto ir_id = context.import_irs().Add(&import_sem_ir); for (const auto import_inst_id : import_sem_ir.inst_blocks().Get(SemIR::InstBlockId::Exports)) { diff --git a/toolchain/check/testdata/basics/builtin_insts.carbon b/toolchain/check/testdata/basics/builtin_insts.carbon index db1e4070d6dc..c9bce5cb8534 100644 --- a/toolchain/check/testdata/basics/builtin_insts.carbon +++ b/toolchain/check/testdata/basics/builtin_insts.carbon @@ -9,7 +9,7 @@ // CHECK:STDOUT: --- // CHECK:STDOUT: filename: builtin_insts.carbon // CHECK:STDOUT: sem_ir: -// CHECK:STDOUT: cross_ref_irs_size: 1 +// CHECK:STDOUT: import_irs_size: 1 // CHECK:STDOUT: name_scopes: // CHECK:STDOUT: name_scope0: {inst: inst+0, enclosing_scope: name_scope, has_error: false, extended_scopes: [], names: {}} // CHECK:STDOUT: bind_names: {} diff --git a/toolchain/check/testdata/basics/multifile_raw_and_textual_ir.carbon b/toolchain/check/testdata/basics/multifile_raw_and_textual_ir.carbon index 9182dc0d34dc..7da96c76fc39 100644 --- a/toolchain/check/testdata/basics/multifile_raw_and_textual_ir.carbon +++ b/toolchain/check/testdata/basics/multifile_raw_and_textual_ir.carbon @@ -21,7 +21,7 @@ fn B() {} // CHECK:STDOUT: --- // CHECK:STDOUT: filename: a.carbon // CHECK:STDOUT: sem_ir: -// CHECK:STDOUT: cross_ref_irs_size: 1 +// CHECK:STDOUT: import_irs_size: 1 // CHECK:STDOUT: name_scopes: // CHECK:STDOUT: name_scope0: {inst: inst+0, enclosing_scope: name_scope, has_error: false, extended_scopes: [], names: {name0: inst+1}} // CHECK:STDOUT: bind_names: {} @@ -65,7 +65,7 @@ fn B() {} // CHECK:STDOUT: --- // CHECK:STDOUT: filename: b.carbon // CHECK:STDOUT: sem_ir: -// CHECK:STDOUT: cross_ref_irs_size: 1 +// CHECK:STDOUT: import_irs_size: 1 // CHECK:STDOUT: name_scopes: // CHECK:STDOUT: name_scope0: {inst: inst+0, enclosing_scope: name_scope, has_error: false, extended_scopes: [], names: {name0: inst+1}} // CHECK:STDOUT: bind_names: {} diff --git a/toolchain/check/testdata/basics/multifile_raw_ir.carbon b/toolchain/check/testdata/basics/multifile_raw_ir.carbon index faf46e7c6299..416186dff6dc 100644 --- a/toolchain/check/testdata/basics/multifile_raw_ir.carbon +++ b/toolchain/check/testdata/basics/multifile_raw_ir.carbon @@ -21,7 +21,7 @@ fn B() {} // CHECK:STDOUT: --- // CHECK:STDOUT: filename: a.carbon // CHECK:STDOUT: sem_ir: -// CHECK:STDOUT: cross_ref_irs_size: 1 +// CHECK:STDOUT: import_irs_size: 1 // CHECK:STDOUT: name_scopes: // CHECK:STDOUT: name_scope0: {inst: inst+0, enclosing_scope: name_scope, has_error: false, extended_scopes: [], names: {name0: inst+1}} // CHECK:STDOUT: bind_names: {} @@ -52,7 +52,7 @@ fn B() {} // CHECK:STDOUT: --- // CHECK:STDOUT: filename: b.carbon // CHECK:STDOUT: sem_ir: -// CHECK:STDOUT: cross_ref_irs_size: 1 +// CHECK:STDOUT: import_irs_size: 1 // CHECK:STDOUT: name_scopes: // CHECK:STDOUT: name_scope0: {inst: inst+0, enclosing_scope: name_scope, has_error: false, extended_scopes: [], names: {name0: inst+1}} // CHECK:STDOUT: bind_names: {} diff --git a/toolchain/check/testdata/basics/raw_and_textual_ir.carbon b/toolchain/check/testdata/basics/raw_and_textual_ir.carbon index a0784c214c73..05fac1098a85 100644 --- a/toolchain/check/testdata/basics/raw_and_textual_ir.carbon +++ b/toolchain/check/testdata/basics/raw_and_textual_ir.carbon @@ -15,7 +15,7 @@ fn Foo(n: i32) -> (i32, i32, f64) { // CHECK:STDOUT: --- // CHECK:STDOUT: filename: raw_and_textual_ir.carbon // CHECK:STDOUT: sem_ir: -// CHECK:STDOUT: cross_ref_irs_size: 1 +// CHECK:STDOUT: import_irs_size: 1 // CHECK:STDOUT: name_scopes: // CHECK:STDOUT: name_scope0: {inst: inst+0, enclosing_scope: name_scope, has_error: false, extended_scopes: [], names: {name0: inst+9}} // CHECK:STDOUT: bind_names: diff --git a/toolchain/check/testdata/basics/raw_ir.carbon b/toolchain/check/testdata/basics/raw_ir.carbon index 0450e128e167..c620d12b3982 100644 --- a/toolchain/check/testdata/basics/raw_ir.carbon +++ b/toolchain/check/testdata/basics/raw_ir.carbon @@ -15,7 +15,7 @@ fn Foo(n: i32) -> (i32, i32, f64) { // CHECK:STDOUT: --- // CHECK:STDOUT: filename: raw_ir.carbon // CHECK:STDOUT: sem_ir: -// CHECK:STDOUT: cross_ref_irs_size: 1 +// CHECK:STDOUT: import_irs_size: 1 // CHECK:STDOUT: name_scopes: // CHECK:STDOUT: name_scope0: {inst: inst+0, enclosing_scope: name_scope, has_error: false, extended_scopes: [], names: {name0: inst+9}} // CHECK:STDOUT: bind_names: diff --git a/toolchain/sem_ir/file.cpp b/toolchain/sem_ir/file.cpp index 01e143178fb9..2d73eaa929b0 100644 --- a/toolchain/sem_ir/file.cpp +++ b/toolchain/sem_ir/file.cpp @@ -66,8 +66,8 @@ File::File(SharedValueStores& value_stores) type_blocks_(allocator_), inst_blocks_(allocator_), constants_(*this, allocator_) { - auto builtins_id = cross_ref_irs_.Add(this); - CARBON_CHECK(builtins_id == CrossRefIRId::Builtins) + auto builtins_id = import_irs_.Add(this); + CARBON_CHECK(builtins_id == ImportIRId::Builtins) << "Builtins must be the first IR, even if self-referential"; insts_.Reserve(BuiltinKind::ValidCount); @@ -100,19 +100,19 @@ File::File(SharedValueStores& value_stores, std::string filename, inst_blocks_(allocator_), constants_(*this, allocator_) { CARBON_CHECK(builtins != nullptr); - auto builtins_id = cross_ref_irs_.Add(builtins); - CARBON_CHECK(builtins_id == CrossRefIRId::Builtins) + auto builtins_id = import_irs_.Add(builtins); + CARBON_CHECK(builtins_id == ImportIRId::Builtins) << "Builtins must be the first IR"; // Copy builtins over. insts_.Reserve(BuiltinKind::ValidCount); - static constexpr auto BuiltinIR = CrossRefIRId(0); for (auto [i, inst] : llvm::enumerate(builtins->insts_.array_ref())) { // We can reuse the type IDs from the builtins IR because they're // special-cased values. auto type_id = inst.type_id(); auto builtin_id = SemIR::InstId(i); - insts_.AddInNoBlock({ImportRefUsed{type_id, BuiltinIR, builtin_id}}); + insts_.AddInNoBlock( + {ImportRefUsed{type_id, ImportIRId::Builtins, builtin_id}}); constant_values_.Set(builtin_id, SemIR::ConstantId::ForTemplateConstant(builtin_id)); } @@ -161,8 +161,7 @@ auto File::OutputYaml(bool include_builtins) const -> Yaml::OutputMapping { map.Add("filename", filename_); map.Add( "sem_ir", Yaml::OutputMapping([&](Yaml::OutputMapping::Map map) { - map.Add("cross_ref_irs_size", - Yaml::OutputScalar(cross_ref_irs_.size())); + map.Add("import_irs_size", Yaml::OutputScalar(import_irs_.size())); map.Add("name_scopes", name_scopes_.OutputYaml()); map.Add("bind_names", bind_names_.OutputYaml()); map.Add("functions", functions_.OutputYaml()); @@ -509,7 +508,7 @@ auto GetExprCategory(const File& file, InstId inst_id) -> ExprCategory { case ImportRefUsed::Kind: { auto import_ref = inst.As(); - ir = ir->cross_ref_irs().Get(import_ref.ir_id); + ir = ir->import_irs().Get(import_ref.ir_id); inst_id = import_ref.inst_id; continue; } diff --git a/toolchain/sem_ir/file.h b/toolchain/sem_ir/file.h index efb7ab17c5ea..3f2f6686318a 100644 --- a/toolchain/sem_ir/file.h +++ b/toolchain/sem_ir/file.h @@ -270,9 +270,9 @@ class File : public Printable { auto interfaces() const -> const ValueStore& { return interfaces_; } - auto cross_ref_irs() -> ValueStore& { return cross_ref_irs_; } - auto cross_ref_irs() const -> const ValueStore& { - return cross_ref_irs_; + auto import_irs() -> ValueStore& { return import_irs_; } + auto import_irs() const -> const ValueStore& { + return import_irs_; } auto names() const -> NameStoreWrapper { return NameStoreWrapper(&identifiers()); @@ -339,10 +339,9 @@ class File : public Printable { // Storage for interfaces. ValueStore interfaces_; - // Related IRs. There will always be at least 2 entries, the builtin IR (used - // for references of builtins) followed by the current IR (used for references - // crossing instruction blocks). - ValueStore cross_ref_irs_; + // Related IRs. There will always be at least one entry, the builtin IR (used + // for references of builtins). + ValueStore import_irs_; // Storage for name scopes. NameScopeStore name_scopes_; diff --git a/toolchain/sem_ir/formatter.cpp b/toolchain/sem_ir/formatter.cpp index 5a53fb18d29d..14cd176c44de 100644 --- a/toolchain/sem_ir/formatter.cpp +++ b/toolchain/sem_ir/formatter.cpp @@ -1005,7 +1005,7 @@ class Formatter { auto FormatArg(InterfaceId id) -> void { FormatInterfaceName(id); } - auto FormatArg(CrossRefIRId id) -> void { out_ << id; } + auto FormatArg(ImportIRId id) -> void { out_ << id; } auto FormatArg(IntId id) -> void { sem_ir_.ints().Get(id).print(out_, /*isSigned=*/false); diff --git a/toolchain/sem_ir/ids.h b/toolchain/sem_ir/ids.h index 077fc6012541..c1c462d707ab 100644 --- a/toolchain/sem_ir/ids.h +++ b/toolchain/sem_ir/ids.h @@ -204,11 +204,11 @@ struct InterfaceId : public IdBase, public Printable { constexpr InterfaceId InterfaceId::Invalid = InterfaceId(InterfaceId::InvalidIndex); -// The ID of a cross-referenced IR. -struct CrossRefIRId : public IdBase, public Printable { +// The ID of an imported IR. +struct ImportIRId : public IdBase, public Printable { using ValueType = const File*; - static const CrossRefIRId Builtins; + static const ImportIRId Builtins; using IdBase::IdBase; auto Print(llvm::raw_ostream& out) const -> void { out << "ir"; @@ -216,7 +216,7 @@ struct CrossRefIRId : public IdBase, public Printable { } }; -constexpr CrossRefIRId CrossRefIRId::Builtins = CrossRefIRId(0); +constexpr ImportIRId ImportIRId::Builtins = ImportIRId(0); // A boolean value. struct BoolValue : public IdBase, public Printable { diff --git a/toolchain/sem_ir/typed_insts.h b/toolchain/sem_ir/typed_insts.h index 958f0e5fdb1d..dfec41fc2aa1 100644 --- a/toolchain/sem_ir/typed_insts.h +++ b/toolchain/sem_ir/typed_insts.h @@ -391,16 +391,16 @@ struct FunctionDecl { }; // An import corresponds to some number of IRs. The range of imported IRs is -// inclusive of last_cross_ref_ir_id, and will always be non-empty. If -// there was an import error, first_cross_ref_ir_id will reference a +// inclusive of last_import_ir_id, and will always be non-empty. If +// there was an import error, first_import_ir_id will reference a // nullptr IR; there should only ever be one nullptr in the range. struct Import { // TODO: Should always be an ImportDirectiveId? static constexpr auto Kind = InstKind::Import.Define("import"); TypeId type_id; - CrossRefIRId first_cross_ref_ir_id; - CrossRefIRId last_cross_ref_ir_id; + ImportIRId first_import_ir_id; + ImportIRId last_import_ir_id; }; // Common representation for all kinds of `ImportRef*` node. @@ -409,7 +409,7 @@ struct AnyImportRef { InstKind::ImportRefUsed}; InstKind kind; - CrossRefIRId ir_id; + ImportIRId ir_id; InstId inst_id; }; @@ -421,7 +421,7 @@ struct ImportRefUnused { static constexpr auto Kind = InstKind::ImportRefUnused.Define("import_ref"); - CrossRefIRId ir_id; + ImportIRId ir_id; InstId inst_id; }; @@ -432,7 +432,7 @@ struct ImportRefUsed { InstKind::ImportRefUsed.Define("import_ref"); TypeId type_id; - CrossRefIRId ir_id; + ImportIRId ir_id; InstId inst_id; }; diff --git a/toolchain/sem_ir/yaml_test.cpp b/toolchain/sem_ir/yaml_test.cpp index be0efea8844b..8581f11dd3ef 100644 --- a/toolchain/sem_ir/yaml_test.cpp +++ b/toolchain/sem_ir/yaml_test.cpp @@ -51,7 +51,7 @@ TEST(SemIRTest, YAML) { Pair("value_rep", Yaml::Mapping(_))))); auto file = Yaml::Mapping(ElementsAre( - Pair("cross_ref_irs_size", "1"), + Pair("import_irs_size", "1"), Pair("name_scopes", Yaml::Mapping(SizeIs(1))), Pair("bind_names", Yaml::Mapping(SizeIs(1))), Pair("functions", Yaml::Mapping(SizeIs(1))),