diff --git a/toolchain/check/check.cpp b/toolchain/check/check.cpp index 529326e529c2..4460a057b54d 100644 --- a/toolchain/check/check.cpp +++ b/toolchain/check/check.cpp @@ -9,6 +9,7 @@ #include "toolchain/check/context.h" #include "toolchain/check/diagnostic_helpers.h" #include "toolchain/check/import.h" +#include "toolchain/diagnostics/diagnostic.h" #include "toolchain/diagnostics/diagnostic_emitter.h" #include "toolchain/lex/token_kind.h" #include "toolchain/parse/node_ids.h" @@ -36,38 +37,69 @@ class SemIRDiagnosticConverter : public DiagnosticConverter { const SemIR::File* sem_ir) : node_converters_(node_converters), sem_ir_(sem_ir) {} + // Converts an instruction's location to a diagnostic location, which will be + // the underlying line of code. Adds context for any imports used in the + // current SemIR to get to the underlying code. auto ConvertLocation(SemIRLocation loc, ContextFnT context_fn) const -> DiagnosticLocation override { - // Parse nodes always refer to the current IR. - if (!loc.is_inst_id) { - CARBON_CHECK(loc.loc_id.is_node_id() || !loc.loc_id.is_valid()) - << "TODO: Handle non-NodeId locs"; - return ConvertLocationInFile(sem_ir_, loc.loc_id.node_id(), - loc.token_only, context_fn); + // Cursors for the current IR and instruction in that IR. + const auto* cursor_ir = sem_ir_; + auto cursor_inst_id = SemIR::InstId::Invalid; + + // Notes an import on the diagnostic and updates cursors to point at the + // imported IR. + auto follow_import_ref = [&](SemIR::ImportIRId ir_id, + SemIR::InstId inst_id) { + const auto& import_ir = cursor_ir->import_irs().Get(ir_id); + auto context_loc = ConvertLocationInFile(cursor_ir, import_ir.node_id, + loc.token_only, context_fn); + CARBON_DIAGNOSTIC(InImport, Note, "In import."); + context_fn(context_loc, InImport); + cursor_ir = import_ir.sem_ir; + cursor_inst_id = inst_id; + }; + + // If the location is is an import, follows it and returns nullopt. + // Otherwise, it's a parse node, so return the final location. + auto handle_loc = + [&](SemIR::LocationId loc_id) -> std::optional { + if (loc_id.is_import_ir_inst_id()) { + auto import_ir_inst = + cursor_ir->import_ir_insts().Get(loc_id.import_ir_inst_id()); + follow_import_ref(import_ir_inst.ir_id, import_ir_inst.inst_id); + return std::nullopt; + } else { + // Parse nodes always refer to the current IR. + return ConvertLocationInFile(cursor_ir, loc_id.node_id(), + loc.token_only, context_fn); + } + }; + + // Handle the base location. + if (loc.is_inst_id) { + cursor_inst_id = loc.inst_id; + } else { + if (auto diag_loc = handle_loc(loc.loc_id)) { + return *diag_loc; + } + CARBON_CHECK(cursor_inst_id.is_valid()) << "Should have been set"; } - const auto* cursor_ir = sem_ir_; - auto cursor_inst_id = loc.inst_id; while (true) { // If the parse node is valid, use it for the location. if (auto loc_id = cursor_ir->insts().GetLocationId(cursor_inst_id); loc_id.is_valid()) { - CARBON_CHECK(loc_id.is_node_id()) << "TODO: Handle non-NodeId locs"; - return ConvertLocationInFile(cursor_ir, loc_id.node_id(), - loc.token_only, context_fn); + if (auto diag_loc = handle_loc(loc_id)) { + return *diag_loc; + } + continue; } // If the parse node was invalid, recurse through import references when // possible. if (auto import_ref = cursor_ir->insts().TryGetAs( cursor_inst_id)) { - const auto& import_ir = cursor_ir->import_irs().Get(import_ref->ir_id); - auto context_loc = ConvertLocationInFile(cursor_ir, import_ir.node_id, - loc.token_only, context_fn); - CARBON_DIAGNOSTIC(InImport, Note, "In import."); - context_fn(context_loc, InImport); - cursor_ir = import_ir.sem_ir; - cursor_inst_id = import_ref->inst_id; + follow_import_ref(import_ref->ir_id, import_ref->inst_id); continue; } diff --git a/toolchain/check/context.cpp b/toolchain/check/context.cpp index 13e59a77ce16..054d66ee5868 100644 --- a/toolchain/check/context.cpp +++ b/toolchain/check/context.cpp @@ -117,9 +117,9 @@ auto Context::AddInstAndPush(SemIR::LocationIdAndInst loc_id_and_inst) -> void { node_stack_.Push(loc_id_and_inst.loc_id.node_id(), inst_id); } -auto Context::ReplaceInstBeforeConstantUse( +auto Context::ReplaceLocationIdAndInstBeforeConstantUse( SemIR::InstId inst_id, SemIR::LocationIdAndInst loc_id_and_inst) -> void { - sem_ir().insts().Set(inst_id, loc_id_and_inst); + sem_ir().insts().SetLocationIdAndInst(inst_id, loc_id_and_inst); CARBON_VLOG() << "ReplaceInst: " << inst_id << " -> " << loc_id_and_inst.inst << "\n"; @@ -135,6 +135,23 @@ auto Context::ReplaceInstBeforeConstantUse( constant_values().Set(inst_id, const_id); } +auto Context::ReplaceInstBeforeConstantUse(SemIR::InstId inst_id, + SemIR::Inst inst) -> void { + sem_ir().insts().Set(inst_id, inst); + + CARBON_VLOG() << "ReplaceInst: " << inst_id << " -> " << inst << "\n"; + + // Redo evaluation. This is only safe to do if this instruction has not + // already been used as a constant, which is the caller's responsibility to + // ensure. + auto const_id = TryEvalInst(*this, inst_id, inst); + if (const_id.is_constant()) { + CARBON_VLOG() << "Constant: " << inst << " -> " << const_id.inst_id() + << "\n"; + } + constant_values().Set(inst_id, const_id); +} + auto Context::AddImportRef(SemIR::ImportIRId ir_id, SemIR::InstId inst_id) -> SemIR::InstId { auto import_ref_id = diff --git a/toolchain/check/context.h b/toolchain/check/context.h index c66493604ff9..11fbc2b76976 100644 --- a/toolchain/check/context.h +++ b/toolchain/check/context.h @@ -67,13 +67,19 @@ class Context { // result. Only valid if the LocationId is for a NodeId. auto AddInstAndPush(SemIR::LocationIdAndInst loc_id_and_inst) -> void; - // Replaces the value of the instruction `inst_id` with `loc_id_and_inst`. + // Replaces the instruction `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 used + // in a position that constant evaluation ignores, such as a return slot. + auto ReplaceLocationIdAndInstBeforeConstantUse( + SemIR::InstId inst_id, SemIR::LocationIdAndInst loc_id_and_inst) -> void; + + // Replaces the instruction `inst_id` with `inst`, not affecting location. // 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 used in a position that constant evaluation ignores, such // as a return slot. - auto ReplaceInstBeforeConstantUse(SemIR::InstId inst_id, - SemIR::LocationIdAndInst loc_id_and_inst) + auto ReplaceInstBeforeConstantUse(SemIR::InstId inst_id, SemIR::Inst inst) -> void; // Adds an import_ref instruction for the specified instruction in the diff --git a/toolchain/check/handle_class.cpp b/toolchain/check/handle_class.cpp index 430c16a675cb..78255f38f6f2 100644 --- a/toolchain/check/handle_class.cpp +++ b/toolchain/check/handle_class.cpp @@ -108,7 +108,7 @@ static auto BuildClassDecl(Context& context, Parse::AnyClassDeclId node_id) } // Write the class ID into the ClassDecl. - context.ReplaceInstBeforeConstantUse(class_decl_id, {node_id, class_decl}); + context.ReplaceInstBeforeConstantUse(class_decl_id, class_decl); if (is_new_class) { // Build the `Self` type using the resulting type constant. diff --git a/toolchain/check/handle_function.cpp b/toolchain/check/handle_function.cpp index 01027804244a..399202750e99 100644 --- a/toolchain/check/handle_function.cpp +++ b/toolchain/check/handle_function.cpp @@ -198,8 +198,7 @@ static auto BuildFunctionDecl(Context& context, } // Write the function ID into the FunctionDecl. - context.ReplaceInstBeforeConstantUse(function_info.decl_id, - {node_id, function_decl}); + context.ReplaceInstBeforeConstantUse(function_info.decl_id, function_decl); if (SemIR::IsEntryPoint(context.sem_ir(), function_decl.function_id)) { // TODO: Update this once valid signatures for the entry point are decided. diff --git a/toolchain/check/handle_interface.cpp b/toolchain/check/handle_interface.cpp index c7454921cb87..61847fe3a914 100644 --- a/toolchain/check/handle_interface.cpp +++ b/toolchain/check/handle_interface.cpp @@ -88,8 +88,7 @@ static auto BuildInterfaceDecl(Context& context, } // Write the interface ID into the InterfaceDecl. - context.ReplaceInstBeforeConstantUse(interface_decl_id, - {node_id, interface_decl}); + context.ReplaceInstBeforeConstantUse(interface_decl_id, interface_decl); return {interface_decl.interface_id, interface_decl_id}; } diff --git a/toolchain/check/handle_let.cpp b/toolchain/check/handle_let.cpp index d875e3e4f0a1..7d5a0c17c426 100644 --- a/toolchain/check/handle_let.cpp +++ b/toolchain/check/handle_let.cpp @@ -46,7 +46,7 @@ static auto BuildAssociatedConstantDecl( // declaration. auto name_id = context.bind_names().Get(binding_pattern->bind_name_id).name_id; - context.ReplaceInstBeforeConstantUse( + context.ReplaceLocationIdAndInstBeforeConstantUse( pattern_id, {node_id, SemIR::AssociatedConstantDecl{ binding_pattern->type_id, name_id}}); auto decl_id = pattern_id; @@ -130,8 +130,7 @@ auto HandleLetDecl(Context& context, Parse::LetDeclId node_id) -> bool { CARBON_CHECK(!bind_name.value_id.is_valid()) << "Binding should not already have a value!"; bind_name.value_id = *value_id; - pattern.inst = bind_name; - context.ReplaceInstBeforeConstantUse(pattern_id, pattern); + context.ReplaceInstBeforeConstantUse(pattern_id, bind_name); context.inst_block_stack().AddInstId(pattern_id); // Add the name of the binding to the current scope. diff --git a/toolchain/check/handle_namespace.cpp b/toolchain/check/handle_namespace.cpp index b6dc0e9d0e53..50be9dddb308 100644 --- a/toolchain/check/handle_namespace.cpp +++ b/toolchain/check/handle_namespace.cpp @@ -28,7 +28,7 @@ auto HandleNamespace(Context& context, Parse::NamespaceId node_id) -> bool { namespace_inst.name_scope_id = context.name_scopes().Add( namespace_id, name_context.name_id_for_new_inst(), name_context.enclosing_scope_id_for_new_inst()); - context.ReplaceInstBeforeConstantUse(namespace_id, {node_id, namespace_inst}); + context.ReplaceInstBeforeConstantUse(namespace_id, namespace_inst); auto existing_inst_id = context.decl_name_stack().LookupOrAddName(name_context, namespace_id); diff --git a/toolchain/check/import.cpp b/toolchain/check/import.cpp index 68ae266d76df..8890a2959bf7 100644 --- a/toolchain/check/import.cpp +++ b/toolchain/check/import.cpp @@ -102,7 +102,7 @@ static auto AddNamespace( auto namespace_id = context.AddPlaceholderInst({node_id, namespace_inst}); namespace_inst.name_scope_id = context.name_scopes().Add(namespace_id, name_id, enclosing_scope_id); - context.ReplaceInstBeforeConstantUse(namespace_id, {node_id, namespace_inst}); + context.ReplaceInstBeforeConstantUse(namespace_id, namespace_inst); // Diagnose if there's a name conflict, but still produce the namespace to // supersede the name conflict in order to avoid repeat diagnostics. diff --git a/toolchain/check/import_ref.cpp b/toolchain/check/import_ref.cpp index 5377b37c5e1c..7277ad8534eb 100644 --- a/toolchain/check/import_ref.cpp +++ b/toolchain/check/import_ref.cpp @@ -158,6 +158,11 @@ class ImportRefResolver { return initial_work < work_stack_.size(); } + auto AddImportIRInst(SemIR::InstId inst_id) -> SemIR::LocationId { + return context_.import_ir_insts().Add( + {.ir_id = import_ir_id_, .inst_id = inst_id}); + } + // Returns the ConstantId for an InstId. Adds unresolved constants to // work_stack_. auto GetLocalConstantId(SemIR::InstId inst_id) -> SemIR::ConstantId { @@ -217,14 +222,20 @@ class ImportRefResolver { // TODO: Consider a different parameter handling to simplify import logic. auto inst = import_ir_.insts().Get(ref_id); auto addr_inst = inst.TryAs(); + auto bind_id = ref_id; + auto param_id = ref_id; + if (addr_inst) { bind_id = addr_inst->inner_id; + param_id = bind_id; inst = import_ir_.insts().Get(bind_id); } + auto bind_inst = inst.TryAs(); if (bind_inst) { - inst = import_ir_.insts().Get(bind_inst->value_id); + param_id = bind_inst->value_id; + inst = import_ir_.insts().Get(param_id); } auto param_inst = inst.As(); @@ -233,7 +244,7 @@ class ImportRefResolver { auto type_id = context_.GetTypeIdForTypeConstant(const_id); auto new_param_id = context_.AddInstInNoBlock( - {Parse::NodeId::Invalid, SemIR::Param{type_id, name_id}}); + {AddImportIRInst(param_id), SemIR::Param{type_id, name_id}}); if (bind_inst) { switch (bind_inst->kind) { case SemIR::InstKind::BindName: { @@ -241,7 +252,7 @@ class ImportRefResolver { {.name_id = name_id, .enclosing_scope_id = SemIR::NameScopeId::Invalid}); new_param_id = context_.AddInstInNoBlock( - {Parse::NodeId::Invalid, + {AddImportIRInst(bind_id), SemIR::BindName{type_id, bind_name_id, new_param_id}}); break; } @@ -254,8 +265,7 @@ class ImportRefResolver { new_bind_inst.value_id = new_param_id; // This is not before constant use, but doesn't change the // constant value of the instruction. - context_.ReplaceInstBeforeConstantUse( - bind_id, {Parse::NodeId::Invalid, new_bind_inst}); + context_.ReplaceInstBeforeConstantUse(bind_id, new_bind_inst); break; } default: { @@ -264,9 +274,10 @@ class ImportRefResolver { } } if (addr_inst) { - new_param_id = context_.AddInstInNoBlock( - {Parse::NodeId::Invalid, - SemIR::AddrPattern{type_id, new_param_id}}); + new_param_id = + context_.AddInstInNoBlock(SemIR::LocationIdAndInst::Untyped( + AddImportIRInst(ref_id), + SemIR::AddrPattern{type_id, new_param_id})); } new_param_refs.push_back(new_param_id); } @@ -374,7 +385,7 @@ class ImportRefResolver { return TryResolveTypedInst(inst.As()); case SemIR::InstKind::BaseDecl: - return TryResolveTypedInst(inst.As()); + return TryResolveTypedInst(inst.As(), inst_id); case SemIR::InstKind::BindAlias: return TryResolveTypedInst(inst.As()); @@ -389,7 +400,7 @@ class ImportRefResolver { return TryResolveTypedInst(inst.As()); case SemIR::InstKind::FieldDecl: - return TryResolveTypedInst(inst.As()); + return TryResolveTypedInst(inst.As(), inst_id); case SemIR::InstKind::FunctionDecl: return TryResolveTypedInst(inst.As()); @@ -404,7 +415,7 @@ class ImportRefResolver { return TryResolveTypedInst(inst.As()); case SemIR::InstKind::StructType: - return TryResolveTypedInst(inst.As()); + return TryResolveTypedInst(inst.As(), inst_id); case SemIR::InstKind::TupleType: return TryResolveTypedInst(inst.As()); @@ -417,11 +428,11 @@ class ImportRefResolver { return {TryEvalInst(context_, inst_id, inst)}; case SemIR::InstKind::BindSymbolicName: - return TryResolveTypedInst(inst.As()); + return TryResolveTypedInst(inst.As(), inst_id); default: context_.TODO( - Parse::NodeId(Parse::NodeId::Invalid), + AddImportIRInst(inst_id), llvm::formatv("TryResolveInst on {0}", inst.kind()).str()); return {SemIR::ConstantId::Error}; } @@ -438,7 +449,7 @@ class ImportRefResolver { auto decl_id = context_.AddImportRef(import_ir_id_, inst.decl_id); auto inst_id = context_.AddInstInNoBlock( - {Parse::NodeId::Invalid, + {AddImportIRInst(inst.decl_id), SemIR::AssociatedEntity{ context_.GetTypeIdForTypeConstant(type_const_id), inst.index, decl_id}}); @@ -465,7 +476,8 @@ class ImportRefResolver { return {context_.constant_values().Get(inst_id)}; } - auto TryResolveTypedInst(SemIR::BaseDecl inst) -> ResolveResult { + auto TryResolveTypedInst(SemIR::BaseDecl inst, SemIR::InstId import_inst_id) + -> ResolveResult { auto initial_work = work_stack_.size(); auto type_const_id = GetLocalConstantId(inst.type_id); auto base_type_const_id = GetLocalConstantId(inst.base_type_id); @@ -474,11 +486,11 @@ class ImportRefResolver { } // Import the instruction in order to update contained base_type_id. - auto inst_id = context_.AddInstInNoBlock( - {Parse::NodeId::Invalid, - SemIR::BaseDecl{context_.GetTypeIdForTypeConstant(type_const_id), - context_.GetTypeIdForTypeConstant(base_type_const_id), - inst.index}}); + auto inst_id = context_.AddInstInNoBlock(SemIR::LocationIdAndInst::Untyped( + AddImportIRInst(import_inst_id), + SemIR::BaseDecl{context_.GetTypeIdForTypeConstant(type_const_id), + context_.GetTypeIdForTypeConstant(base_type_const_id), + inst.index})); return {context_.constant_values().Get(inst_id)}; } @@ -491,7 +503,8 @@ class ImportRefResolver { return {value_id}; } - auto TryResolveTypedInst(SemIR::BindSymbolicName inst) -> ResolveResult { + auto TryResolveTypedInst(SemIR::BindSymbolicName inst, + SemIR::InstId import_inst_id) -> ResolveResult { auto initial_work = work_stack_.size(); auto type_id = GetLocalConstantId(inst.type_id); if (HasNewWork(initial_work)) { @@ -504,7 +517,7 @@ class ImportRefResolver { {.name_id = name_id, .enclosing_scope_id = SemIR::NameScopeId::Invalid}); auto new_bind_id = context_.AddInstInNoBlock( - {Parse::NodeId::Invalid, + {AddImportIRInst(import_inst_id), SemIR::BindSymbolicName{context_.GetTypeIdForTypeConstant(type_id), bind_name_id, SemIR::InstId::Invalid}}); return {context_.constant_values().Get(new_bind_id)}; @@ -519,7 +532,8 @@ class ImportRefResolver { SemIR::ClassDecl{SemIR::TypeId::Invalid, SemIR::ClassId::Invalid, SemIR::InstBlockId::Empty}; auto class_decl_id = - context_.AddPlaceholderInst({Parse::NodeId::Invalid, class_decl}); + context_.AddPlaceholderInst(SemIR::LocationIdAndInst::Untyped( + AddImportIRInst(import_class.decl_id), class_decl)); // Regardless of whether ClassDecl is a complete type, we first need an // incomplete type so that any references have something to point at. class_decl.class_id = context_.classes().Add({ @@ -533,8 +547,7 @@ class ImportRefResolver { }); // Write the class ID into the ClassDecl. - context_.ReplaceInstBeforeConstantUse(class_decl_id, - {Parse::NodeId::Invalid, class_decl}); + context_.ReplaceInstBeforeConstantUse(class_decl_id, class_decl); auto self_const_id = context_.constant_values().Get(class_decl_id); // Build the `Self` type using the resulting type constant. @@ -644,16 +657,17 @@ class ImportRefResolver { SemIR::ConstType{SemIR::TypeId::TypeType, inner_type_id})}; } - auto TryResolveTypedInst(SemIR::FieldDecl inst) -> ResolveResult { + auto TryResolveTypedInst(SemIR::FieldDecl inst, SemIR::InstId import_inst_id) + -> ResolveResult { auto initial_work = work_stack_.size(); auto const_id = GetLocalConstantId(inst.type_id); if (HasNewWork(initial_work)) { return ResolveResult::Retry(); } - auto inst_id = context_.AddInstInNoBlock( - {Parse::NodeId::Invalid, - SemIR::FieldDecl{context_.GetTypeIdForTypeConstant(const_id), - GetLocalNameId(inst.name_id), inst.index}}); + auto inst_id = context_.AddInstInNoBlock(SemIR::LocationIdAndInst::Untyped( + AddImportIRInst(import_inst_id), + SemIR::FieldDecl{context_.GetTypeIdForTypeConstant(const_id), + GetLocalNameId(inst.name_id), inst.index})); return {context_.constant_values().Get(inst_id)}; } @@ -684,8 +698,9 @@ class ImportRefResolver { auto function_decl = SemIR::FunctionDecl{ context_.GetTypeIdForTypeConstant(type_const_id), SemIR::FunctionId::Invalid, SemIR::InstBlockId::Empty}; - auto function_decl_id = context_.AddPlaceholderInstInNoBlock( - {Parse::NodeId::Invalid, function_decl}); + auto function_decl_id = + context_.AddPlaceholderInstInNoBlock(SemIR::LocationIdAndInst::Untyped( + AddImportIRInst(function.decl_id), function_decl)); auto new_return_type_id = return_type_const_id.is_valid() @@ -710,10 +725,7 @@ class ImportRefResolver { .is_extern = function.is_extern, .builtin_kind = function.builtin_kind}); // Write the function ID into the FunctionDecl. - // TODO: The Invalid NodeId means it's hard to associate diagnostics. We - // should replace this. - context_.ReplaceInstBeforeConstantUse( - function_decl_id, {Parse::NodeId::Invalid, function_decl}); + context_.ReplaceInstBeforeConstantUse(function_decl_id, function_decl); return {context_.constant_values().Get(function_decl_id)}; } @@ -725,7 +737,8 @@ class ImportRefResolver { SemIR::InterfaceId::Invalid, SemIR::InstBlockId::Empty}; auto interface_decl_id = - context_.AddPlaceholderInst({Parse::NodeId::Invalid, interface_decl}); + context_.AddPlaceholderInst(SemIR::LocationIdAndInst::Untyped( + AddImportIRInst(import_interface.decl_id), interface_decl)); // Start with an incomplete interface. SemIR::Interface new_interface = { @@ -737,8 +750,7 @@ class ImportRefResolver { // Write the interface ID into the InterfaceDecl. interface_decl.interface_id = context_.interfaces().Add(new_interface); - context_.ReplaceInstBeforeConstantUse( - interface_decl_id, {Parse::NodeId::Invalid, interface_decl}); + context_.ReplaceInstBeforeConstantUse(interface_decl_id, interface_decl); // Set the constant value for the imported interface. return context_.constant_values().Get(interface_decl_id); @@ -825,7 +837,8 @@ class ImportRefResolver { context_.GetPointerType(pointee_type_id))}; } - auto TryResolveTypedInst(SemIR::StructType inst) -> ResolveResult { + auto TryResolveTypedInst(SemIR::StructType inst, SemIR::InstId import_inst_id) + -> ResolveResult { // Collect all constants first, locating unresolved ones in a single pass. auto initial_work = work_stack_.size(); CARBON_CHECK(inst.type_id == SemIR::TypeId::TypeType); @@ -851,7 +864,7 @@ class ImportRefResolver { auto name_id = GetLocalNameId(field.name_id); auto field_type_id = context_.GetTypeIdForTypeConstant(field_const_id); fields.push_back(context_.AddInstInNoBlock( - {Parse::NodeId::Invalid, + {AddImportIRInst(import_inst_id), SemIR::StructTypeField{.name_id = name_id, .field_type_id = field_type_id}})); } diff --git a/toolchain/check/pending_block.h b/toolchain/check/pending_block.h index 33f24e49a693..39e639a92749 100644 --- a/toolchain/check/pending_block.h +++ b/toolchain/check/pending_block.h @@ -63,17 +63,17 @@ class PendingBlock { if (insts_.empty()) { // 1) The block is empty. Replace `target_id` with an empty splice // pointing at `value_id`. - context_.ReplaceInstBeforeConstantUse( + context_.ReplaceLocationIdAndInstBeforeConstantUse( target_id, {value.loc_id, SemIR::SpliceBlock{value.inst.type_id(), SemIR::InstBlockId::Empty, value_id}}); } else if (insts_.size() == 1 && insts_[0] == value_id) { // 2) The block is {value_id}. Replace `target_id` with the instruction // referred to by `value_id`. This is intended to be the common case. - context_.ReplaceInstBeforeConstantUse(target_id, value); + context_.ReplaceLocationIdAndInstBeforeConstantUse(target_id, value); } else { // 3) Anything else: splice it into the IR, replacing `target_id`. - context_.ReplaceInstBeforeConstantUse( + context_.ReplaceLocationIdAndInstBeforeConstantUse( target_id, {value.loc_id, SemIR::SpliceBlock{value.inst.type_id(), diff --git a/toolchain/check/testdata/class/cross_package_import.carbon b/toolchain/check/testdata/class/cross_package_import.carbon index b185436797e1..247547f96dea 100644 --- a/toolchain/check/testdata/class/cross_package_import.carbon +++ b/toolchain/check/testdata/class/cross_package_import.carbon @@ -45,10 +45,15 @@ library "extern" api; import Other library "extern"; -// CHECK:STDERR: fail_extern.carbon:[[@LINE+5]]:8: ERROR: Variable has incomplete type `C`. +// CHECK:STDERR: fail_extern.carbon:[[@LINE+10]]:8: ERROR: Variable has incomplete type `C`. // CHECK:STDERR: var c: Other.C = {}; // CHECK:STDERR: ^~~~~~~ -// CHECK:STDERR: fail_extern.carbon: Class was forward declared here. +// CHECK:STDERR: fail_extern.carbon:[[@LINE-5]]:1: In import. +// CHECK:STDERR: import Other library "extern"; +// CHECK:STDERR: ^~~~~~ +// CHECK:STDERR: other_extern.carbon:5:1: Class was forward declared here. +// CHECK:STDERR: class C; +// CHECK:STDERR: ^~~~~~~~ // CHECK:STDERR: var c: Other.C = {}; @@ -213,7 +218,7 @@ var c: Other.C = {}; // CHECK:STDOUT: // CHECK:STDOUT: fn @__global_init() { // CHECK:STDOUT: !entry: -// CHECK:STDOUT: %.loc11: {} = struct_literal () +// CHECK:STDOUT: %.loc16: {} = struct_literal () // CHECK:STDOUT: assign file.%c.var, // CHECK:STDOUT: return // CHECK:STDOUT: } diff --git a/toolchain/check/testdata/class/fail_import_misuses.carbon b/toolchain/check/testdata/class/fail_import_misuses.carbon index 2edb61eec4eb..2b53467d73d5 100644 --- a/toolchain/check/testdata/class/fail_import_misuses.carbon +++ b/toolchain/check/testdata/class/fail_import_misuses.carbon @@ -32,10 +32,15 @@ import library "a"; class Empty { } -// CHECK:STDERR: fail_b.carbon:[[@LINE+4]]:8: ERROR: Variable has incomplete type `Incomplete`. +// CHECK:STDERR: fail_b.carbon:[[@LINE+9]]:8: ERROR: Variable has incomplete type `Incomplete`. // CHECK:STDERR: var a: Incomplete; // CHECK:STDERR: ^~~~~~~~~~ -// CHECK:STDERR: fail_b.carbon: Class was forward declared here. +// CHECK:STDERR: fail_b.carbon:[[@LINE-18]]:1: In import. +// CHECK:STDERR: import library "a"; +// CHECK:STDERR: ^~~~~~ +// CHECK:STDERR: a.carbon:7:1: Class was forward declared here. +// CHECK:STDERR: class Incomplete; +// CHECK:STDERR: ^~~~~~~~~~~~~~~~~ var a: Incomplete; // CHECK:STDOUT: --- a.carbon diff --git a/toolchain/check/testdata/function/definition/import.carbon b/toolchain/check/testdata/function/definition/import.carbon index 28f4261d9c69..e9cfc63ffa91 100644 --- a/toolchain/check/testdata/function/definition/import.carbon +++ b/toolchain/check/testdata/function/definition/import.carbon @@ -37,16 +37,26 @@ library "def_ownership" api; import library "a"; -// CHECK:STDERR: fail_def_ownership.carbon:[[@LINE+5]]:1: ERROR: Only one library can declare function A without `extern`. +// CHECK:STDERR: fail_def_ownership.carbon:[[@LINE+10]]:1: ERROR: Only one library can declare function A without `extern`. // CHECK:STDERR: fn A() {}; // CHECK:STDERR: ^~~~~~~~ -// CHECK:STDERR: fail_def_ownership.carbon: Previously declared here. +// CHECK:STDERR: fail_def_ownership.carbon:[[@LINE-5]]:1: In import. +// CHECK:STDERR: import library "a"; +// CHECK:STDERR: ^~~~~~ +// CHECK:STDERR: a.carbon:4:1: Previously declared here. +// CHECK:STDERR: fn A() {} +// CHECK:STDERR: ^~~~~~~~ // CHECK:STDERR: fn A() {}; -// CHECK:STDERR: fail_def_ownership.carbon:[[@LINE+5]]:1: ERROR: Only one library can declare function B without `extern`. +// CHECK:STDERR: fail_def_ownership.carbon:[[@LINE+10]]:1: ERROR: Only one library can declare function B without `extern`. // CHECK:STDERR: fn B(b: i32) -> i32; // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~ -// CHECK:STDERR: fail_def_ownership.carbon: Previously declared here. +// CHECK:STDERR: fail_def_ownership.carbon:[[@LINE-16]]:1: In import. +// CHECK:STDERR: import library "a"; +// CHECK:STDERR: ^~~~~~ +// CHECK:STDERR: a.carbon:5:1: Previously declared here. +// CHECK:STDERR: fn B(b: i32) -> i32 { return b; } +// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~ // CHECK:STDERR: fn B(b: i32) -> i32; @@ -57,10 +67,15 @@ library "mix_extern_decl" api; import library "a"; extern fn D(); -// CHECK:STDERR: fail_mix_extern_decl.carbon:[[@LINE+4]]:1: ERROR: Only one library can declare function D without `extern`. +// CHECK:STDERR: fail_mix_extern_decl.carbon:[[@LINE+9]]:1: ERROR: Only one library can declare function D without `extern`. // CHECK:STDERR: fn D() {} // CHECK:STDERR: ^~~~~~~~ -// CHECK:STDERR: fail_mix_extern_decl.carbon: Previously declared here. +// CHECK:STDERR: fail_mix_extern_decl.carbon:[[@LINE-6]]:1: In import. +// CHECK:STDERR: import library "a"; +// CHECK:STDERR: ^~~~~~ +// CHECK:STDERR: a.carbon:7:1: Previously declared here. +// CHECK:STDERR: fn D(); +// CHECK:STDERR: ^~~~~~~ fn D() {} // CHECK:STDOUT: --- a.carbon @@ -195,8 +210,8 @@ fn D() {} // CHECK:STDOUT: %import_ref.4 = import_ref ir1, inst+30, unused // CHECK:STDOUT: %A: = fn_decl @A [template] {} // CHECK:STDOUT: %B: = fn_decl @B [template] { -// CHECK:STDOUT: %b.loc17_6.1: i32 = param b -// CHECK:STDOUT: %b.loc17_6.2: i32 = bind_name b, %b.loc17_6.1 +// CHECK:STDOUT: %b.loc27_6.1: i32 = param b +// CHECK:STDOUT: %b.loc27_6.2: i32 = bind_name b, %b.loc27_6.1 // CHECK:STDOUT: %return.var: ref i32 = var // CHECK:STDOUT: } // CHECK:STDOUT: } @@ -222,7 +237,7 @@ fn D() {} // CHECK:STDOUT: %import_ref.3 = import_ref ir1, inst+20, unused // CHECK:STDOUT: %import_ref.4: = import_ref ir1, inst+30, used [template = imports.%D] // CHECK:STDOUT: %D.loc6: = fn_decl @D [template] {} -// CHECK:STDOUT: %D.loc11: = fn_decl @D [template] {} +// CHECK:STDOUT: %D.loc16: = fn_decl @D [template] {} // CHECK:STDOUT: } // CHECK:STDOUT: // CHECK:STDOUT: fn @D() { diff --git a/toolchain/check/testdata/packages/cross_package_import.carbon b/toolchain/check/testdata/packages/cross_package_import.carbon index f0ff127d0f7c..89f806f532f9 100644 --- a/toolchain/check/testdata/packages/cross_package_import.carbon +++ b/toolchain/check/testdata/packages/cross_package_import.carbon @@ -132,10 +132,15 @@ import library "other_ns"; // CHECK:STDERR: import Other library "fn"; -// CHECK:STDERR: fail_main_namespace_conflict.carbon:[[@LINE+5]]:1: ERROR: Only one library can declare function F without `extern`. +// CHECK:STDERR: fail_main_namespace_conflict.carbon:[[@LINE+10]]:1: ERROR: Only one library can declare function F without `extern`. // CHECK:STDERR: fn Other.F() {} // CHECK:STDERR: ^~~~~~~~~~~~~~ -// CHECK:STDERR: fail_main_namespace_conflict.carbon: Previously declared here. +// CHECK:STDERR: fail_main_namespace_conflict.carbon:[[@LINE-5]]:1: In import. +// CHECK:STDERR: import Other library "fn"; +// CHECK:STDERR: ^~~~~~ +// CHECK:STDERR: other_fn.carbon:4:1: Previously declared here. +// CHECK:STDERR: fn F() {} +// CHECK:STDERR: ^~~~~~~~ // CHECK:STDERR: fn Other.F() {} diff --git a/toolchain/sem_ir/inst.h b/toolchain/sem_ir/inst.h index 3ad8f2c46ec3..dcb13b5cfbac 100644 --- a/toolchain/sem_ir/inst.h +++ b/toolchain/sem_ir/inst.h @@ -390,16 +390,21 @@ class InstStore { return loc_ids_[inst_id.index]; } - // Overwrites a given instruction and location ID with a new value. - auto Set(InstId inst_id, LocationIdAndInst loc_id_and_inst) -> void { - values_.Get(inst_id) = loc_id_and_inst.inst; - loc_ids_[inst_id.index] = loc_id_and_inst.loc_id; - } + // Overwrites a given instruction with a new value. + auto Set(InstId inst_id, Inst inst) -> void { values_.Get(inst_id) = inst; } + // Overwrites a given instruction's location with a new value. auto SetLocationId(InstId inst_id, LocationId loc_id) -> void { loc_ids_[inst_id.index] = loc_id; } + // Overwrites a given instruction and location ID with a new value. + auto SetLocationIdAndInst(InstId inst_id, LocationIdAndInst loc_id_and_inst) + -> void { + Set(inst_id, loc_id_and_inst.inst); + SetLocationId(inst_id, loc_id_and_inst.loc_id); + } + // Reserves space. auto Reserve(size_t size) -> void { loc_ids_.reserve(size);