diff --git a/toolchain/check/cpp/import.cpp b/toolchain/check/cpp/import.cpp index b1e211f27136..a2c06bcdb4aa 100644 --- a/toolchain/check/cpp/import.cpp +++ b/toolchain/check/cpp/import.cpp @@ -681,10 +681,9 @@ static auto BuildClassDecl(Context& context, auto class_decl = SemIR::ClassDecl{.type_id = SemIR::TypeType::TypeId, .class_id = SemIR::ClassId::None, .decl_block_id = SemIR::InstBlockId::None}; - auto class_decl_id = AddPlaceholderInstInNoBlock( + auto class_decl_id = AddPlaceholderImportedInstInNoBlock( context, - SemIR::LocIdAndInst::UncheckedLoc(import_ir_inst_id, class_decl)); - context.imports().push_back(class_decl_id); + MakeImportedLocIdAndInst(context, import_ir_inst_id, class_decl)); SemIR::Class class_info = { {.name_id = name_id, @@ -951,11 +950,12 @@ static auto ImportClassObjectRepr(Context& context, SemIR::ClassId class_id, // TODO: Add a field to prevent tail padding reuse if necessary. - return AddTypeInst( - context, import_ir_inst_id, - {.type_id = SemIR::TypeType::TypeId, - .fields_id = context.struct_type_fields().Add(fields), - .layout_id = context.custom_layouts().Add(layout)}); + return AddTypeInst(context, + MakeImportedLocIdAndInst( + context, import_ir_inst_id, + {.type_id = SemIR::TypeType::TypeId, + .fields_id = context.struct_type_fields().Add(fields), + .layout_id = context.custom_layouts().Add(layout)})); } // Creates a Carbon class definition based on the information in the given Clang @@ -976,10 +976,12 @@ static auto BuildClassDefinition(Context& context, // Compute the class's object representation. auto object_repr_id = ImportClassObjectRepr( context, class_id, import_ir_inst_id, class_inst_id, clang_def); - class_info.complete_type_witness_id = AddInst( - context, import_ir_inst_id, - {.type_id = GetSingletonType(context, SemIR::WitnessType::TypeInstId), - .object_repr_type_inst_id = object_repr_id}); + class_info.complete_type_witness_id = AddInst( + context, + MakeImportedLocIdAndInst( + context, import_ir_inst_id, + {.type_id = GetSingletonType(context, SemIR::WitnessType::TypeInstId), + .object_repr_type_inst_id = object_repr_id})); class_info.body_block_id = context.inst_block_stack().Pop(); } @@ -996,11 +998,13 @@ static auto ImportEnumObjectRepresentation( auto int_kind = int_type->isSignedIntegerType() ? SemIR::IntKind::Signed : SemIR::IntKind::Unsigned; - auto bit_width_id = GetOrAddInst( - context, import_ir_inst_id, - {.type_id = GetSingletonType(context, SemIR::IntLiteralType::TypeInstId), - .int_id = context.ints().AddUnsigned( - llvm::APInt(64, context.ast_context().getIntWidth(int_type)))}); + auto bit_width_id = GetOrAddInst( + context, MakeImportedLocIdAndInst( + context, import_ir_inst_id, + {.type_id = GetSingletonType( + context, SemIR::IntLiteralType::TypeInstId), + .int_id = context.ints().AddUnsigned(llvm::APInt( + 64, context.ast_context().getIntWidth(int_type)))})); return context.types().GetAsTypeInstId( GetOrAddInst(context, SemIR::LocIdAndInst::NoLoc(SemIR::IntType{ .type_id = SemIR::TypeType::TypeId, @@ -1029,13 +1033,15 @@ static auto BuildEnumDefinition(Context& context, auto object_repr_id = ImportEnumObjectRepresentation(context, import_ir_inst_id, enum_decl); class_info.adapt_id = AddInst( - context, SemIR::LocIdAndInst::UncheckedLoc( - import_ir_inst_id, + context, MakeImportedLocIdAndInst( + context, import_ir_inst_id, SemIR::AdaptDecl{.adapted_type_inst_id = object_repr_id})); - class_info.complete_type_witness_id = AddInst( - context, import_ir_inst_id, - {.type_id = GetSingletonType(context, SemIR::WitnessType::TypeInstId), - .object_repr_type_inst_id = object_repr_id}); + class_info.complete_type_witness_id = AddInst( + context, + MakeImportedLocIdAndInst( + context, import_ir_inst_id, + {.type_id = GetSingletonType(context, SemIR::WitnessType::TypeInstId), + .object_repr_type_inst_id = object_repr_id})); class_info.body_block_id = context.inst_block_stack().Pop(); } @@ -1055,10 +1061,12 @@ static auto ImportEnumConstantDecl(Context& context, // Build a corresponding IntValue. auto int_id = context.ints().Add(enumerator_decl->getInitVal()); - auto loc_id = + auto import_ir_inst_id = AddImportIRInst(context.sem_ir(), enumerator_decl->getLocation()); - auto inst_id = AddInstInNoBlock( - context, loc_id, {.type_id = type_id, .int_id = int_id}); + auto inst_id = AddInstInNoBlock( + context, + MakeImportedLocIdAndInst( + context, import_ir_inst_id, {.type_id = type_id, .int_id = int_id})); context.imports().push_back(inst_id); context.clang_decls().Add({.key = key, .inst_id = inst_id}); return inst_id; @@ -1667,9 +1675,8 @@ static auto ImportFunction(Context& context, SemIR::LocId loc_id, auto function_decl = SemIR::FunctionDecl{ SemIR::TypeId::None, SemIR::FunctionId::None, decl_block_id}; - auto decl_id = - AddPlaceholderInstInNoBlock(context, Parse::NodeId::None, function_decl); - context.imports().push_back(decl_id); + auto decl_id = AddPlaceholderImportedInstInNoBlock( + context, SemIR::LocIdAndInst::NoLoc(function_decl)); auto virtual_modifier = SemIR::Function::VirtualModifier::None; int32_t virtual_index = -1; @@ -1886,7 +1893,7 @@ static auto ImportVarDecl(Context& context, SemIR::LocId loc_id, // We can't use the convenience for `AddPlaceholderInstInNoBlock()` with typed // nodes because it doesn't support insts with cleanup. SemIR::InstId var_storage_inst_id = - AddPlaceholderInstInNoBlock(context, {loc_id, var_storage}); + AddPlaceholderImportedInstInNoBlock(context, {loc_id, var_storage}); auto clang_decl_id = context.clang_decls().Add( {.key = SemIR::ClangDeclKey(var_decl), .inst_id = var_storage_inst_id}); @@ -1915,7 +1922,6 @@ static auto ImportVarDecl(Context& context, SemIR::LocId loc_id, // Finalize the `VarStorage` instruction. ReplaceInstBeforeConstantUse(context, var_storage_inst_id, var_storage); - context.imports().push_back(var_storage_inst_id); return var_storage_inst_id; } diff --git a/toolchain/check/import.cpp b/toolchain/check/import.cpp index 090bea8f413a..7bee6e1b2f13 100644 --- a/toolchain/check/import.cpp +++ b/toolchain/check/import.cpp @@ -144,8 +144,8 @@ auto AddImportNamespace(Context& context, SemIR::TypeId namespace_type_id, MakeImportedNamespaceLocIdAndInst(context, import_id, namespace_inst); AddImportNamespaceResult result = { .name_scope_id = SemIR::NameScopeId::None, - .inst_id = AddPlaceholderInstInNoBlock(context, namespace_inst_and_loc)}; - context.imports().push_back(result.inst_id); + .inst_id = + AddPlaceholderImportedInstInNoBlock(context, namespace_inst_and_loc)}; namespace_inst.name_scope_id = context.name_scopes().Add(result.inst_id, name_id, parent_scope_id); result.name_scope_id = namespace_inst.name_scope_id; diff --git a/toolchain/check/import.h b/toolchain/check/import.h index fc89754ff7fb..f9ce93979127 100644 --- a/toolchain/check/import.h +++ b/toolchain/check/import.h @@ -114,7 +114,7 @@ auto MakeImportedLocIdAndInst(Context& context, Internal::CheckCompatibleImportedNodeKind(context, imported_loc_id, InstT::Kind); } - return SemIR::LocIdAndInst::UncheckedLoc(imported_loc_id, inst); + return SemIR::LocIdAndInst::UncheckedLoc(SemIR::LocId(imported_loc_id), inst); } } // namespace Carbon::Check diff --git a/toolchain/check/import_ref.cpp b/toolchain/check/import_ref.cpp index c188f8d43d41..c03e5f9ae415 100644 --- a/toolchain/check/import_ref.cpp +++ b/toolchain/check/import_ref.cpp @@ -89,15 +89,12 @@ auto AddImportRef(Context& context, SemIR::ImportIRInst import_ir_inst, SemIR::EntityNameId entity_name_id = SemIR::EntityNameId::None) -> SemIR::InstId { auto import_ir_inst_id = context.import_ir_insts().Add(import_ir_inst); - SemIR::ImportRefUnloaded inst = {.import_ir_inst_id = import_ir_inst_id, - .entity_name_id = entity_name_id}; - auto import_ref_id = AddPlaceholderInstInNoBlock( - context, MakeImportedLocIdAndInst(context, import_ir_inst_id, inst)); - - // ImportRefs have a dedicated block because this may be called during - // processing where the instruction shouldn't be inserted in the current inst - // block. - context.imports().push_back(import_ref_id); + auto import_ref_id = AddPlaceholderImportedInstInNoBlock( + context, + MakeImportedLocIdAndInst( + context, import_ir_inst_id, + SemIR::ImportRefUnloaded{.import_ir_inst_id = import_ir_inst_id, + .entity_name_id = entity_name_id})); return import_ref_id; } @@ -515,11 +512,10 @@ static auto AddLoadedImportRef(ImportContext& context, SemIR::ImportRefLoaded inst = {.type_id = local_type_id, .import_ir_inst_id = import_ir_inst_id, .entity_name_id = SemIR::EntityNameId::None}; - auto inst_id = AddPlaceholderInstInNoBlock( + auto inst_id = AddPlaceholderImportedInstInNoBlock( context.local_context(), MakeImportedLocIdAndInst(context.local_context(), import_ir_inst_id, inst)); - context.local_context().imports().push_back(inst_id); context.local_constant_values().Set(inst_id, local_const_id); context.local_constant_values_for_import_insts().Set(import_inst_id, @@ -561,14 +557,11 @@ template static auto AddPlaceholderImportedInst(ImportContext& context, SemIR::InstId import_inst_id, InstT inst) -> SemIR::InstId { - auto inst_id = context.local_insts().AddInNoBlock(MakeImportedLocIdAndInst( - context.local_context(), AddImportIRInst(context, import_inst_id), inst)); - CARBON_VLOG_TO(context.local_context().vlog_stream(), - "AddPlaceholderImportedInst: {0}\n", inst); - // Track the instruction in the imports block so that it's included in - // formatted SemIR if it's referenced. - context.local_context().imports().push_back(inst_id); - return inst_id; + auto import_ir_inst_id = AddImportIRInst(context, import_inst_id); + return AddPlaceholderImportedInstInNoBlock( + context.local_context(), + MakeImportedLocIdAndInst(context.local_context(), import_ir_inst_id, + inst)); } // Replace an imported instruction that was added by @@ -583,7 +576,7 @@ static auto ReplacePlaceholderImportedInst(ImportContext& context, context.local_insts().Set(inst_id, inst); CARBON_CHECK(context.local_constant_values().Get(inst_id) == - SemIR::ConstantId::NotConstant); + SemIR::ConstantId::None); return SetConstantValue(context.local_context(), inst_id, inst); } diff --git a/toolchain/check/inst.cpp b/toolchain/check/inst.cpp index 6c874553fb67..f1bef4e68bdd 100644 --- a/toolchain/check/inst.cpp +++ b/toolchain/check/inst.cpp @@ -203,6 +203,14 @@ auto AddPlaceholderInstInNoBlock(Context& context, return inst_id; } +auto AddPlaceholderImportedInstInNoBlock(Context& context, + SemIR::LocIdAndInst loc_id_and_inst) + -> SemIR::InstId { + auto inst_id = AddPlaceholderInstInNoBlock(context, loc_id_and_inst); + context.imports().push_back(inst_id); + return inst_id; +} + auto AddPlaceholderInst(Context& context, SemIR::LocIdAndInst loc_id_and_inst) -> SemIR::InstId { auto inst_id = AddPlaceholderInstInNoBlock(context, loc_id_and_inst); diff --git a/toolchain/check/inst.h b/toolchain/check/inst.h index 6b332bab7187..1ca4353a0dfe 100644 --- a/toolchain/check/inst.h +++ b/toolchain/check/inst.h @@ -29,12 +29,16 @@ auto AddInst(Context& context, LocT loc, InstT inst) -> SemIR::InstId { // Like AddInst, but for instructions with a type_id of `TypeType`, which is // encoded in the return type of `TypeInstId`. +inline auto AddTypeInst(Context& context, SemIR::LocIdAndInst loc_id_and_inst) + -> SemIR::TypeInstId { + return context.types().GetAsTypeInstId(AddInst(context, loc_id_and_inst)); +} + template requires(!InstT::Kind.has_cleanup() && std::convertible_to) auto AddTypeInst(Context& context, LocT loc, InstT inst) -> SemIR::TypeInstId { - return context.types().GetAsTypeInstId( - AddInst(context, SemIR::LocIdAndInst(loc, inst))); + return AddTypeInst(context, SemIR::LocIdAndInst(loc, inst)); } // Pushes a parse tree node onto the stack, storing the SemIR::Inst as the @@ -173,6 +177,12 @@ auto AddPlaceholderInstInNoBlock(Context& context, LocT loc, InstT inst) return AddPlaceholderInstInNoBlock(context, SemIR::LocIdAndInst(loc, inst)); } +// Similar to `AddPlaceholderInstInNoBlock`, but also tracks the instruction as +// an import. +auto AddPlaceholderImportedInstInNoBlock(Context& context, + SemIR::LocIdAndInst loc_id_and_inst) + -> SemIR::InstId; + // Replaces the instruction at `inst_id` with `loc_id_and_inst`. The // instruction is required to not have been used in any constant evaluation, // either because it's newly created and entirely unused, or because it's only 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 c10aea4c7c9d..9270b2ba9cf5 100644 --- a/toolchain/check/testdata/basics/raw_sem_ir/cpp_interop.carbon +++ b/toolchain/check/testdata/basics/raw_sem_ir/cpp_interop.carbon @@ -281,8 +281,8 @@ fn G(x: Cpp.X) { // CHECK:STDOUT: 4: inst6000002F // CHECK:STDOUT: 5: inst6000003A // CHECK:STDOUT: 6: inst60000042 -// CHECK:STDOUT: 7: inst6000004E -// CHECK:STDOUT: 8: inst6000004B +// CHECK:STDOUT: 7: inst6000004B +// CHECK:STDOUT: 8: inst6000004E // CHECK:STDOUT: global_init: {} // CHECK:STDOUT: inst_block60000004: // CHECK:STDOUT: 0: inst60000011 diff --git a/toolchain/sem_ir/inst.h b/toolchain/sem_ir/inst.h index e328b46b2cb5..7e42a28532bf 100644 --- a/toolchain/sem_ir/inst.h +++ b/toolchain/sem_ir/inst.h @@ -411,8 +411,12 @@ struct LocIdAndInst { } // Unsafely form a pair of a location and an instruction. Used in the cases - // where we can't statically enforce the type matches. - static auto UncheckedLoc(LocId loc_id, Inst inst) -> LocIdAndInst { + // where we can't statically enforce the type matches. For `ImportIRInstId`, + // use `MakeImportedLocIdAndInst` in `import.h`. + template + requires(std::convertible_to && + !std::same_as) + static auto UncheckedLoc(LocT loc_id, Inst inst) -> LocIdAndInst { return LocIdAndInst(loc_id, inst, /*is_unchecked=*/true); } @@ -428,6 +432,10 @@ struct LocIdAndInst { requires(Internal::HasUntypedNodeId) LocIdAndInst(LocId loc_id, InstT inst) : loc_id(loc_id), inst(inst) {} + // For `ImportIRInstId`, use `MakeImportedLocIdAndInst` in `import.h`. + template + LocIdAndInst(ImportIRInstId loc_id, InstT inst) = delete; + LocId loc_id; Inst inst;