From 8167c44a034e66b6f70cf61a355d762dbb669383 Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Fri, 26 Jan 2024 16:36:58 -0800 Subject: [PATCH] Merging CrossRef into ImportRefUsed, shifting builtins over. (#3659) This is a bit of a cleanup; I probably should've just renamed CrossRef instead of adding ImportRefUsed. Adding `is_builtin` to InstId is more about providing a standard API for the check, which I expect to add a little more of. Shifts import tests to validate that the BuildValueRepr CHECK isn't accidentally hit. --- toolchain/check/context.cpp | 28 +++++++-------- toolchain/check/eval.cpp | 8 +++-- .../testdata/basics/builtin_insts.carbon | 18 +++++----- ...{import.carbon => fail_todo_import.carbon} | 35 ++++++++++++++++--- .../fail_todo_import.carbon} | 23 +++++++----- .../fail_todo_import.carbon} | 23 +++++++++--- .../testdata/var/fail_todo_import.carbon | 4 +-- toolchain/lower/file_context.cpp | 2 +- toolchain/lower/function_context.h | 2 +- toolchain/lower/handle.cpp | 5 --- toolchain/sem_ir/file.cpp | 21 ++++------- toolchain/sem_ir/file.h | 4 +-- toolchain/sem_ir/formatter.cpp | 8 +---- toolchain/sem_ir/ids.h | 13 +++++-- toolchain/sem_ir/inst_kind.def | 1 - toolchain/sem_ir/typed_insts.h | 12 ------- toolchain/sem_ir/yaml_test.cpp | 4 +-- 17 files changed, 116 insertions(+), 95 deletions(-) rename toolchain/check/testdata/class/{import.carbon => fail_todo_import.carbon} (53%) rename toolchain/check/testdata/function/{definition/import.carbon => declaration/fail_todo_import.carbon} (64%) rename toolchain/check/testdata/function/{declaration/import.carbon => definition/fail_todo_import.carbon} (55%) diff --git a/toolchain/check/context.cpp b/toolchain/check/context.cpp index 45fb8c461053..0c3722ef275f 100644 --- a/toolchain/check/context.cpp +++ b/toolchain/check/context.cpp @@ -787,19 +787,17 @@ class TypeCompleter { return value_rep; }; - auto BuildCrossRefValueRepr(SemIR::TypeId type_id, SemIR::CrossRef xref) const + auto BuildImportRefUsedValueRepr(SemIR::TypeId type_id, + SemIR::ImportRefUsed import_ref) const -> SemIR::ValueRepr { - auto xref_inst = - context_.cross_ref_irs().Get(xref.ir_id)->insts().Get(xref.inst_id); + CARBON_CHECK(import_ref.inst_id.is_builtin()) + << "TODO: Handle non-builtin ImportRefUsed cases, such as functions, " + "classes, and interfaces"; - // The canonical description of a type should only have cross-references - // for entities owned by another File, such as builtins, which are owned - // by the prelude, and named entities like classes and interfaces, which - // we don't support yet. - CARBON_CHECK(xref_inst.kind() == SemIR::Builtin::Kind) - << "TODO: Handle other kinds of inst cross-references"; + const auto& import_ir = context_.cross_ref_irs().Get(import_ref.ir_id); + auto import_inst = import_ir->insts().Get(import_ref.inst_id); - switch (xref_inst.As().builtin_kind) { + switch (import_inst.As().builtin_kind) { case SemIR::BuiltinKind::TypeType: case SemIR::BuiltinKind::Error: case SemIR::BuiltinKind::Invalid: @@ -945,7 +943,6 @@ class TypeCompleter { case SemIR::InterfaceDecl::Kind: case SemIR::IntLiteral::Kind: case SemIR::ImportRefUnused::Kind: - case SemIR::ImportRefUsed::Kind: case SemIR::NameRef::Kind: case SemIR::Namespace::Kind: case SemIR::Param::Kind: @@ -972,9 +969,6 @@ class TypeCompleter { case SemIR::VarStorage::Kind: CARBON_FATAL() << "Type refers to non-type inst " << inst; - case SemIR::CrossRef::Kind: - return BuildCrossRefValueRepr(type_id, inst.As()); - case SemIR::ArrayType::Kind: { // For arrays, it's convenient to always use a pointer representation, // even when the array has zero or one element, in order to support @@ -982,6 +976,10 @@ class TypeCompleter { return MakePointerValueRepr(type_id, SemIR::ValueRepr::ObjectAggregate); } + case SemIR::ImportRefUsed::Kind: + return BuildImportRefUsedValueRepr(type_id, + inst.As()); + case SemIR::StructType::Kind: return BuildStructTypeValueRepr(type_id, inst.As()); @@ -1000,7 +998,7 @@ class TypeCompleter { SemIR::ValueRepr::ObjectAggregate); case SemIR::Builtin::Kind: - CARBON_FATAL() << "Builtins should be named as cross-references"; + CARBON_FATAL() << "Builtins should be named as ImportRefUsed"; case SemIR::BindSymbolicName::Kind: case SemIR::PointerType::Kind: diff --git a/toolchain/check/eval.cpp b/toolchain/check/eval.cpp index 845284bce269..0d71dedba3ff 100644 --- a/toolchain/check/eval.cpp +++ b/toolchain/check/eval.cpp @@ -402,8 +402,8 @@ auto TryEvalInst(Context& context, SemIR::InstId inst_id, SemIR::Inst inst) // TODO: These need special handling. case SemIR::BindValue::Kind: case SemIR::Call::Kind: - case SemIR::CrossRef::Kind: case SemIR::Deref::Kind: + case SemIR::ImportRefUsed::Kind: case SemIR::Temporary::Kind: case SemIR::TemporaryStorage::Kind: case SemIR::ValueAsRef::Kind: @@ -473,8 +473,6 @@ auto TryEvalInst(Context& context, SemIR::InstId inst_id, SemIR::Inst inst) case SemIR::BranchWithArg::Kind: case SemIR::ClassDecl::Kind: case SemIR::Import::Kind: - case SemIR::ImportRefUnused::Kind: - case SemIR::ImportRefUsed::Kind: case SemIR::InterfaceDecl::Kind: case SemIR::Param::Kind: case SemIR::ReturnExpr::Kind: @@ -483,6 +481,10 @@ auto TryEvalInst(Context& context, SemIR::InstId inst_id, SemIR::Inst inst) case SemIR::TupleLiteral::Kind: case SemIR::VarStorage::Kind: break; + + case SemIR::ImportRefUnused::Kind: + CARBON_FATAL() << "ImportRefUnused should transform to ImportRefUsed " + "before TryEvalInst."; } return SemIR::ConstantId::NotConstant; } diff --git a/toolchain/check/testdata/basics/builtin_insts.carbon b/toolchain/check/testdata/basics/builtin_insts.carbon index 6ee2b3b5736f..db1e4070d6dc 100644 --- a/toolchain/check/testdata/basics/builtin_insts.carbon +++ b/toolchain/check/testdata/basics/builtin_insts.carbon @@ -19,15 +19,15 @@ // CHECK:STDOUT: type0: {constant: template instNamespaceType, value_rep: {kind: copy, type: type0}} // CHECK:STDOUT: type_blocks: {} // CHECK:STDOUT: insts: -// CHECK:STDOUT: instTypeType: {kind: CrossRef, arg0: ir0, arg1: instTypeType, type: typeTypeType} -// CHECK:STDOUT: instError: {kind: CrossRef, arg0: ir0, arg1: instError, type: typeError} -// CHECK:STDOUT: instBoolType: {kind: CrossRef, arg0: ir0, arg1: instBoolType, type: typeTypeType} -// CHECK:STDOUT: instIntType: {kind: CrossRef, arg0: ir0, arg1: instIntType, type: typeTypeType} -// CHECK:STDOUT: instFloatType: {kind: CrossRef, arg0: ir0, arg1: instFloatType, type: typeTypeType} -// CHECK:STDOUT: instStringType: {kind: CrossRef, arg0: ir0, arg1: instStringType, type: typeTypeType} -// CHECK:STDOUT: instFunctionType: {kind: CrossRef, arg0: ir0, arg1: instFunctionType, type: typeTypeType} -// CHECK:STDOUT: instBoundMethodType: {kind: CrossRef, arg0: ir0, arg1: instBoundMethodType, type: typeTypeType} -// CHECK:STDOUT: instNamespaceType: {kind: CrossRef, arg0: ir0, arg1: instNamespaceType, type: typeTypeType} +// CHECK:STDOUT: instTypeType: {kind: ImportRefUsed, arg0: ir0, arg1: instTypeType, type: typeTypeType} +// CHECK:STDOUT: instError: {kind: ImportRefUsed, arg0: ir0, arg1: instError, type: typeError} +// CHECK:STDOUT: instBoolType: {kind: ImportRefUsed, arg0: ir0, arg1: instBoolType, type: typeTypeType} +// CHECK:STDOUT: instIntType: {kind: ImportRefUsed, arg0: ir0, arg1: instIntType, type: typeTypeType} +// CHECK:STDOUT: instFloatType: {kind: ImportRefUsed, arg0: ir0, arg1: instFloatType, type: typeTypeType} +// CHECK:STDOUT: instStringType: {kind: ImportRefUsed, arg0: ir0, arg1: instStringType, type: typeTypeType} +// CHECK:STDOUT: instFunctionType: {kind: ImportRefUsed, arg0: ir0, arg1: instFunctionType, type: typeTypeType} +// CHECK:STDOUT: instBoundMethodType: {kind: ImportRefUsed, arg0: ir0, arg1: instBoundMethodType, type: typeTypeType} +// CHECK:STDOUT: instNamespaceType: {kind: ImportRefUsed, arg0: ir0, arg1: instNamespaceType, type: typeTypeType} // CHECK:STDOUT: inst+0: {kind: Namespace, arg0: name_scope0, arg1: inst, type: type0} // CHECK:STDOUT: constant_values: // CHECK:STDOUT: instTypeType: template instTypeType diff --git a/toolchain/check/testdata/class/import.carbon b/toolchain/check/testdata/class/fail_todo_import.carbon similarity index 53% rename from toolchain/check/testdata/class/import.carbon rename to toolchain/check/testdata/class/fail_todo_import.carbon index 42b9c2f19cfa..48c2c085ccf0 100644 --- a/toolchain/check/testdata/class/import.carbon +++ b/toolchain/check/testdata/class/fail_todo_import.carbon @@ -3,6 +3,8 @@ // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // // AUTOUPDATE +// CHECK:STDERR: b.carbon: ERROR: Semantics TODO: `TODO: ResolveIfImportRefUnused for ClassDecl`. +// CHECK:STDERR: b.carbon: ERROR: Semantics TODO: `TODO: ResolveIfImportRefUnused for ClassDecl`. // --- a.carbon @@ -23,7 +25,11 @@ library "b" api; import library "a"; -// TODO: When ready, consider tests of basic import functionality. +fn Run() { + var x: Empty = {}; + var y: ForwardDeclared = {}; + y.F(); +} // CHECK:STDOUT: --- a.carbon // CHECK:STDOUT: @@ -56,9 +62,30 @@ import library "a"; // CHECK:STDOUT: // CHECK:STDOUT: --- b.carbon // CHECK:STDOUT: +// CHECK:STDOUT: constants { +// CHECK:STDOUT: %.1: type = struct_type {} [template] +// CHECK:STDOUT: } +// CHECK:STDOUT: // CHECK:STDOUT: file { -// CHECK:STDOUT: package: = namespace {.Empty = %import_ref.1, .ForwardDeclared = %import_ref.2} [template] -// CHECK:STDOUT: %import_ref.1 = import_ref ir1, inst+1, unused -// CHECK:STDOUT: %import_ref.2 = import_ref ir1, inst+4, unused +// CHECK:STDOUT: package: = namespace {.Empty = %import_ref.1, .ForwardDeclared = %import_ref.2, .Run = %Run} [template] +// CHECK:STDOUT: %import_ref.1: = import_ref ir1, inst+1, used +// CHECK:STDOUT: %import_ref.2: = import_ref ir1, inst+4, used +// CHECK:STDOUT: %Run: = fn_decl @Run [template] +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: fn @Run() { +// CHECK:STDOUT: !entry: +// CHECK:STDOUT: %Empty.ref: = name_ref Empty, file.%import_ref.1 +// CHECK:STDOUT: %x.var: ref = var x +// CHECK:STDOUT: %x: ref = bind_name x, %x.var +// CHECK:STDOUT: %.loc7: {} = struct_literal () +// CHECK:STDOUT: assign %x.var, +// CHECK:STDOUT: %ForwardDeclared.ref: = name_ref ForwardDeclared, file.%import_ref.2 +// CHECK:STDOUT: %y.var: ref = var y +// CHECK:STDOUT: %y: ref = bind_name y, %y.var +// CHECK:STDOUT: %.loc8: {} = struct_literal () +// CHECK:STDOUT: assign %y.var, +// CHECK:STDOUT: %y.ref: ref = name_ref y, %y +// CHECK:STDOUT: return // CHECK:STDOUT: } // CHECK:STDOUT: diff --git a/toolchain/check/testdata/function/definition/import.carbon b/toolchain/check/testdata/function/declaration/fail_todo_import.carbon similarity index 64% rename from toolchain/check/testdata/function/definition/import.carbon rename to toolchain/check/testdata/function/declaration/fail_todo_import.carbon index 4a79f3db66de..d6cae9c25446 100644 --- a/toolchain/check/testdata/function/definition/import.carbon +++ b/toolchain/check/testdata/function/declaration/fail_todo_import.carbon @@ -3,12 +3,13 @@ // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // // AUTOUPDATE +// CHECK:STDERR: b.carbon: ERROR: Semantics TODO: `TODO: ResolveIfImportRefUnused for FunctionDecl`. // --- a.carbon library "a" api; -fn F() {} +fn F(); // --- b.carbon @@ -16,7 +17,9 @@ library "b" api; import library "a"; -// TODO: When ready, consider tests of basic import functionality. +fn Run() { + F(); +} // CHECK:STDOUT: --- a.carbon // CHECK:STDOUT: @@ -25,15 +28,19 @@ import library "a"; // CHECK:STDOUT: %F: = fn_decl @F [template] // CHECK:STDOUT: } // CHECK:STDOUT: -// CHECK:STDOUT: fn @F() { -// CHECK:STDOUT: !entry: -// CHECK:STDOUT: return -// CHECK:STDOUT: } +// CHECK:STDOUT: fn @F(); // CHECK:STDOUT: // CHECK:STDOUT: --- b.carbon // CHECK:STDOUT: // CHECK:STDOUT: file { -// CHECK:STDOUT: package: = namespace {.F = %import_ref} [template] -// CHECK:STDOUT: %import_ref = import_ref ir1, inst+1, unused +// CHECK:STDOUT: package: = namespace {.F = %import_ref, .Run = %Run} [template] +// CHECK:STDOUT: %import_ref: = import_ref ir1, inst+1, used +// CHECK:STDOUT: %Run: = fn_decl @Run [template] +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: fn @Run() { +// CHECK:STDOUT: !entry: +// CHECK:STDOUT: %F.ref: = name_ref F, file.%import_ref +// CHECK:STDOUT: return // CHECK:STDOUT: } // CHECK:STDOUT: diff --git a/toolchain/check/testdata/function/declaration/import.carbon b/toolchain/check/testdata/function/definition/fail_todo_import.carbon similarity index 55% rename from toolchain/check/testdata/function/declaration/import.carbon rename to toolchain/check/testdata/function/definition/fail_todo_import.carbon index 2f8523a8377f..52c003533d81 100644 --- a/toolchain/check/testdata/function/declaration/import.carbon +++ b/toolchain/check/testdata/function/definition/fail_todo_import.carbon @@ -3,12 +3,13 @@ // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // // AUTOUPDATE +// CHECK:STDERR: b.carbon: ERROR: Semantics TODO: `TODO: ResolveIfImportRefUnused for FunctionDecl`. // --- a.carbon library "a" api; -fn F(); +fn F() {} // --- b.carbon @@ -16,7 +17,9 @@ library "b" api; import library "a"; -// TODO: When ready, consider tests of basic import functionality. +fn Run() { + F(); +} // CHECK:STDOUT: --- a.carbon // CHECK:STDOUT: @@ -25,12 +28,22 @@ import library "a"; // CHECK:STDOUT: %F: = fn_decl @F [template] // CHECK:STDOUT: } // CHECK:STDOUT: -// CHECK:STDOUT: fn @F(); +// CHECK:STDOUT: fn @F() { +// CHECK:STDOUT: !entry: +// CHECK:STDOUT: return +// CHECK:STDOUT: } // CHECK:STDOUT: // CHECK:STDOUT: --- b.carbon // CHECK:STDOUT: // CHECK:STDOUT: file { -// CHECK:STDOUT: package: = namespace {.F = %import_ref} [template] -// CHECK:STDOUT: %import_ref = import_ref ir1, inst+1, unused +// CHECK:STDOUT: package: = namespace {.F = %import_ref, .Run = %Run} [template] +// CHECK:STDOUT: %import_ref: = import_ref ir1, inst+1, used +// CHECK:STDOUT: %Run: = fn_decl @Run [template] +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: fn @Run() { +// CHECK:STDOUT: !entry: +// CHECK:STDOUT: %F.ref: = name_ref F, file.%import_ref +// CHECK:STDOUT: return // CHECK:STDOUT: } // CHECK:STDOUT: diff --git a/toolchain/check/testdata/var/fail_todo_import.carbon b/toolchain/check/testdata/var/fail_todo_import.carbon index 8a633b9f89e5..d38c9135182f 100644 --- a/toolchain/check/testdata/var/fail_todo_import.carbon +++ b/toolchain/check/testdata/var/fail_todo_import.carbon @@ -42,12 +42,12 @@ var a: () = a_ref; // CHECK:STDOUT: // CHECK:STDOUT: file { // CHECK:STDOUT: package: = namespace {.a_ref = %import_ref, .a = %a} [template] -// CHECK:STDOUT: %import_ref: = import_ref ir1, inst+2, used +// CHECK:STDOUT: %import_ref: ref = import_ref ir1, inst+2, used // CHECK:STDOUT: %.loc4_9.1: () = tuple_literal () // CHECK:STDOUT: %.loc4_9.2: type = converted %.loc4_9.1, constants.%.1 [template = constants.%.1] // CHECK:STDOUT: %a.var: ref () = var a // CHECK:STDOUT: %a: ref () = bind_name a, %a.var -// CHECK:STDOUT: %a_ref.ref: = name_ref a_ref, %import_ref +// CHECK:STDOUT: %a_ref.ref: ref = name_ref a_ref, %import_ref // CHECK:STDOUT: assign %a.var, // CHECK:STDOUT: } // CHECK:STDOUT: diff --git a/toolchain/lower/file_context.cpp b/toolchain/lower/file_context.cpp index b429ffe9737d..e89d80432a2e 100644 --- a/toolchain/lower/file_context.cpp +++ b/toolchain/lower/file_context.cpp @@ -57,7 +57,7 @@ auto FileContext::Run() -> std::unique_ptr { auto FileContext::GetGlobal(SemIR::InstId inst_id) -> llvm::Value* { // All builtins are types, with the same empty lowered value. - if (inst_id.index < SemIR::BuiltinKind::ValidCount) { + if (inst_id.is_builtin()) { return GetTypeAsValue(); } diff --git a/toolchain/lower/function_context.h b/toolchain/lower/function_context.h index f84da3238a4b..3e61090c1fa3 100644 --- a/toolchain/lower/function_context.h +++ b/toolchain/lower/function_context.h @@ -41,7 +41,7 @@ class FunctionContext { // Returns a value for the given instruction. auto GetValue(SemIR::InstId inst_id) -> llvm::Value* { // All builtins are types, with the same empty lowered value. - if (inst_id.index < SemIR::BuiltinKind::ValidCount) { + if (inst_id.is_builtin()) { return GetTypeAsValue(); } diff --git a/toolchain/lower/handle.cpp b/toolchain/lower/handle.cpp index 5b24b0530853..dfeb86bee731 100644 --- a/toolchain/lower/handle.cpp +++ b/toolchain/lower/handle.cpp @@ -180,11 +180,6 @@ auto HandleConverted(FunctionContext& context, SemIR::InstId inst_id, context.SetLocal(inst_id, context.GetValue(inst.result_id)); } -auto HandleCrossRef(FunctionContext& /*context*/, SemIR::InstId /*inst_id*/, - SemIR::CrossRef inst) -> void { - FatalErrorIfEncountered(inst); -} - auto HandleDeref(FunctionContext& context, SemIR::InstId inst_id, SemIR::Deref inst) -> void { context.SetLocal(inst_id, context.GetValue(inst.pointer_id)); diff --git a/toolchain/sem_ir/file.cpp b/toolchain/sem_ir/file.cpp index f87075e86079..01e143178fb9 100644 --- a/toolchain/sem_ir/file.cpp +++ b/toolchain/sem_ir/file.cpp @@ -112,8 +112,7 @@ File::File(SharedValueStores& value_stores, std::string filename, // special-cased values. auto type_id = inst.type_id(); auto builtin_id = SemIR::InstId(i); - insts_.AddInNoBlock( - {Parse::NodeId::Invalid, CrossRef{type_id, BuiltinIR, builtin_id}}); + insts_.AddInNoBlock({ImportRefUsed{type_id, BuiltinIR, builtin_id}}); constant_values_.Set(builtin_id, SemIR::ConstantId::ForTemplateConstant(builtin_id)); } @@ -215,12 +214,6 @@ static auto GetTypePrecedence(InstKind kind) -> int { case PointerType::Kind: return -2; - case CrossRef::Kind: - // TODO: Once we support stringification of cross-references, we'll need - // to determine the precedence of the target of the cross-reference. For - // now, all cross-references refer to builtin types from the prelude. - return 0; - case AddrOf::Kind: case AddrPattern::Kind: case ArrayIndex::Kind: @@ -303,7 +296,7 @@ auto File::StringifyTypeExpr(InstId outer_inst_id) const -> std::string { } // Builtins have designated labels. - if (step.inst_id.index < BuiltinKind::ValidCount) { + if (step.inst_id.is_builtin()) { out << BuiltinKind::FromInt(step.inst_id.index).label(); continue; } @@ -446,7 +439,6 @@ auto File::StringifyTypeExpr(InstId outer_inst_id) const -> std::string { case ClassElementAccess::Kind: case ClassInit::Kind: case Converted::Kind: - case CrossRef::Kind: case Deref::Kind: case FieldDecl::Kind: case FunctionDecl::Kind: @@ -508,7 +500,6 @@ auto GetExprCategory(const File& file, InstId inst_id) -> ExprCategory { case FunctionDecl::Kind: case Import::Kind: case ImportRefUnused::Kind: - case ImportRefUsed::Kind: case InterfaceDecl::Kind: case Namespace::Kind: case Return::Kind: @@ -516,10 +507,10 @@ auto GetExprCategory(const File& file, InstId inst_id) -> ExprCategory { case StructTypeField::Kind: return ExprCategory::NotExpr; - case CrossRef::Kind: { - auto xref = inst.As(); - ir = ir->cross_ref_irs().Get(xref.ir_id); - inst_id = xref.inst_id; + case ImportRefUsed::Kind: { + auto import_ref = inst.As(); + ir = ir->cross_ref_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 9ddefad610d9..9966015eb570 100644 --- a/toolchain/sem_ir/file.h +++ b/toolchain/sem_ir/file.h @@ -350,8 +350,8 @@ class File : public Printable { // the data is provided by allocator_. BlockValueStore type_blocks_; - // All instructions. The first entries will always be cross-references to - // builtins, at indices matching BuiltinKind ordering. + // All instructions. The first entries will always be ImportRefs to builtins, + // at indices matching BuiltinKind ordering. InstStore insts_; // Constant values for instructions. diff --git a/toolchain/sem_ir/formatter.cpp b/toolchain/sem_ir/formatter.cpp index 682a5f1ea9cf..5a53fb18d29d 100644 --- a/toolchain/sem_ir/formatter.cpp +++ b/toolchain/sem_ir/formatter.cpp @@ -169,7 +169,7 @@ class InstNamer { } // Check for a builtin. - if (inst_id.index < BuiltinKind::ValidCount) { + if (inst_id.is_builtin()) { return BuiltinKind::FromInt(inst_id.index).label().str(); } @@ -940,12 +940,6 @@ class Formatter { FormatReturnSlot(init.dest_id); } - auto FormatInstructionRHS(CrossRef inst) -> void { - // TODO: Figure out a way to make this meaningful. We'll need some way to - // name cross-reference IRs, perhaps by the instruction ID of the import? - out_ << " " << inst.ir_id << ", " << inst.inst_id; - } - auto FormatInstructionRHS(ImportRefUnused inst) -> void { // Don't format the inst_id because it refers to a different IR. // TODO: Consider a better way to format the InstID from other IRs. diff --git a/toolchain/sem_ir/ids.h b/toolchain/sem_ir/ids.h index c5fd2b1d70cd..077fc6012541 100644 --- a/toolchain/sem_ir/ids.h +++ b/toolchain/sem_ir/ids.h @@ -37,18 +37,25 @@ struct InstId : public IdBase, public Printable { // The namespace for a `package` expression. static const InstId PackageNamespace; - // Returns the cross-reference instruction ID for a builtin. This relies on - // File guarantees for builtin cross-reference placement. + // Returns the instruction ID for a builtin. This relies on File guarantees + // for builtin ImportRefUsed placement. static constexpr auto ForBuiltin(BuiltinKind kind) -> InstId { return InstId(kind.AsInt()); } using IdBase::IdBase; + + // Returns true if the instruction is a builtin. Requires is_valid. + auto is_builtin() const -> bool { + CARBON_CHECK(is_valid()); + return index < BuiltinKind::ValidCount; + } + auto Print(llvm::raw_ostream& out) const -> void { out << "inst"; if (!is_valid()) { IdBase::Print(out); - } else if (index < BuiltinKind::ValidCount) { + } else if (is_builtin()) { out << BuiltinKind::FromInt(index); } else { // Use the `+` as a small reminder that this is a delta, rather than an diff --git a/toolchain/sem_ir/inst_kind.def b/toolchain/sem_ir/inst_kind.def index 16ef241b9378..e99b3ec6f7b8 100644 --- a/toolchain/sem_ir/inst_kind.def +++ b/toolchain/sem_ir/inst_kind.def @@ -41,7 +41,6 @@ CARBON_SEM_IR_INST_KIND(ClassInit) CARBON_SEM_IR_INST_KIND(ClassType) CARBON_SEM_IR_INST_KIND(ConstType) CARBON_SEM_IR_INST_KIND(Converted) -CARBON_SEM_IR_INST_KIND(CrossRef) CARBON_SEM_IR_INST_KIND(Deref) CARBON_SEM_IR_INST_KIND(FieldDecl) CARBON_SEM_IR_INST_KIND(FunctionDecl) diff --git a/toolchain/sem_ir/typed_insts.h b/toolchain/sem_ir/typed_insts.h index 4157088c5f0f..2120a264aa43 100644 --- a/toolchain/sem_ir/typed_insts.h +++ b/toolchain/sem_ir/typed_insts.h @@ -364,18 +364,6 @@ struct Converted { InstId result_id; }; -// A cross-reference between IRs. -struct CrossRef { - static constexpr auto Kind = InstKind::CrossRef.Define("xref"); - - // No parse node: an instruction's parse tree node must refer to a node in the - // current parse tree. This cannot use the cross-referenced instruction's - // parse tree node because it will be in a different parse tree. - TypeId type_id; - CrossRefIRId ir_id; - InstId inst_id; -}; - struct Deref { // TODO: Make Parse::NodeId more specific. static constexpr auto Kind = InstKind::Deref.Define("deref"); diff --git a/toolchain/sem_ir/yaml_test.cpp b/toolchain/sem_ir/yaml_test.cpp index 0e82e83685ff..be0efea8844b 100644 --- a/toolchain/sem_ir/yaml_test.cpp +++ b/toolchain/sem_ir/yaml_test.cpp @@ -38,8 +38,8 @@ TEST(SemIRTest, YAML) { d.RunCommand( {"compile", "--phase=check", "--dump-raw-sem-ir", "test.carbon"}); - // Matches the ID of an instruction. The numbers may change because of builtin - // cross-references, so this code is only doing loose structural checks. + // Matches the ID of an instruction. Instruction counts may change as various + // support changes, so this code is only doing loose structural checks. auto int_id = Yaml::Scalar(MatchesRegex(R"(int\d+)")); auto inst_id = Yaml::Scalar(MatchesRegex(R"(inst\+\d+)")); auto constant_id =