Ver código fonte

Construct LocId from InstId directly (explicitly) instead of doing lookups when possible (#5355)

Remove calls to `InstStore::GetLocId()` to build a LocId from an InstId
now that they can be constructed directly from the InstId. Most uses of
LocId are just plumbing, so this does not affect them. However places
that want to look inside the LocId do not want to work with the InstId
form. In these places, introduce `InstStore::GetResolvedLocId()` which
converts a LocId (or an InstId as an optimization) into a LocId which is
not backed by an InstId. These locations can be printed (they have a
line and column when they are a NodeId), they can have flags added to
them (`ToImplicit`, `ToTokenOnly`), they can be converted to an
underlying ImportIRInstId, or they may be `None`.

`Dump()` is made to print a resolved location instead of printing the
InstId in the location, since (at least in my experience) the resolved
location is what is interesting in debugging, and this saves manual
`MakeInstId` steps in the debugger every time a location is of interest.

The LocId constructor from InstId is made `explicit` to add clarity to
function calls passing an `inst_id` now directly instead of calling
`context.insts().GetLocId(inst_id)`. To avoid needing to construct
`SemIR::LocId(...)` explicitly in all cases though, the diagnostics code
in Check uses `DiagnosticLocId` as its template parameter which accepts
InstId as well and does the construction of LocId from it.

Because LocId now requires an explicit construction from InstId, any
callers to `AddInst()` functions will have to explicitly convert to
LocId if they had an InstId, but not if they pass a NodeId. To make this
difference clear to callers, we `requires` that the input type can be
converted to LocId. This ensures that passing an InstId results in an
error at the callsite where the InstId is passed, instead of generating
a compiler error when trying to construct `LocIdAndInst` inside
`AddInst()`, which is less clear about what went wrong and doesn't seem
entirely intentional.

---------

Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Dana Jansens 1 ano atrás
pai
commit
315e206ff1
52 arquivos alterados com 335 adições e 243 exclusões
  1. 2 2
      toolchain/check/call.cpp
  2. 1 1
      toolchain/check/check_unit.cpp
  3. 4 0
      toolchain/check/context.cpp
  4. 1 0
      toolchain/check/context.h
  5. 2 2
      toolchain/check/control_flow.h
  6. 14 16
      toolchain/check/convert.cpp
  7. 8 7
      toolchain/check/decl_name_stack.cpp
  8. 5 3
      toolchain/check/diagnostic_emitter.cpp
  9. 1 1
      toolchain/check/diagnostic_emitter.h
  10. 18 5
      toolchain/check/diagnostic_helpers.h
  11. 9 9
      toolchain/check/dump.cpp
  12. 9 8
      toolchain/check/eval.cpp
  13. 21 19
      toolchain/check/eval_inst.cpp
  14. 17 17
      toolchain/check/facet_type.cpp
  15. 9 9
      toolchain/check/generic.cpp
  16. 1 1
      toolchain/check/handle_array.cpp
  17. 2 1
      toolchain/check/handle_binding_pattern.cpp
  18. 6 4
      toolchain/check/handle_class.cpp
  19. 6 1
      toolchain/check/handle_export.cpp
  20. 1 1
      toolchain/check/handle_expr_statement.cpp
  21. 10 8
      toolchain/check/handle_function.cpp
  22. 5 4
      toolchain/check/handle_impl.cpp
  23. 1 2
      toolchain/check/handle_index.cpp
  24. 5 4
      toolchain/check/handle_interface.cpp
  25. 2 1
      toolchain/check/handle_let_and_var.cpp
  26. 3 2
      toolchain/check/handle_name.cpp
  27. 6 3
      toolchain/check/handle_namespace.cpp
  28. 7 5
      toolchain/check/handle_operator.cpp
  29. 7 6
      toolchain/check/impl.cpp
  30. 1 1
      toolchain/check/impl_lookup.cpp
  31. 9 9
      toolchain/check/import.cpp
  32. 5 4
      toolchain/check/import_ref.cpp
  33. 16 6
      toolchain/check/inst.h
  34. 1 1
      toolchain/check/interface.cpp
  35. 3 5
      toolchain/check/member_access.cpp
  36. 2 2
      toolchain/check/modifiers.cpp
  37. 7 6
      toolchain/check/name_lookup.cpp
  38. 1 1
      toolchain/check/name_lookup.h
  39. 10 6
      toolchain/check/operator.cpp
  40. 20 23
      toolchain/check/pattern_match.cpp
  41. 2 0
      toolchain/check/pending_block.h
  42. 2 1
      toolchain/check/subst.cpp
  43. 2 2
      toolchain/diagnostics/diagnostic_emitter.h
  44. 3 2
      toolchain/lower/file_context.cpp
  45. 5 4
      toolchain/sem_ir/absolute_node_id.cpp
  46. 3 0
      toolchain/sem_ir/absolute_node_id.h
  47. 2 2
      toolchain/sem_ir/formatter.cpp
  48. 26 15
      toolchain/sem_ir/ids.h
  49. 1 1
      toolchain/sem_ir/ids_test.cpp
  50. 1 1
      toolchain/sem_ir/import_ir.cpp
  51. 23 3
      toolchain/sem_ir/inst.h
  52. 7 6
      toolchain/sem_ir/inst_namer.cpp

+ 2 - 2
toolchain/check/call.cpp

@@ -151,7 +151,7 @@ static auto BuildCalleeSpecificFunction(
     // specific function in the impl that corresponds to the specific function
     // we deduced.
     callee_id =
-        GetOrAddInst(context, context.insts().GetLocId(generic_callee_id),
+        GetOrAddInst(context, SemIR::LocId(generic_callee_id),
                      SemIR::SpecificImplFunction{
                          .type_id = GetSingletonType(
                              context, SemIR::SpecificFunctionType::TypeInstId),
@@ -161,7 +161,7 @@ static auto BuildCalleeSpecificFunction(
     // This is a regular generic function. The callee is the specific function
     // we deduced.
     callee_id =
-        GetOrAddInst(context, context.insts().GetLocId(generic_callee_id),
+        GetOrAddInst(context, SemIR::LocId(generic_callee_id),
                      SemIR::SpecificFunction{
                          .type_id = GetSingletonType(
                              context, SemIR::SpecificFunctionType::TypeInstId),

+ 1 - 1
toolchain/check/check_unit.cpp

@@ -409,7 +409,7 @@ auto CheckUnit::CheckRequiredDeclarations() -> void {
     if (!function.first_owning_decl_id.has_value() &&
         function.extern_library_id == context_.sem_ir().library_id()) {
       auto function_loc_id =
-          context_.insts().GetLocId(function.non_owning_decl_id);
+          context_.insts().GetCanonicalLocId(function.non_owning_decl_id);
       CARBON_CHECK(function_loc_id.kind() ==
                    SemIR::LocId::Kind::ImportIRInstId);
       auto import_ir_id = context_.sem_ir()

+ 4 - 0
toolchain/check/context.cpp

@@ -43,6 +43,10 @@ auto Context::TODO(SemIR::LocId loc_id, std::string label) -> bool {
   return false;
 }
 
+auto Context::TODO(SemIR::InstId loc_inst_id, std::string label) -> bool {
+  return TODO(SemIR::LocId(loc_inst_id), label);
+}
+
 auto Context::VerifyOnFinish() const -> void {
   // Information in all the various context objects should be cleaned up as
   // various pieces of context go out of scope. At this point, nothing should

+ 1 - 0
toolchain/check/context.h

@@ -58,6 +58,7 @@ class Context {
 
   // Marks an implementation TODO. Always returns false.
   auto TODO(SemIR::LocId loc_id, std::string label) -> bool;
+  auto TODO(SemIR::InstId loc_inst_id, std::string label) -> bool;
 
   // Runs verification that the processing cleanly finished.
   auto VerifyOnFinish() const -> void;

+ 2 - 2
toolchain/check/control_flow.h

@@ -72,7 +72,7 @@ auto MaybeAddCleanupForInst(Context& context, SemIR::TypeId type_id,
 
 // Adds an instruction that has cleanup associated.
 template <typename InstT, typename LocT>
-  requires(InstT::Kind.has_cleanup())
+  requires(InstT::Kind.has_cleanup() && std::convertible_to<LocT, SemIR::LocId>)
 auto AddInstWithCleanup(Context& context, LocT loc, InstT inst)
     -> SemIR::InstId {
   auto inst_id = AddInst(context, SemIR::LocIdAndInst(loc, inst));
@@ -82,7 +82,7 @@ auto AddInstWithCleanup(Context& context, LocT loc, InstT inst)
 
 // Adds an instruction that has cleanup associated.
 template <typename InstT, typename LocT>
-  requires(InstT::Kind.has_cleanup())
+  requires(InstT::Kind.has_cleanup() && std::convertible_to<LocT, SemIR::LocId>)
 auto AddInstWithCleanupInNoBlock(Context& context, LocT loc, InstT inst)
     -> SemIR::InstId {
   auto inst_id = AddInstInNoBlock(context, SemIR::LocIdAndInst(loc, inst));

+ 14 - 16
toolchain/check/convert.cpp

@@ -71,7 +71,7 @@ static auto FinalizeTemporary(Context& context, SemIR::InstId init_id,
                  "initialized multiple times? Have {0}",
                  sem_ir.insts().Get(return_slot_arg_id));
     auto init = sem_ir.insts().Get(init_id);
-    return AddInst<SemIR::Temporary>(context, sem_ir.insts().GetLocId(init_id),
+    return AddInst<SemIR::Temporary>(context, SemIR::LocId(init_id),
                                      {.type_id = init.type_id(),
                                       .storage_id = return_slot_arg_id,
                                       .init_id = init_id});
@@ -87,10 +87,9 @@ static auto FinalizeTemporary(Context& context, SemIR::InstId init_id,
   // TODO: Consider using `None` to mean that we immediately materialize and
   // initialize a temporary, rather than two separate instructions.
   auto init = sem_ir.insts().Get(init_id);
-  auto loc_id = sem_ir.insts().GetLocId(init_id);
   auto temporary_id = AddInstWithCleanup<SemIR::TemporaryStorage>(
-      context, loc_id, {.type_id = init.type_id()});
-  return AddInst<SemIR::Temporary>(context, loc_id,
+      context, SemIR::LocId(init_id), {.type_id = init.type_id()});
+  return AddInst<SemIR::Temporary>(context, SemIR::LocId(init_id),
                                    {.type_id = init.type_id(),
                                     .storage_id = temporary_id,
                                     .init_id = init_id});
@@ -200,7 +199,7 @@ static auto ConvertTupleToArray(Context& context, SemIR::TupleType tuple_type,
   auto tuple_elem_types = sem_ir.inst_blocks().Get(tuple_type.type_elements_id);
 
   auto value = sem_ir.insts().Get(value_id);
-  auto value_loc_id = sem_ir.insts().GetLocId(value_id);
+  SemIR::LocId value_loc_id(value_id);
 
   // If we're initializing from a tuple literal, we will use its elements
   // directly. Otherwise, materialize a temporary if needed and index into the
@@ -300,7 +299,7 @@ static auto ConvertTupleToTuple(Context& context, SemIR::TupleType src_type,
   auto dest_elem_types = sem_ir.inst_blocks().Get(dest_type.type_elements_id);
 
   auto value = sem_ir.insts().Get(value_id);
-  auto value_loc_id = sem_ir.insts().GetLocId(value_id);
+  SemIR::LocId value_loc_id(value_id);
 
   // If we're initializing from a tuple literal, we will use its elements
   // directly. Otherwise, materialize a temporary if needed and index into the
@@ -398,7 +397,7 @@ static auto ConvertStructToStructOrClass(
   auto dest_elem_fields_size = dest_elem_fields.size() - dest_vptr_offset;
 
   auto value = sem_ir.insts().Get(value_id);
-  auto value_loc_id = sem_ir.insts().GetLocId(value_id);
+  SemIR::LocId value_loc_id(value_id);
 
   // If we're initializing from a struct literal, we will use its elements
   // directly. Otherwise, materialize a temporary if needed and index into the
@@ -585,7 +584,7 @@ static auto ConvertStructToClass(
     target.kind = ConversionTarget::Initializer;
     target.init_block = &target_block;
     target.init_id = target_block.AddInstWithCleanup<SemIR::TemporaryStorage>(
-        context.insts().GetLocId(value_id), {.type_id = target.type_id});
+        SemIR::LocId(value_id), {.type_id = target.type_id});
   }
 
   auto result_id = ConvertStructToStructOrClass<SemIR::ClassElementAccess>(
@@ -594,11 +593,10 @@ static auto ConvertStructToClass(
 
   if (need_temporary) {
     target_block.InsertHere();
-    result_id =
-        AddInst<SemIR::Temporary>(context, context.insts().GetLocId(value_id),
-                                  {.type_id = target.type_id,
-                                   .storage_id = target.init_id,
-                                   .init_id = result_id});
+    result_id = AddInst<SemIR::Temporary>(context, SemIR::LocId(value_id),
+                                          {.type_id = target.type_id,
+                                           .storage_id = target.init_id,
+                                           .init_id = result_id});
   }
   return result_id;
 }
@@ -1358,7 +1356,7 @@ auto Convert(Context& context, SemIR::LocId loc_id, SemIR::InstId expr_id,
       // If we have a reference and don't want one, form a value binding.
       // TODO: Support types with custom value representations.
       expr_id = AddInst<SemIR::BindValue>(
-          context, context.insts().GetLocId(expr_id),
+          context, SemIR::LocId(expr_id),
           {.type_id = target.type_id, .value_id = expr_id});
       // We now have a value expression.
       [[fallthrough]];
@@ -1398,14 +1396,14 @@ auto Initialize(Context& context, SemIR::LocId loc_id, SemIR::InstId target_id,
 
 auto ConvertToValueExpr(Context& context, SemIR::InstId expr_id)
     -> SemIR::InstId {
-  return Convert(context, context.insts().GetLocId(expr_id), expr_id,
+  return Convert(context, SemIR::LocId(expr_id), expr_id,
                  {.kind = ConversionTarget::Value,
                   .type_id = context.insts().Get(expr_id).type_id()});
 }
 
 auto ConvertToValueOrRefExpr(Context& context, SemIR::InstId expr_id)
     -> SemIR::InstId {
-  return Convert(context, context.insts().GetLocId(expr_id), expr_id,
+  return Convert(context, SemIR::LocId(expr_id), expr_id,
                  {.kind = ConversionTarget::ValueOrRef,
                   .type_id = context.insts().Get(expr_id).type_id()});
 }

+ 8 - 7
toolchain/check/decl_name_stack.cpp

@@ -183,7 +183,7 @@ auto DeclNameStack::AddNameOrDiagnose(NameContext name_context,
                          name_context.poisoning_loc_id, name_context.loc_id);
   } else if (auto id = name_context.prev_inst_id(); id.has_value()) {
     DiagnoseDuplicateName(*context_, name_context.name_id, name_context.loc_id,
-                          id);
+                          SemIR::LocId(id));
   } else {
     AddName(name_context, target_id, access_kind);
   }
@@ -443,14 +443,14 @@ auto DeclNameStack::ResolveAsScope(const NameContext& name_context,
       // This is specifically for qualified name handling.
       if (!CheckRedeclParamsMatch(
               *context_, new_params,
-              DeclParams(name_context.resolved_inst_id, Parse::NodeId::None,
-                         Parse::NodeId::None, SemIR::InstBlockId::None,
-                         SemIR::InstBlockId::None))) {
+              DeclParams(SemIR::LocId(name_context.resolved_inst_id),
+                         Parse::NodeId::None, Parse::NodeId::None,
+                         SemIR::InstBlockId::None, SemIR::InstBlockId::None))) {
         return InvalidResult;
       }
       if (scope.is_closed_import()) {
         DiagnoseQualifiedDeclInImportedPackage(*context_, name_context.loc_id,
-                                               scope.inst_id());
+                                               SemIR::LocId(scope.inst_id()));
         // Only error once per package. Recover by allowing this package name to
         // be used as a name qualifier.
         scope.set_is_closed_import(false);
@@ -458,8 +458,9 @@ auto DeclNameStack::ResolveAsScope(const NameContext& name_context,
       return {scope_id, SemIR::GenericId::None};
     }
     default: {
-      DiagnoseQualifiedDeclInNonScope(*context_, name_context.loc_id,
-                                      name_context.resolved_inst_id);
+      DiagnoseQualifiedDeclInNonScope(
+          *context_, name_context.loc_id,
+          SemIR::LocId(name_context.resolved_inst_id));
       return InvalidResult;
     }
   }

+ 5 - 3
toolchain/check/diagnostic_emitter.cpp

@@ -14,18 +14,20 @@
 
 namespace Carbon::Check {
 
-auto DiagnosticEmitter::ConvertLoc(SemIR::LocId loc_id,
+auto DiagnosticEmitter::ConvertLoc(LocIdForDiagnostics loc_id,
                                    ContextFnT context_fn) const
     -> Diagnostics::ConvertedLoc {
   // TODO: Instead of special casing Clang location here, support it within
   // `GetAbsoluteNodeId()`. See discussion in
   // https://github.com/carbon-language/carbon-lang/pull/5262/files/20a3f9dcfab5c6f6c5089554fd5e22d5f1ca75a3#r2040308805.
-  auto converted_clang_loc = TryConvertClangDiagnosticLoc(loc_id);
+  auto converted_clang_loc =
+      TryConvertClangDiagnosticLoc(static_cast<SemIR::LocId>(loc_id));
   if (converted_clang_loc) {
     return *converted_clang_loc;
   }
 
-  auto converted = ConvertLocImpl(loc_id, context_fn);
+  auto converted =
+      ConvertLocImpl(static_cast<SemIR::LocId>(loc_id), context_fn);
 
   // Use the token when possible, but -1 is the default value.
   auto last_offset = -1;

+ 1 - 1
toolchain/check/diagnostic_emitter.h

@@ -43,7 +43,7 @@ class DiagnosticEmitter : public DiagnosticEmitterBase {
   // For the last byte offset, this uses `last_token_` exclusively for imported
   // locations, or `loc` if it's in the same file and (for whatever reason)
   // later.
-  auto ConvertLoc(SemIR::LocId loc_id, ContextFnT context_fn) const
+  auto ConvertLoc(LocIdForDiagnostics loc_id, ContextFnT context_fn) const
       -> Diagnostics::ConvertedLoc override;
 
  private:

+ 18 - 5
toolchain/check/diagnostic_helpers.h

@@ -5,20 +5,33 @@
 #ifndef CARBON_TOOLCHAIN_CHECK_DIAGNOSTIC_HELPERS_H_
 #define CARBON_TOOLCHAIN_CHECK_DIAGNOSTIC_HELPERS_H_
 
+#include <concepts>
+
 #include "llvm/ADT/APSInt.h"
 #include "toolchain/parse/node_ids.h"
 #include "toolchain/sem_ir/ids.h"
 
 namespace Carbon::Check {
 
-// TODO: Consider instead changing calls to `SemIR::LocId::TokenOnly(...)`.
-inline auto TokenOnly(SemIR::LocId loc_id) -> SemIR::LocId {
-  return loc_id.ToTokenOnly();
-}
+// The `DiagnosticEmitterBase` is templated on this type so that
+// diagnostics can be passed an `InstId` as a location, without having to
+// explicitly construct a `LocId` from it first.
+class LocIdForDiagnostics {
+ public:
+  template <class LocT>
+    requires std::constructible_from<SemIR::LocId, LocT>
+  // NOLINTNEXTLINE(google-explicit-constructor)
+  LocIdForDiagnostics(LocT loc_id) : loc_id_(SemIR::LocId(loc_id)) {}
+
+  explicit operator SemIR::LocId() const { return loc_id_; }
+
+ private:
+  SemIR::LocId loc_id_;
+};
 
 // We define the emitter separately for dependencies, so only provide a base
 // here.
-using DiagnosticEmitterBase = Diagnostics::Emitter<SemIR::LocId>;
+using DiagnosticEmitterBase = Diagnostics::Emitter<LocIdForDiagnostics>;
 
 using DiagnosticBuilder = DiagnosticEmitterBase::Builder;
 

+ 9 - 9
toolchain/check/dump.cpp

@@ -81,8 +81,7 @@ LLVM_DUMP_METHOD static auto Dump(const Context& context, SemIR::ImplId impl_id)
     return out.TakeStr();
   }
   const auto& impl = context.sem_ir().impls().Get(impl_id);
-  auto loc_id = context.sem_ir().insts().GetLocId(impl.witness_id);
-  out << "\nwitness loc: " << Dump(context, loc_id);
+  out << "\nwitness loc: " << Dump(context, SemIR::LocId(impl.witness_id));
   return out.TakeStr();
 }
 
@@ -95,9 +94,8 @@ LLVM_DUMP_METHOD static auto Dump(const Context& context,
 LLVM_DUMP_METHOD static auto Dump(const Context& context, SemIR::InstId inst_id)
     -> std::string {
   RawStringOstream out;
-  auto loc_id = context.sem_ir().insts().GetLocId(inst_id);
   out << SemIR::Dump(context.sem_ir(), inst_id) << '\n'
-      << "  - " << Dump(context, loc_id);
+      << "  - " << Dump(context, SemIR::LocId(inst_id));
   return out.TakeStr();
 }
 
@@ -110,6 +108,10 @@ LLVM_DUMP_METHOD static auto Dump(const Context& context,
 LLVM_DUMP_METHOD static auto Dump(const Context& context, SemIR::LocId loc_id)
     -> std::string {
   RawStringOstream out;
+  // TODO: If the canonical location is None but the original is an InstId,
+  // should we dump the InstId anyway even though it has no location? Is that
+  // ever useful?
+  loc_id = context.sem_ir().insts().GetCanonicalLocId(loc_id);
   switch (loc_id.kind()) {
     case SemIR::LocId::Kind::None: {
       out << "LocId(<none>)";
@@ -128,11 +130,6 @@ LLVM_DUMP_METHOD static auto Dump(const Context& context, SemIR::LocId loc_id)
       break;
     }
 
-    case SemIR::LocId::Kind::InstId: {
-      out << "LocId(" << SemIR::Dump(context.sem_ir(), loc_id.inst_id()) << ")";
-      break;
-    }
-
     case SemIR::LocId::Kind::NodeId: {
       auto token = context.parse_tree().node_token(loc_id.node_id());
       auto line = context.tokens().GetLineNumber(token);
@@ -142,6 +139,9 @@ LLVM_DUMP_METHOD static auto Dump(const Context& context, SemIR::LocId loc_id)
           << line << ":" << col << implicit << ")";
       break;
     }
+
+    case SemIR::LocId::Kind::InstId:
+      CARBON_FATAL("unexpected LocId kind");
   }
   return out.TakeStr();
 }

+ 9 - 8
toolchain/check/eval.cpp

@@ -64,9 +64,11 @@ class EvalContext {
   auto GetDiagnosticLoc(llvm::ArrayRef<SemIR::InstId> inst_ids)
       -> SemIR::LocId {
     for (auto inst_id : inst_ids) {
-      if (inst_id.has_value() &&
-          context_->insts().GetLocId(inst_id).has_value()) {
-        return inst_id;
+      if (inst_id.has_value()) {
+        auto loc_id = context_->insts().GetCanonicalLocId(inst_id);
+        if (loc_id.has_value()) {
+          return loc_id;
+        }
       }
     }
     return fallback_loc_id_;
@@ -745,7 +747,7 @@ static auto ReplaceAllFieldsWithConstantValues(EvalContext& eval_context,
 
 auto AddImportedConstant(Context& context, SemIR::Inst inst)
     -> SemIR::ConstantId {
-  EvalContext eval_context(&context, SemIR::InstId::None);
+  EvalContext eval_context(&context, SemIR::LocId::None);
   Phase phase = Phase::Concrete;
   switch (inst.kind().value_kind()) {
     case SemIR::InstValueKind::Typed:
@@ -1683,7 +1685,7 @@ static auto MakeConstantForCall(EvalContext& eval_context, SemIR::LocId loc_id,
 
 // Given an instruction, compute its phase based on its operands.
 static auto ComputeInstPhase(Context& context, SemIR::Inst inst) -> Phase {
-  EvalContext eval_context(&context, SemIR::InstId::None);
+  EvalContext eval_context(&context, SemIR::LocId::None);
 
   auto phase = GetPhase(context.constant_values(),
                         context.types().GetConstantId(inst.type_id()));
@@ -1753,8 +1755,7 @@ static auto TryEvalTypedInst(EvalContext& eval_context, SemIR::InstId inst_id,
       return MakeConstantResult(eval_context.context(), inst, phase);
     } else if constexpr (ConstantKind == SemIR::InstConstantKind::InstAction) {
       auto result_inst_id = PerformDelayedAction(
-          eval_context.context(), eval_context.insts().GetLocId(inst_id),
-          inst.As<InstT>());
+          eval_context.context(), SemIR::LocId(inst_id), inst.As<InstT>());
       if (result_inst_id.has_value()) {
         // The result is an instruction.
         return MakeConstantResult(
@@ -1975,7 +1976,7 @@ static auto TryEvalInstInContext(EvalContext& eval_context,
 
 auto TryEvalInstUnsafe(Context& context, SemIR::InstId inst_id,
                        SemIR::Inst inst) -> SemIR::ConstantId {
-  EvalContext eval_context(&context, inst_id);
+  EvalContext eval_context(&context, SemIR::LocId(inst_id));
   return TryEvalInstInContext(eval_context, inst_id, inst);
 }
 

+ 21 - 19
toolchain/check/eval_inst.cpp

@@ -183,7 +183,7 @@ auto EvalConstantInst(Context& context, SemIR::FacetAccessType inst)
 
 auto EvalConstantInst(Context& context, SemIR::InstId inst_id,
                       SemIR::FloatType inst) -> ConstantEvalResult {
-  return ValidateFloatType(context, inst_id, inst)
+  return ValidateFloatType(context, SemIR::LocId(inst_id), inst)
              ? ConstantEvalResult::NewSamePhase(inst)
              : ConstantEvalResult::Error;
 }
@@ -206,9 +206,8 @@ auto EvalConstantInst(Context& context, SemIR::InstId inst_id,
   inst.query_self_inst_id =
       GetCanonicalizedFacetOrTypeValue(context, inst.query_self_inst_id);
 
-  auto result =
-      EvalLookupSingleImplWitness(context, context.insts().GetLocId(inst_id),
-                                  inst, non_canonical_query_self_inst_id);
+  auto result = EvalLookupSingleImplWitness(
+      context, SemIR::LocId(inst_id), inst, non_canonical_query_self_inst_id);
   if (!result.has_value()) {
     // We use NotConstant to communicate back to impl lookup that the lookup
     // failed. This can not happen for a deferred symbolic lookup in a generic
@@ -277,7 +276,7 @@ auto EvalConstantInst(Context& context, SemIR::InitializeFrom inst)
 
 auto EvalConstantInst(Context& context, SemIR::InstId inst_id,
                       SemIR::IntType inst) -> ConstantEvalResult {
-  return ValidateIntType(context, inst_id, inst)
+  return ValidateIntType(context, SemIR::LocId(inst_id), inst)
              ? ConstantEvalResult::NewSamePhase(inst)
              : ConstantEvalResult::Error;
 }
@@ -315,17 +314,18 @@ auto EvalConstantInst(Context& context, SemIR::InstId inst_id,
   auto complete_type_id =
       context.types().GetTypeIdForTypeInstId(inst.complete_type_inst_id);
   if (complete_type_id.is_concrete()) {
-    if (!TryToCompleteType(context, complete_type_id, inst_id, [&] {
-          CARBON_DIAGNOSTIC(IncompleteTypeInMonomorphization, Error,
-                            "{0} evaluates to incomplete type {1}",
-                            InstIdAsType, InstIdAsType);
-          return context.emitter().Build(
-              inst_id, IncompleteTypeInMonomorphization,
-              context.insts()
-                  .GetAs<SemIR::RequireCompleteType>(inst_id)
-                  .complete_type_inst_id,
-              inst.complete_type_inst_id);
-        })) {
+    if (!TryToCompleteType(
+            context, complete_type_id, SemIR::LocId(inst_id), [&] {
+              CARBON_DIAGNOSTIC(IncompleteTypeInMonomorphization, Error,
+                                "{0} evaluates to incomplete type {1}",
+                                InstIdAsType, InstIdAsType);
+              return context.emitter().Build(
+                  inst_id, IncompleteTypeInMonomorphization,
+                  context.insts()
+                      .GetAs<SemIR::RequireCompleteType>(inst_id)
+                      .complete_type_inst_id,
+                  inst.complete_type_inst_id);
+            })) {
       return ConstantEvalResult::Error;
     }
     return ConstantEvalResult::NewSamePhase(SemIR::CompleteTypeWitness{
@@ -392,8 +392,10 @@ auto EvalConstantInst(Context& context, SemIR::InstId inst_id,
   CARBON_CHECK(static_cast<int>(interface_fn_args.size()) >= remaining_params);
   args.append(interface_fn_args.end() - remaining_params,
               interface_fn_args.end());
-  auto specific_id = MakeSpecific(context, inst_id, generic_id, args);
-  context.definitions_required_by_use().push_back({inst_id, specific_id});
+  auto specific_id =
+      MakeSpecific(context, SemIR::LocId(inst_id), generic_id, args);
+  context.definitions_required_by_use().push_back(
+      {SemIR::LocId(inst_id), specific_id});
 
   return ConstantEvalResult::NewSamePhase(
       SemIR::SpecificFunction{.type_id = inst.type_id,
@@ -408,7 +410,7 @@ auto EvalConstantInst(Context& context, SemIR::InstId inst_id,
     // This is not an associated function. Those will be required to be defined
     // as part of checking that the impl is complete.
     context.definitions_required_by_use().push_back(
-        {inst_id, inst.specific_id});
+        {SemIR::LocId(inst_id), inst.specific_id});
   }
   // Create new constant for a specific function.
   return ConstantEvalResult::NewSamePhase(inst);

+ 17 - 17
toolchain/check/facet_type.cpp

@@ -80,12 +80,11 @@ auto InitialFacetTypeImplWitness(
          .specific_id = self_specific_id});
   }
 
-  if (!RequireCompleteType(context, facet_type_id,
-                           context.insts().GetLocId(facet_type_inst_id), [&] {
-                             return IncompleteFacetTypeDiagnosticBuilder(
-                                 context, witness_loc_id, facet_type_inst_id,
-                                 is_definition);
-                           })) {
+  if (!RequireCompleteType(
+          context, facet_type_id, SemIR::LocId(facet_type_inst_id), [&] {
+            return IncompleteFacetTypeDiagnosticBuilder(
+                context, witness_loc_id, facet_type_inst_id, is_definition);
+          })) {
     return SemIR::ErrorInst::InstId;
   }
 
@@ -191,15 +190,16 @@ auto InitialFacetTypeImplWitness(
       // Get the type of the associated constant in this interface with this
       // value for `Self`.
       assoc_const_type_id = GetTypeForSpecificAssociatedEntity(
-          context, facet_type_inst_id, interface_to_witness.specific_id,
-          decl_id, context.types().GetTypeIdForTypeInstId(self_type_inst_id),
+          context, SemIR::LocId(facet_type_inst_id),
+          interface_to_witness.specific_id, decl_id,
+          context.types().GetTypeIdForTypeInstId(self_type_inst_id),
           witness_inst_id);
       // Perform the conversion of the value to the type. We skipped this when
       // forming the facet type because the type of the associated constant
       // was symbolic.
-      auto converted_inst_id = ConvertToValueOfType(
-          context, context.insts().GetLocId(facet_type_inst_id),
-          rewrite_inst_id, assoc_const_type_id);
+      auto converted_inst_id =
+          ConvertToValueOfType(context, SemIR::LocId(facet_type_inst_id),
+                               rewrite_inst_id, assoc_const_type_id);
       // Canonicalize the converted constant value.
       converted_inst_id =
           context.constant_values().GetConstantInstId(converted_inst_id);
@@ -239,12 +239,12 @@ auto RequireCompleteFacetTypeForImplDefinition(
     -> bool {
   auto facet_type_id =
       context.types().GetTypeIdForTypeInstId(facet_type_inst_id);
-  return RequireCompleteType(context, facet_type_id,
-                             context.insts().GetLocId(facet_type_inst_id), [&] {
-                               return IncompleteFacetTypeDiagnosticBuilder(
-                                   context, loc_id, facet_type_inst_id,
-                                   /*is_definition=*/true);
-                             });
+  return RequireCompleteType(
+      context, facet_type_id, SemIR::LocId(facet_type_inst_id), [&] {
+        return IncompleteFacetTypeDiagnosticBuilder(context, loc_id,
+                                                    facet_type_inst_id,
+                                                    /*is_definition=*/true);
+      });
 }
 
 auto AllocateFacetTypeImplWitness(Context& context,

+ 9 - 9
toolchain/check/generic.cpp

@@ -256,8 +256,8 @@ static auto AddGenericConstantToEvalBlock(
   // we've not encountered it before.
   auto const_inst_id = context.constant_values().GetConstantInstId(inst_id);
   auto callbacks = RebuildGenericConstantInEvalBlockCallbacks(
-      &context, generic_id, region, context.insts().GetLocId(inst_id),
-      constants_in_generic, inside_redeclaration);
+      &context, generic_id, region, SemIR::LocId(inst_id), constants_in_generic,
+      inside_redeclaration);
   auto new_inst_id = SubstInst(context, const_inst_id, callbacks);
   CARBON_CHECK(new_inst_id != const_inst_id,
                "No substitutions performed for generic constant {0}",
@@ -275,11 +275,11 @@ static auto AddTemplateActionToEvalBlock(
     ConstantsInGenericMap& constants_in_generic, bool inside_redeclaration,
     SemIR::InstId inst_id) -> void {
   // Substitute into the constant value and rebuild it in the eval block.
-  auto new_inst_id = SubstInst(
-      context, inst_id,
-      RebuildTemplateActionInEvalBlockCallbacks(
-          &context, generic_id, region, context.insts().GetLocId(inst_id),
-          constants_in_generic, inside_redeclaration, inst_id));
+  auto new_inst_id =
+      SubstInst(context, inst_id,
+                RebuildTemplateActionInEvalBlockCallbacks(
+                    &context, generic_id, region, SemIR::LocId(inst_id),
+                    constants_in_generic, inside_redeclaration, inst_id));
   CARBON_CHECK(new_inst_id == inst_id,
                "Substitution changed InstId of template action");
   constants_in_generic.Insert(inst_id, inst_id);
@@ -341,7 +341,7 @@ static auto MakeGenericEvalBlock(Context& context, SemIR::GenericId generic_id,
         GenericRegionStack::DependencyKind::None) {
       auto inst = context.insts().Get(inst_id);
       auto type_id = AddGenericTypeToEvalBlock(
-          context, generic_id, region, context.insts().GetLocId(inst_id),
+          context, generic_id, region, SemIR::LocId(inst_id),
           constants_in_generic, inside_redeclaration, inst.type_id());
       // If the generic declaration is invalid, it can result in an error.
       if (type_id == SemIR::ErrorInst::TypeId) {
@@ -485,7 +485,7 @@ auto BuildGenericDecl(Context& context, SemIR::InstId decl_id)
     -> SemIR::GenericId {
   SemIR::GenericId generic_id = BuildGeneric(context, decl_id);
   if (generic_id.has_value()) {
-    FinishGenericDecl(context, decl_id, generic_id);
+    FinishGenericDecl(context, SemIR::LocId(decl_id), generic_id);
   }
   return generic_id;
 }

+ 1 - 1
toolchain/check/handle_array.cpp

@@ -48,7 +48,7 @@ auto HandleParseNode(Context& context, Parse::ArrayExprId node_id) -> bool {
   }
 
   bound_inst_id = ConvertToValueOfType(
-      context, context.insts().GetLocId(bound_inst_id), bound_inst_id,
+      context, SemIR::LocId(bound_inst_id), bound_inst_id,
       GetSingletonType(context, SemIR::IntLiteralType::TypeInstId));
   AddInstAndPush<SemIR::ArrayType>(
       context, node_id,

+ 2 - 1
toolchain/check/handle_binding_pattern.cpp

@@ -407,7 +407,8 @@ auto HandleParseNode(Context& context, Parse::AddrId node_id) -> bool {
   } else {
     CARBON_DIAGNOSTIC(AddrOnNonSelfParam, Error,
                       "`addr` can only be applied to a `self` parameter");
-    context.emitter().Emit(TokenOnly(node_id), AddrOnNonSelfParam);
+    context.emitter().Emit(SemIR::LocId(node_id).ToTokenOnly(),
+                           AddrOnNonSelfParam);
     context.node_stack().Push(node_id, param_pattern_id);
   }
   return true;

+ 6 - 4
toolchain/check/handle_class.cpp

@@ -57,7 +57,7 @@ static auto MergeClassRedecl(Context& context, Parse::AnyClassDeclId node_id,
                              SemIR::ClassId prev_class_id,
                              SemIR::ImportIRId prev_import_ir_id) -> bool {
   auto& prev_class = context.classes().Get(prev_class_id);
-  SemIR::LocId prev_loc_id = prev_class.latest_decl_id();
+  SemIR::LocId prev_loc_id(prev_class.latest_decl_id());
 
   // Check the generic parameters match, if they were specified.
   if (!CheckRedeclParamsMatch(context, DeclParams(new_class),
@@ -158,7 +158,7 @@ static auto MergeOrAddName(Context& context, Parse::AnyClassDeclId node_id,
   if (!prev_class_id.has_value()) {
     // This is a redeclaration of something other than a class.
     DiagnoseDuplicateName(context, name_context.name_id, name_context.loc_id,
-                          prev_id);
+                          SemIR::LocId(prev_id));
     return;
   }
 
@@ -368,7 +368,8 @@ auto HandleParseNode(Context& context, Parse::AdaptDeclId node_id) -> bool {
 
   auto& class_info = context.classes().Get(parent_class_decl->class_id);
   if (class_info.adapt_id.has_value()) {
-    DiagnoseClassSpecificDeclRepeated(context, node_id, class_info.adapt_id,
+    DiagnoseClassSpecificDeclRepeated(context, node_id,
+                                      SemIR::LocId(class_info.adapt_id),
                                       Lex::TokenKind::Adapt);
     return true;
   }
@@ -502,7 +503,8 @@ auto HandleParseNode(Context& context, Parse::BaseDeclId node_id) -> bool {
 
   auto& class_info = context.classes().Get(parent_class_decl->class_id);
   if (class_info.base_id.has_value()) {
-    DiagnoseClassSpecificDeclRepeated(context, node_id, class_info.base_id,
+    DiagnoseClassSpecificDeclRepeated(context, node_id,
+                                      SemIR::LocId(class_info.base_id),
                                       Lex::TokenKind::Base);
     return true;
   }

+ 6 - 1
toolchain/check/handle_export.cpp

@@ -62,7 +62,12 @@ auto HandleParseNode(Context& context, Parse::ExportDeclId node_id) -> bool {
     context.emitter()
         .Build(node_id, ExportRedundant)
         // Use the location of the export itself, not the exported instruction.
-        .Note(context.insts().GetLocId(inst_id), ExportPrevious)
+        //
+        // TODO: This construction of a LocId that does not just contain the
+        // InstId prevents GetAbsoluteNodeIdImpl() from seeing the `ExportDecl`
+        // instruction, which prevents it from chasing through it to the entity
+        // being exported. It might be nice to make this more explicit.
+        .Note(context.insts().GetCanonicalLocId(inst_id), ExportPrevious)
         .Emit();
     return true;
   }

+ 1 - 1
toolchain/check/handle_expr_statement.cpp

@@ -16,7 +16,7 @@ static auto HandleDiscardedExpr(Context& context, SemIR::InstId expr_id)
   // If we discard an initializing expression, convert it to a value or
   // reference so that it has something to initialize.
   auto expr = context.insts().Get(expr_id);
-  Convert(context, context.insts().GetLocId(expr_id), expr_id,
+  Convert(context, SemIR::LocId(expr_id), expr_id,
           {.kind = ConversionTarget::Discarded, .type_id = expr.type_id()});
 
   // TODO: This will eventually need to do some "do not discard" analysis.

+ 10 - 8
toolchain/check/handle_function.cpp

@@ -165,7 +165,7 @@ static auto MergeFunctionRedecl(Context& context,
   DiagnoseIfInvalidRedecl(
       context, Lex::TokenKind::Fn, prev_function.name_id,
       RedeclInfo(new_function, node_id, new_is_definition),
-      RedeclInfo(prev_function, prev_function.latest_decl_id(),
+      RedeclInfo(prev_function, SemIR::LocId(prev_function.latest_decl_id()),
                  prev_function.has_definition_started()),
       prev_import_ir_id);
   if (new_is_definition && prev_function.has_definition_started()) {
@@ -246,7 +246,7 @@ static auto TryMergeRedecl(Context& context, Parse::AnyFunctionDeclId node_id,
 
   if (!prev_function_id.has_value()) {
     DiagnoseDuplicateName(context, name_context.name_id, name_context.loc_id,
-                          prev_id);
+                          SemIR::LocId(prev_id));
     return;
   }
 
@@ -553,7 +553,8 @@ static auto BuildFunctionDecl(Context& context,
           SemIR::Function::VirtualModifier::Abstract) {
     CARBON_DIAGNOSTIC(DefinedAbstractFunction, Error,
                       "definition of `abstract` function");
-    context.emitter().Emit(TokenOnly(node_id), DefinedAbstractFunction);
+    context.emitter().Emit(SemIR::LocId(node_id).ToTokenOnly(),
+                           DefinedAbstractFunction);
   }
 
   // Add to name lookup if needed, now that the decl is built.
@@ -584,9 +585,9 @@ static auto CheckFunctionDefinitionSignature(Context& context,
 
   // Check the return type is complete.
   if (function.return_slot_pattern_id.has_value()) {
-    CheckFunctionReturnType(
-        context, context.insts().GetLocId(function.return_slot_pattern_id),
-        function, SemIR::SpecificId::None);
+    CheckFunctionReturnType(context,
+                            SemIR::LocId(function.return_slot_pattern_id),
+                            function, SemIR::SpecificId::None);
     params_to_complete = params_to_complete.drop_back();
   }
 
@@ -599,7 +600,7 @@ static auto CheckFunctionDefinitionSignature(Context& context,
     // The parameter types need to be complete.
     RequireCompleteType(
         context, context.insts().GetAs<SemIR::AnyParam>(param_ref_id).type_id,
-        context.insts().GetLocId(param_ref_id), [&] {
+        SemIR::LocId(param_ref_id), [&] {
           CARBON_DIAGNOSTIC(
               IncompleteTypeInFunctionParam, Error,
               "parameter has incomplete type {0} in function definition",
@@ -672,7 +673,8 @@ auto HandleParseNode(Context& context, Parse::FunctionDefinitionId node_id)
       CARBON_DIAGNOSTIC(
           MissingReturnStatement, Error,
           "missing `return` at end of function with declared return type");
-      context.emitter().Emit(TokenOnly(node_id), MissingReturnStatement);
+      context.emitter().Emit(SemIR::LocId(node_id).ToTokenOnly(),
+                             MissingReturnStatement);
     } else {
       AddReturnCleanupBlock(context, node_id);
     }

+ 5 - 4
toolchain/check/handle_impl.cpp

@@ -191,8 +191,8 @@ static auto ExtendImpl(Context& context, Parse::NodeId extend_node,
     parent_scope.set_has_error();
   } else {
     bool is_complete = RequireCompleteType(
-        context, constraint_type_id,
-        context.insts().GetLocId(constraint_type_inst_id), [&] {
+        context, constraint_type_id, SemIR::LocId(constraint_type_inst_id),
+        [&] {
           CARBON_DIAGNOSTIC(ExtendImplAsIncomplete, Error,
                             "`extend impl as` incomplete facet type {0}",
                             InstIdAsType);
@@ -423,7 +423,8 @@ static auto BuildImplDecl(Context& context, Parse::AnyImplDeclId node_id,
       // also look into the facet (eg, so you can name associated constants from
       // within the impl).
     }
-    FinishGenericDecl(context, impl_decl_id, impl_info.generic_id);
+    FinishGenericDecl(context, SemIR::LocId(impl_decl_id),
+                      impl_info.generic_id);
     impl_decl.impl_id = context.impls().Add(impl_info);
     lookup_bucket_ref.push_back(impl_decl.impl_id);
 
@@ -491,7 +492,7 @@ static auto BuildImplDecl(Context& context, Parse::AnyImplDeclId node_id,
     auto extend_node = introducer.modifier_node_id(ModifierOrder::Extend);
     if (impl_info.generic_id.has_value()) {
       constraint_type_inst_id = AddTypeInst<SemIR::SpecificConstant>(
-          context, context.insts().GetLocId(constraint_type_inst_id),
+          context, SemIR::LocId(constraint_type_inst_id),
           {.type_id = SemIR::TypeType::TypeId,
            .inst_id = constraint_type_inst_id,
            .specific_id =

+ 1 - 2
toolchain/check/handle_index.cpp

@@ -136,9 +136,8 @@ auto HandleParseNode(Context& context, Parse::IndexExprId node_id) -> bool {
 
   CARBON_KIND_SWITCH(context.types().GetAsInst(operand_type_id)) {
     case CARBON_KIND(SemIR::ArrayType array_type): {
-      auto index_loc_id = context.insts().GetLocId(index_inst_id);
       auto cast_index_id = ConvertToValueOfType(
-          context, index_loc_id, index_inst_id,
+          context, SemIR::LocId(index_inst_id), index_inst_id,
           // TODO: Replace this with impl lookup rather than hardcoding `i32`.
           MakeIntType(context, node_id, SemIR::IntKind::Signed,
                       context.ints().Add(32)));

+ 5 - 4
toolchain/check/handle_interface.cpp

@@ -81,8 +81,8 @@ static auto BuildInterfaceDecl(Context& context,
           context.interfaces().Get(existing_interface_decl->interface_id);
       if (CheckRedeclParamsMatch(
               context,
-              DeclParams(interface_decl_id, name.first_param_node_id,
-                         name.last_param_node_id,
+              DeclParams(SemIR::LocId(interface_decl_id),
+                         name.first_param_node_id, name.last_param_node_id,
                          name.implicit_param_patterns_id,
                          name.param_patterns_id),
               DeclParams(existing_interface))) {
@@ -92,7 +92,8 @@ static auto BuildInterfaceDecl(Context& context,
         DiagnoseIfInvalidRedecl(
             context, Lex::TokenKind::Interface, existing_interface.name_id,
             RedeclInfo(interface_info, node_id, is_definition),
-            RedeclInfo(existing_interface, existing_interface.latest_decl_id(),
+            RedeclInfo(existing_interface,
+                       SemIR::LocId(existing_interface.latest_decl_id()),
                        existing_interface.has_definition_started()),
             /*prev_import_ir_id=*/SemIR::ImportIRId::None);
 
@@ -109,7 +110,7 @@ static auto BuildInterfaceDecl(Context& context,
     } else {
       // This is a redeclaration of something other than a interface.
       DiagnoseDuplicateName(context, name_context.name_id, name_context.loc_id,
-                            existing_id);
+                            SemIR::LocId(existing_id));
     }
   }
 

+ 2 - 1
toolchain/check/handle_let_and_var.cpp

@@ -364,7 +364,8 @@ auto HandleParseNode(Context& context, Parse::LetDeclId node_id) -> bool {
     CARBON_DIAGNOSTIC(
         ExpectedInitializerAfterLet, Error,
         "expected `=`; `let` declaration must have an initializer");
-    context.emitter().Emit(TokenOnly(node_id), ExpectedInitializerAfterLet);
+    context.emitter().Emit(SemIR::LocId(node_id).ToTokenOnly(),
+                           ExpectedInitializerAfterLet);
   }
   return true;
 }

+ 3 - 2
toolchain/check/handle_name.cpp

@@ -54,8 +54,9 @@ auto HandleParseNode(Context& context, Parse::PointerMemberAccessExprId node_id)
                       "cannot apply `->` operator to non-pointer type {0}",
                       SemIR::TypeId);
 
-    auto builder = context.emitter().Build(
-        TokenOnly(node_id), ArrowOperatorOfNonPointer, not_pointer_type_id);
+    auto builder =
+        context.emitter().Build(SemIR::LocId(node_id).ToTokenOnly(),
+                                ArrowOperatorOfNonPointer, not_pointer_type_id);
     builder.Emit();
   };
 

+ 6 - 3
toolchain/check/handle_namespace.cpp

@@ -71,7 +71,8 @@ auto HandleParseNode(Context& context, Parse::NamespaceId node_id) -> bool {
               .is_closed_import()) {
         // The existing name is a package name, so this is a name conflict.
         DiagnoseDuplicateName(context, name_context.name_id,
-                              name_context.loc_id, existing_inst_id);
+                              name_context.loc_id,
+                              SemIR::LocId(existing_inst_id));
 
         // Treat this as a local namespace name from now on to avoid further
         // diagnostics.
@@ -79,14 +80,16 @@ auto HandleParseNode(Context& context, Parse::NamespaceId node_id) -> bool {
             .Get(existing->name_scope_id)
             .set_is_closed_import(false);
       } else if (existing->import_id.has_value() &&
-                 !context.insts().GetLocId(existing_inst_id).has_value()) {
+                 !context.insts()
+                      .GetCanonicalLocId(existing_inst_id)
+                      .has_value()) {
         // When the name conflict is an imported namespace, fill the location ID
         // so that future diagnostics point at this declaration.
         SetNamespaceNodeId(context, existing_inst_id, node_id);
       }
     } else {
       DiagnoseDuplicateName(context, name_context.name_id, name_context.loc_id,
-                            existing_inst_id);
+                            SemIR::LocId(existing_inst_id));
     }
   }
 

+ 7 - 5
toolchain/check/handle_operator.cpp

@@ -237,13 +237,14 @@ auto HandleParseNode(Context& context, Parse::PrefixOperatorAmpId node_id)
     case SemIR::ExprCategory::EphemeralRef:
       CARBON_DIAGNOSTIC(AddrOfEphemeralRef, Error,
                         "cannot take the address of a temporary object");
-      context.emitter().Emit(TokenOnly(node_id), AddrOfEphemeralRef);
+      context.emitter().Emit(SemIR::LocId(node_id).ToTokenOnly(),
+                             AddrOfEphemeralRef);
       value_id = SemIR::ErrorInst::InstId;
       break;
     default:
       CARBON_DIAGNOSTIC(AddrOfNonRef, Error,
                         "cannot take the address of non-reference expression");
-      context.emitter().Emit(TokenOnly(node_id), AddrOfNonRef);
+      context.emitter().Emit(SemIR::LocId(node_id).ToTokenOnly(), AddrOfNonRef);
       value_id = SemIR::ErrorInst::InstId;
       break;
   }
@@ -325,15 +326,16 @@ auto HandleParseNode(Context& context, Parse::PrefixOperatorStarId node_id)
                           "cannot dereference operand of non-pointer type {0}",
                           SemIR::TypeId);
 
-        auto builder = context.emitter().Build(
-            TokenOnly(node_id), DerefOfNonPointer, not_pointer_type_id);
+        auto builder =
+            context.emitter().Build(SemIR::LocId(node_id).ToTokenOnly(),
+                                    DerefOfNonPointer, not_pointer_type_id);
 
         // TODO: Check for any facet here, rather than only a type.
         if (not_pointer_type_id == SemIR::TypeType::TypeId) {
           CARBON_DIAGNOSTIC(
               DerefOfType, Note,
               "to form a pointer type, write the `*` after the pointee type");
-          builder.Note(TokenOnly(node_id), DerefOfType);
+          builder.Note(SemIR::LocId(node_id).ToTokenOnly(), DerefOfType);
         }
 
         builder.Emit();

+ 7 - 6
toolchain/check/impl.cpp

@@ -69,7 +69,8 @@ static auto CheckAssociatedFunctionImplementation(
   // parameters.
   auto interface_function_specific_id =
       GetSelfSpecificForInterfaceMemberWithSelfType(
-          context, impl_decl_id, interface_function_type.specific_id,
+          context, SemIR::LocId(impl_decl_id),
+          interface_function_type.specific_id,
           context.functions()
               .Get(interface_function_type.function_id)
               .generic_id,
@@ -97,8 +98,8 @@ auto ImplWitnessForDeclaration(Context& context, const SemIR::Impl& impl,
   }
 
   return InitialFacetTypeImplWitness(
-      context, context.insts().GetLocId(impl.latest_decl_id()),
-      impl.constraint_id, impl.self_id, impl.interface,
+      context, SemIR::LocId(impl.latest_decl_id()), impl.constraint_id,
+      impl.self_id, impl.interface,
       context.generics().GetSelfSpecific(impl.generic_id), has_definition);
 }
 
@@ -122,7 +123,7 @@ auto ImplWitnessStartDefinition(Context& context, SemIR::Impl& impl) -> void {
   if (witness_table.elements_id != SemIR::InstBlockId::Empty &&
       witness_block.empty()) {
     if (!RequireCompleteFacetTypeForImplDefinition(
-            context, impl.latest_decl_id(), impl.constraint_id)) {
+            context, SemIR::LocId(impl.latest_decl_id()), impl.constraint_id)) {
       return;
     }
 
@@ -205,8 +206,8 @@ auto FinishImplWitness(Context& context, SemIR::Impl& impl) -> void {
         }
         auto& fn = context.functions().Get(fn_type->function_id);
         auto lookup_result =
-            LookupNameInExactScope(context, context.insts().GetLocId(decl_id),
-                                   fn.name_id, impl.scope_id, impl_scope);
+            LookupNameInExactScope(context, SemIR::LocId(decl_id), fn.name_id,
+                                   impl.scope_id, impl_scope);
         if (lookup_result.is_found()) {
           used_decl_ids.push_back(lookup_result.target_inst_id());
           witness_value = CheckAssociatedFunctionImplementation(

+ 1 - 1
toolchain/check/impl_lookup.cpp

@@ -369,7 +369,7 @@ static auto GetOrAddLookupImplWitness(Context& context, SemIR::LocId loc_id,
                                       SemIR::SpecificInterface interface)
     -> SemIR::InstId {
   auto witness_const_id = EvalOrAddInst(
-      context, loc_id.ToImplicit(),
+      context, context.insts().GetCanonicalLocId(loc_id).ToImplicit(),
       SemIR::LookupImplWitness{
           .type_id = GetSingletonType(context, SemIR::WitnessType::TypeInstId),
           .query_self_inst_id =

+ 9 - 9
toolchain/check/import.cpp

@@ -103,20 +103,18 @@ static auto MakeImportedNamespaceLocIdAndInst(Context& context,
     return SemIR::LocIdAndInst::NoLoc(namespace_inst);
   }
 
-  SemIR::LocId import_loc_id = context.insts().GetLocId(import_id);
-  if (!import_loc_id.has_value()) {
-    // TODO: Either document the use-case for this, or require a location.
-    return SemIR::LocIdAndInst::NoLoc(namespace_inst);
-  }
+  SemIR::LocId import_loc_id = context.insts().GetCanonicalLocId(import_id);
   switch (import_loc_id.kind()) {
     case SemIR::LocId::Kind::ImportIRInstId:
       return MakeImportedLocIdAndInst(
           context, import_loc_id.import_ir_inst_id(), namespace_inst);
     case SemIR::LocId::Kind::NodeId:
-    case SemIR::LocId::Kind::None:
       return SemIR::LocIdAndInst(context.parse_tree().As<Parse::AnyNamespaceId>(
                                      import_loc_id.node_id()),
                                  namespace_inst);
+    case SemIR::LocId::Kind::None:
+      // TODO: Either document the use-case for this, or require a location.
+      return SemIR::LocIdAndInst::NoLoc(namespace_inst);
     case SemIR::LocId::Kind::InstId:
       CARBON_FATAL("Unexpected LocId kind");
   }
@@ -164,7 +162,8 @@ auto AddImportNamespaceToScope(
           CARBON_CHECK(import_id.has_value());
           // TODO: Pass the import package name location instead of the import
           // id to get more accurate location.
-          DiagnoseDuplicateName(context, name_id, import_id, prev_inst_id);
+          DiagnoseDuplicateName(context, name_id, SemIR::LocId(import_id),
+                                SemIR::LocId(prev_inst_id));
         }
         return {.add_result = {.name_scope_id = namespace_inst->name_scope_id,
                                .inst_id = prev_inst_id},
@@ -192,8 +191,9 @@ auto AddImportNamespaceToScope(
   if (!lookup_result.is_poisoned() && !inserted) {
     // TODO: Pass the import namespace name location instead of the namespace
     // id to get more accurate location.
-    DiagnoseDuplicateName(context, name_id, result.add_result.inst_id,
-                          lookup_result.target_inst_id());
+    DiagnoseDuplicateName(context, name_id,
+                          SemIR::LocId(result.add_result.inst_id),
+                          SemIR::LocId(lookup_result.target_inst_id()));
   }
   lookup_result = SemIR::ScopeLookupResult::MakeFound(
       result.add_result.inst_id, SemIR::AccessKind::Public);

+ 5 - 4
toolchain/check/import_ref.cpp

@@ -148,7 +148,8 @@ auto VerifySameCanonicalImportIRInst(Context& context, SemIR::NameId name_id,
   auto conflict_id =
       AddImportRef(context, SemIR::ImportIRInst(new_ir_id, new_inst_id));
   // TODO: Pass the imported name location instead of the conflict id.
-  DiagnoseDuplicateName(context, name_id, conflict_id, prev_id);
+  DiagnoseDuplicateName(context, name_id, SemIR::LocId(conflict_id),
+                        SemIR::LocId(prev_id));
 }
 
 // Returns an instruction that has the specified constant value.
@@ -590,7 +591,7 @@ class ImportRefResolver : public ImportContext {
     auto cursor_inst_id = inst_id;
 
     while (true) {
-      auto loc_id = cursor_ir->insts().GetLocId(cursor_inst_id);
+      auto loc_id = cursor_ir->insts().GetCanonicalLocId(cursor_inst_id);
       if (loc_id.kind() != SemIR::LocId::Kind::ImportIRInstId) {
         return result;
       }
@@ -3176,8 +3177,8 @@ static auto FinishPendingGeneric(ImportRefResolver& resolver,
   resolver.local_generics().Get(pending.local_id).decl_block_id = decl_block_id;
 
   auto local_decl_id = resolver.local_generics().Get(pending.local_id).decl_id;
-  auto self_specific_id = MakeSelfSpecific(resolver.local_context(),
-                                           local_decl_id, pending.local_id);
+  auto self_specific_id = MakeSelfSpecific(
+      resolver.local_context(), SemIR::LocId(local_decl_id), pending.local_id);
   resolver.local_generics().Get(pending.local_id).self_specific_id =
       self_specific_id;
   resolver.AddPendingSpecific({.import_id = import_generic.self_specific_id,

+ 16 - 6
toolchain/check/inst.h

@@ -5,6 +5,8 @@
 #ifndef CARBON_TOOLCHAIN_CHECK_INST_H_
 #define CARBON_TOOLCHAIN_CHECK_INST_H_
 
+#include <concepts>
+
 #include "toolchain/check/context.h"
 #include "toolchain/sem_ir/ids.h"
 #include "toolchain/sem_ir/inst.h"
@@ -19,7 +21,8 @@ auto AddInst(Context& context, SemIR::LocIdAndInst loc_id_and_inst)
 //
 // As a safety check, prevent use with storage insts (see `AddInstWithCleanup`).
 template <typename InstT, typename LocT>
-  requires(!InstT::Kind.has_cleanup())
+  requires(!InstT::Kind.has_cleanup() &&
+           std::convertible_to<LocT, SemIR::LocId>)
 auto AddInst(Context& context, LocT loc, InstT inst) -> SemIR::InstId {
   return AddInst(context, SemIR::LocIdAndInst(loc, inst));
 }
@@ -27,7 +30,8 @@ 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`.
 template <typename InstT, typename LocT>
-  requires(!InstT::Kind.has_cleanup())
+  requires(!InstT::Kind.has_cleanup() &&
+           std::convertible_to<LocT, SemIR::LocId>)
 auto AddTypeInst(Context& context, LocT loc, InstT inst) -> SemIR::TypeInstId {
   return context.types().GetAsTypeInstId(
       AddInst(context, SemIR::LocIdAndInst(loc, inst)));
@@ -54,7 +58,8 @@ auto AddInstInNoBlock(Context& context, SemIR::LocIdAndInst loc_id_and_inst)
 //
 // As a safety check, prevent use with storage insts (see `AddInstWithCleanup`).
 template <typename InstT, typename LocT>
-  requires(!InstT::Kind.has_cleanup())
+  requires(!InstT::Kind.has_cleanup() &&
+           std::convertible_to<LocT, SemIR::LocId>)
 auto AddInstInNoBlock(Context& context, LocT loc, InstT inst) -> SemIR::InstId {
   return AddInstInNoBlock(context, SemIR::LocIdAndInst(loc, inst));
 }
@@ -68,7 +73,8 @@ auto GetOrAddInst(Context& context, SemIR::LocIdAndInst loc_id_and_inst)
 //
 // As a safety check, prevent use with storage insts (see `AddInstWithCleanup`).
 template <typename InstT, typename LocT>
-  requires(!InstT::Kind.has_cleanup())
+  requires(!InstT::Kind.has_cleanup() &&
+           std::convertible_to<LocT, SemIR::LocId>)
 auto GetOrAddInst(Context& context, LocT loc, InstT inst) -> SemIR::InstId {
   return GetOrAddInst(context, SemIR::LocIdAndInst(loc, inst));
 }
@@ -84,7 +90,8 @@ auto EvalOrAddInst(Context& context, SemIR::LocIdAndInst loc_id_and_inst)
 //
 // As a safety check, prevent use with storage insts (see `AddInstWithCleanup`).
 template <typename InstT, typename LocT>
-  requires(!InstT::Kind.has_cleanup())
+  requires(!InstT::Kind.has_cleanup() &&
+           std::convertible_to<LocT, SemIR::LocId>)
 auto EvalOrAddInst(Context& context, LocT loc, InstT inst)
     -> SemIR::ConstantId {
   return EvalOrAddInst(context, SemIR::LocIdAndInst(loc, inst));
@@ -99,6 +106,7 @@ auto AddDependentActionInst(Context& context,
 
 // Convenience wrapper for AddDependentActionInst.
 template <typename InstT, typename LocT>
+  requires std::convertible_to<LocT, SemIR::LocId>
 auto AddDependentActionInst(Context& context, LocT loc, InstT inst)
     -> SemIR::InstId {
   return AddDependentActionInst(context, SemIR::LocIdAndInst(loc, inst));
@@ -107,6 +115,7 @@ auto AddDependentActionInst(Context& context, LocT loc, InstT inst)
 // Like AddDependentActionInst, but for instructions with a type_id of
 // `TypeType`, which is encoded in the return type of `TypeInstId`.
 template <typename InstT, typename LocT>
+  requires std::convertible_to<LocT, SemIR::LocId>
 auto AddDependentActionTypeInst(Context& context, LocT loc, InstT inst)
     -> SemIR::TypeInstId {
   return context.types().GetAsTypeInstId(
@@ -158,7 +167,8 @@ auto AddPlaceholderInstInNoBlock(Context& context,
 //
 // As a safety check, prevent use with storage insts (see `AddInstWithCleanup`).
 template <typename InstT, typename LocT>
-  requires(!InstT::Kind.has_cleanup())
+  requires(!InstT::Kind.has_cleanup() &&
+           std::convertible_to<LocT, SemIR::LocId>)
 auto AddPlaceholderInstInNoBlock(Context& context, LocT loc, InstT inst)
     -> SemIR::InstId {
   return AddPlaceholderInstInNoBlock(context, SemIR::LocIdAndInst(loc, inst));

+ 1 - 1
toolchain/check/interface.cpp

@@ -43,7 +43,7 @@ auto BuildAssociatedEntity(Context& context, SemIR::InterfaceId interface_id,
   auto type_id =
       GetAssociatedEntityType(context, interface_id, interface_specific_id);
   return AddInst<SemIR::AssociatedEntity>(
-      context, context.insts().GetLocId(decl_id),
+      context, SemIR::LocId(decl_id),
       {.type_id = type_id, .index = index, .decl_id = decl_id});
 }
 

+ 3 - 5
toolchain/check/member_access.cpp

@@ -69,7 +69,7 @@ static auto GetHighestAllowedAccess(Context& context, SemIR::LocId loc_id,
                                     SemIR::ConstantId name_scope_const_id)
     -> SemIR::AccessKind {
   SemIR::ScopeLookupResult lookup_result =
-      LookupUnqualifiedName(context, loc_id.node_id(), SemIR::NameId::SelfType,
+      LookupUnqualifiedName(context, loc_id, SemIR::NameId::SelfType,
                             /*required=*/false)
           .scope_result;
   CARBON_CHECK(!lookup_result.is_poisoned());
@@ -473,8 +473,7 @@ static auto PerformActionHelper(Context& context, SemIR::LocId loc_id,
 
   // If the base isn't a scope, it must have a complete type.
   auto base_type_id = context.insts().Get(base_id).type_id();
-  auto base_loc_id = context.insts().GetLocId(base_id);
-  if (!RequireCompleteType(context, base_type_id, base_loc_id, [&] {
+  if (!RequireCompleteType(context, base_type_id, SemIR::LocId(base_id), [&] {
         CARBON_DIAGNOSTIC(IncompleteTypeInMemberAccess, Error,
                           "member access into object of incomplete type {0}",
                           TypeOfInstId);
@@ -735,9 +734,8 @@ auto PerformTupleAccess(Context& context, SemIR::LocId loc_id,
   }
 
   SemIR::TypeId element_type_id = SemIR::ErrorInst::TypeId;
-  auto index_node_id = context.insts().GetLocId(index_inst_id);
   index_inst_id = ConvertToValueOfType(
-      context, index_node_id, index_inst_id,
+      context, SemIR::LocId(index_inst_id), index_inst_id,
       GetSingletonType(context, SemIR::IntLiteralType::TypeInstId));
   auto index_const_id = context.constant_values().Get(index_inst_id);
   if (index_const_id == SemIR::ErrorInst::ConstantId) {

+ 2 - 2
toolchain/check/modifiers.cpp

@@ -160,7 +160,7 @@ auto CheckMethodModifiersOnFunction(
             "`virtual` not allowed; requires `abstract` or `base` class scope");
         ForbidModifiersOnDecl(context, ModifierVirtualNotAllowed, introducer,
                               KeywordModifierSet::Virtual,
-                              context.insts().GetLocId(parent_scope_inst_id));
+                              SemIR::LocId(parent_scope_inst_id));
       }
       if (inheritance_kind != SemIR::Class::Abstract) {
         CARBON_DIAGNOSTIC(
@@ -168,7 +168,7 @@ auto CheckMethodModifiersOnFunction(
             "`abstract` not allowed; requires `abstract` class scope");
         ForbidModifiersOnDecl(context, ModifierAbstractNotAllowed, introducer,
                               KeywordModifierSet::Abstract,
-                              context.insts().GetLocId(parent_scope_inst_id));
+                              SemIR::LocId(parent_scope_inst_id));
       }
       return;
     }

+ 7 - 6
toolchain/check/name_lookup.cpp

@@ -24,7 +24,8 @@ auto AddNameToLookup(Context& context, SemIR::NameId name_id,
       existing.has_value()) {
     // TODO: Add coverage to this use case and use the location of the name
     // instead of the target.
-    DiagnoseDuplicateName(context, name_id, target_id, existing);
+    DiagnoseDuplicateName(context, name_id, SemIR::LocId(target_id),
+                          SemIR::LocId(existing));
   }
 }
 
@@ -85,7 +86,7 @@ auto LookupNameInDecl(Context& context, SemIR::LocId loc_id,
   }
 }
 
-auto LookupUnqualifiedName(Context& context, Parse::NodeId node_id,
+auto LookupUnqualifiedName(Context& context, SemIR::LocId loc_id,
                            SemIR::NameId name_id, bool required)
     -> LookupResult {
   // TODO: Check for shadowed lookup results.
@@ -99,7 +100,7 @@ auto LookupUnqualifiedName(Context& context, Parse::NodeId node_id,
   for (auto [index, lookup_scope_id, specific_id] :
        llvm::reverse(non_lexical_scopes)) {
     if (auto non_lexical_result =
-            LookupQualifiedName(context, node_id, name_id,
+            LookupQualifiedName(context, loc_id, name_id,
                                 LookupScope{.name_scope_id = lookup_scope_id,
                                             .specific_id = specific_id},
                                 /*required=*/false);
@@ -118,7 +119,7 @@ auto LookupUnqualifiedName(Context& context, Parse::NodeId node_id,
           const auto& interface =
               context.interfaces().Get(interface_decl.interface_id);
           SemIR::InstId result_inst_id = GetAssociatedValue(
-              context, node_id, interface.self_param_id, target_inst_id,
+              context, loc_id, interface.self_param_id, target_inst_id,
               assoc_type->GetSpecificInterface());
           non_lexical_result.scope_result = SemIR::ScopeLookupResult::MakeFound(
               result_inst_id, non_lexical_result.scope_result.access_kind());
@@ -131,7 +132,7 @@ auto LookupUnqualifiedName(Context& context, Parse::NodeId node_id,
   if (lexical_result == SemIR::InstId::InitTombstone) {
     CARBON_DIAGNOSTIC(UsedBeforeInitialization, Error,
                       "`{0}` used before initialization", SemIR::NameId);
-    context.emitter().Emit(node_id, UsedBeforeInitialization, name_id);
+    context.emitter().Emit(loc_id, UsedBeforeInitialization, name_id);
     return {.specific_id = SemIR::SpecificId::None,
             .scope_result = SemIR::ScopeLookupResult::MakeError()};
   }
@@ -147,7 +148,7 @@ auto LookupUnqualifiedName(Context& context, Parse::NodeId node_id,
 
   // We didn't find anything at all.
   if (required) {
-    DiagnoseNameNotFound(context, node_id, name_id);
+    DiagnoseNameNotFound(context, loc_id, name_id);
   }
 
   return {.specific_id = SemIR::SpecificId::None,

+ 1 - 1
toolchain/check/name_lookup.h

@@ -56,7 +56,7 @@ auto LookupNameInDecl(Context& context, SemIR::LocId loc_id,
                       ScopeIndex scope_index) -> SemIR::ScopeLookupResult;
 
 // Performs an unqualified name lookup, returning the referenced `InstId`.
-auto LookupUnqualifiedName(Context& context, Parse::NodeId node_id,
+auto LookupUnqualifiedName(Context& context, SemIR::LocId loc_id,
                            SemIR::NameId name_id, bool required = true)
     -> LookupResult;
 

+ 10 - 6
toolchain/check/operator.cpp

@@ -17,17 +17,21 @@ namespace Carbon::Check {
 // Returns the `Op` function for the specified operator.
 static auto GetOperatorOpFunction(Context& context, SemIR::LocId loc_id,
                                   Operator op) -> SemIR::InstId {
+  auto implicit_loc_id = context.insts().GetCanonicalLocId(loc_id).ToImplicit();
+
   // Look up the interface, and pass it any generic arguments.
-  auto interface_id = LookupNameInCore(context, loc_id, op.interface_name);
+  auto interface_id =
+      LookupNameInCore(context, implicit_loc_id, op.interface_name);
   if (!op.interface_args_ref.empty()) {
-    interface_id =
-        PerformCall(context, loc_id, interface_id, op.interface_args_ref);
+    interface_id = PerformCall(context, implicit_loc_id, interface_id,
+                               op.interface_args_ref);
   }
 
   // Look up the interface member.
   auto op_name_id =
       SemIR::NameId::ForIdentifier(context.identifiers().Add(op.op_name));
-  return PerformMemberAccess(context, loc_id, interface_id, op_name_id);
+  return PerformMemberAccess(context, implicit_loc_id, interface_id,
+                             op_name_id);
 }
 
 auto BuildUnaryOperator(Context& context, SemIR::LocId loc_id, Operator op,
@@ -35,7 +39,7 @@ auto BuildUnaryOperator(Context& context, SemIR::LocId loc_id, Operator op,
                         MakeDiagnosticBuilderFn missing_impl_diagnoser)
     -> SemIR::InstId {
   // Look up the operator function.
-  auto op_fn = GetOperatorOpFunction(context, loc_id.ToImplicit(), op);
+  auto op_fn = GetOperatorOpFunction(context, loc_id, op);
 
   // Form `operand.(Op)`.
   auto bound_op_id = PerformCompoundMemberAccess(context, loc_id, operand_id,
@@ -53,7 +57,7 @@ auto BuildBinaryOperator(Context& context, SemIR::LocId loc_id, Operator op,
                          MakeDiagnosticBuilderFn missing_impl_diagnoser)
     -> SemIR::InstId {
   // Look up the operator function.
-  auto op_fn = GetOperatorOpFunction(context, loc_id.ToImplicit(), op);
+  auto op_fn = GetOperatorOpFunction(context, loc_id, op);
 
   // Form `lhs.(Op)`.
   auto bound_op_id = PerformCompoundMemberAccess(context, loc_id, lhs_id, op_fn,

+ 20 - 23
toolchain/check/pattern_match.cpp

@@ -88,8 +88,7 @@ class MatchContext {
 
   // Implementations of `EmitPatternMatch` for particular pattern inst kinds.
   // The pattern argument is always equal to
-  // `context.insts().Get(entry.pattern_id)`, and `pattern_loc_id` is always
-  // equal to `context.insts().GetLocId(entry.pattern_id)`.
+  // `context.insts().Get(entry.pattern_id)`.
   auto DoEmitPatternMatch(Context& context,
                           SemIR::AnyBindingPattern binding_pattern,
                           SemIR::InstId pattern_inst_id, WorkItem entry)
@@ -161,7 +160,6 @@ auto MatchContext::DoWork(Context& context) -> SemIR::InstBlockId {
 static auto InsertHere(Context& context, SemIR::ExprRegionId region_id)
     -> SemIR::InstId {
   auto region = context.sem_ir().expr_regions().Get(region_id);
-  auto loc_id = context.insts().GetLocId(region.result_id);
   auto exit_block = context.inst_blocks().Get(region.block_ids.back());
   if (region.block_ids.size() == 1) {
     // TODO: Is it possible to avoid leaving an "orphan" block in the IR in the
@@ -174,13 +172,13 @@ static auto InsertHere(Context& context, SemIR::ExprRegionId region_id)
       return region.result_id;
     }
     return AddInst<SemIR::SpliceBlock>(
-        context, loc_id,
+        context, SemIR::LocId(region.result_id),
         {.type_id = context.insts().Get(region.result_id).type_id(),
          .block_id = region.block_ids.front(),
          .result_id = region.result_id});
   }
   if (context.region_stack().empty()) {
-    context.TODO(loc_id,
+    context.TODO(region.result_id,
                  "Control flow expressions are currently only supported inside "
                  "functions.");
     return SemIR::ErrorInst::InstId;
@@ -195,7 +193,8 @@ static auto InsertHere(Context& context, SemIR::ExprRegionId region_id)
       context.insts().GetAs<SemIR::Branch>(exit_block.back()).target_id;
   CARBON_CHECK(context.inst_blocks().GetOrEmpty(resume_with_block_id).empty());
   context.inst_block_stack().Push(resume_with_block_id);
-  context.region_stack().AddToRegion(resume_with_block_id, loc_id);
+  context.region_stack().AddToRegion(resume_with_block_id,
+                                     SemIR::LocId(region.result_id));
   return region.result_id;
 }
 
@@ -221,8 +220,7 @@ auto MatchContext::DoEmitPatternMatch(Context& context,
   auto value_id = SemIR::InstId::None;
   if (kind_ == MatchKind::Local) {
     value_id =
-        Convert(context, context.insts().GetLocId(entry.scrutinee_id),
-                entry.scrutinee_id,
+        Convert(context, SemIR::LocId(entry.scrutinee_id), entry.scrutinee_id,
                 {.kind = bind_name_id.has_value() ? ConversionTarget::ValueOrRef
                                                   : ConversionTarget::Discarded,
                  .type_id = context.insts().Get(bind_name_id).type_id()});
@@ -264,7 +262,7 @@ auto MatchContext::DoEmitPatternMatch(Context& context,
       CARBON_DIAGNOSTIC(AddrSelfIsNonRef, Error,
                         "`addr self` method cannot be invoked on a value");
       context.emitter().Emit(
-          TokenOnly(context.insts().GetLocId(entry.scrutinee_id)),
+          context.insts().GetCanonicalLocId(entry.scrutinee_id).ToTokenOnly(),
           AddrSelfIsNonRef);
       // Add fake reference expression to preserve invariants.
       auto scrutinee = context.insts().GetWithLocId(entry.scrutinee_id);
@@ -275,7 +273,7 @@ auto MatchContext::DoEmitPatternMatch(Context& context,
   auto scrutinee_ref_type_inst_id =
       context.types().GetInstId(scrutinee_ref.type_id());
   auto new_scrutinee = AddInst<SemIR::AddrOf>(
-      context, context.insts().GetLocId(scrutinee_ref_id),
+      context, SemIR::LocId(scrutinee_ref_id),
       {.type_id = GetPointerType(context, scrutinee_ref_type_inst_id),
        .lvalue_id = scrutinee_ref_id});
   AddWork({.pattern_id = addr_pattern.inner_id, .scrutinee_id = new_scrutinee});
@@ -296,8 +294,7 @@ auto MatchContext::DoEmitPatternMatch(Context& context,
         results_.push_back(SemIR::ErrorInst::InstId);
       } else {
         results_.push_back(ConvertToValueOfType(
-            context, context.insts().GetLocId(entry.scrutinee_id),
-            entry.scrutinee_id,
+            context, SemIR::LocId(entry.scrutinee_id), entry.scrutinee_id,
             ExtractScrutineeType(
                 context.sem_ir(),
                 SemIR::GetTypeOfInstInSpecific(
@@ -312,7 +309,7 @@ auto MatchContext::DoEmitPatternMatch(Context& context,
       param_pattern.index = NextRuntimeIndex();
       ReplaceInstBeforeConstantUse(context, entry.pattern_id, param_pattern);
       auto param_id = AddInst<SemIR::ValueParam>(
-          context, context.insts().GetLocId(pattern_inst_id),
+          context, SemIR::LocId(pattern_inst_id),
           {.type_id =
                ExtractScrutineeType(context.sem_ir(), param_pattern.type_id),
            .index = param_pattern.index,
@@ -354,7 +351,7 @@ auto MatchContext::DoEmitPatternMatch(Context& context,
       param_pattern.index = NextRuntimeIndex();
       ReplaceInstBeforeConstantUse(context, entry.pattern_id, param_pattern);
       auto param_id = AddInst<SemIR::RefParam>(
-          context, context.insts().GetLocId(pattern_inst_id),
+          context, SemIR::LocId(pattern_inst_id),
           {.type_id =
                ExtractScrutineeType(context.sem_ir(), param_pattern.type_id),
            .index = param_pattern.index,
@@ -400,7 +397,7 @@ auto MatchContext::DoEmitPatternMatch(Context& context,
       param_pattern.index = NextRuntimeIndex();
       ReplaceInstBeforeConstantUse(context, entry.pattern_id, param_pattern);
       auto param_id = AddInst<SemIR::OutParam>(
-          context, context.insts().GetLocId(pattern_inst_id),
+          context, SemIR::LocId(pattern_inst_id),
           {.type_id =
                ExtractScrutineeType(context.sem_ir(), param_pattern.type_id),
            .index = param_pattern.index,
@@ -424,7 +421,7 @@ auto MatchContext::DoEmitPatternMatch(
   auto type_id =
       ExtractScrutineeType(context.sem_ir(), return_slot_pattern.type_id);
   auto return_slot_id = AddInst<SemIR::ReturnSlot>(
-      context, context.insts().GetLocId(pattern_inst_id),
+      context, SemIR::LocId(pattern_inst_id),
       {.type_id = type_id,
        .type_inst_id = context.types().GetInstId(type_id),
        .storage_id = entry.scrutinee_id});
@@ -459,7 +456,7 @@ auto MatchContext::DoEmitPatternMatch(Context& context,
     }
     case MatchKind::Caller: {
       storage_id = AddInstWithCleanup<SemIR::TemporaryStorage>(
-          context, context.insts().GetLocId(pattern_inst_id),
+          context, SemIR::LocId(pattern_inst_id),
           {.type_id =
                ExtractScrutineeType(context.sem_ir(), var_pattern.type_id)});
       CARBON_CHECK(entry.scrutinee_id.has_value());
@@ -473,11 +470,11 @@ auto MatchContext::DoEmitPatternMatch(Context& context,
     context.global_init().Resume();
   }
   if (entry.scrutinee_id.has_value()) {
-    auto loc_id = context.insts().GetLocId(pattern_inst_id);
-    auto init_id = Initialize(context, loc_id, storage_id, entry.scrutinee_id);
+    auto init_id = Initialize(context, SemIR::LocId(pattern_inst_id),
+                              storage_id, entry.scrutinee_id);
     // TODO: Consider using different instruction kinds for assignment
     // versus initialization.
-    AddInst<SemIR::Assign>(context, loc_id,
+    AddInst<SemIR::Assign>(context, SemIR::LocId(pattern_inst_id),
                            {.lhs_id = storage_id, .rhs_id = init_id});
   }
   AddWork(
@@ -529,9 +526,9 @@ auto MatchContext::DoEmitPatternMatch(Context& context,
 
   auto tuple_type_id =
       ExtractScrutineeType(context.sem_ir(), tuple_pattern.type_id);
-  auto converted_scrutinee = ConvertToValueOrRefOfType(
-      context, context.insts().GetLocId(pattern_inst_id), entry.scrutinee_id,
-      tuple_type_id);
+  auto converted_scrutinee =
+      ConvertToValueOrRefOfType(context, SemIR::LocId(pattern_inst_id),
+                                entry.scrutinee_id, tuple_type_id);
   if (auto scrutinee_value =
           context.insts().TryGetAs<SemIR::TupleValue>(converted_scrutinee)) {
     add_all_subscrutinees(

+ 2 - 0
toolchain/check/pending_block.h

@@ -44,6 +44,7 @@ class PendingBlock {
   };
 
   template <typename InstT, typename LocT>
+    requires(std::convertible_to<LocT, SemIR::LocId>)
   auto AddInst(LocT loc_id, InstT inst) -> SemIR::InstId {
     auto inst_id = AddInstInNoBlock(*context_, loc_id, inst);
     insts_.push_back(inst_id);
@@ -51,6 +52,7 @@ class PendingBlock {
   }
 
   template <typename InstT, typename LocT>
+    requires(std::convertible_to<LocT, SemIR::LocId>)
   auto AddInstWithCleanup(LocT loc_id, InstT inst) -> SemIR::InstId {
     auto inst_id = AddInstWithCleanupInNoBlock(*context_, loc_id, inst);
     insts_.push_back(inst_id);

+ 2 - 1
toolchain/check/subst.cpp

@@ -386,7 +386,8 @@ class SubstConstantCallbacks final : public SubstInstCallbacks {
     auto const_id = EvalOrAddInst(
         *context_,
         SemIR::LocIdAndInst::UncheckedLoc(
-            context_->insts().GetLocId(old_inst_id).ToImplicit(), new_inst));
+            context_->insts().GetCanonicalLocId(old_inst_id).ToImplicit(),
+            new_inst));
     CARBON_CHECK(const_id.has_value(),
                  "Substitution into constant produced non-constant");
     return context_->constant_values().GetInstId(const_id);

+ 2 - 2
toolchain/diagnostics/diagnostic_emitter.h

@@ -277,7 +277,7 @@ auto Emitter<LocT>::Builder::Note(
   CARBON_CHECK(diagnostic_base.Level == Level::Note ||
                    diagnostic_base.Level == Level::LocationInfo,
                "{0}", static_cast<int>(diagnostic_base.Level));
-  AddMessage(loc, diagnostic_base, {emitter_->MakeAny<Args>(args)...});
+  AddMessage(LocT(loc), diagnostic_base, {emitter_->MakeAny<Args>(args)...});
   return *this;
 }
 
@@ -315,7 +315,7 @@ Emitter<LocT>::Builder::Builder(Emitter<LocT>* emitter, LocT loc,
                                 const DiagnosticBase<Args...>& diagnostic_base,
                                 llvm::SmallVector<llvm::Any> args)
     : emitter_(emitter), diagnostic_({.level = diagnostic_base.Level}) {
-  AddMessage(loc, diagnostic_base, std::move(args));
+  AddMessage(LocT(loc), diagnostic_base, std::move(args));
   CARBON_CHECK(diagnostic_base.Level != Level::Note);
 }
 

+ 3 - 2
toolchain/lower/file_context.cpp

@@ -712,7 +712,8 @@ auto FileContext::BuildGlobalVariableDecl(SemIR::VarStorage var_storage)
 }
 
 auto FileContext::GetLocForDI(SemIR::InstId inst_id) -> LocForDI {
-  SemIR::AbsoluteNodeId resolved = GetAbsoluteNodeId(sem_ir_, inst_id).back();
+  SemIR::AbsoluteNodeId resolved =
+      GetAbsoluteNodeId(sem_ir_, SemIR::LocId(inst_id)).back();
   const auto& tree_and_subtrees =
       (*tree_and_subtrees_getters_for_debug_info_)[resolved.check_ir_id
                                                        .index]();
@@ -748,7 +749,7 @@ auto FileContext::BuildVtable(const SemIR::Class& class_info)
   std::string mangled_name = m.MangleVTable(class_info);
 
   auto first_owning_decl_loc =
-      sem_ir().insts().GetLocId(class_info.first_owning_decl_id);
+      sem_ir().insts().GetCanonicalLocId(class_info.first_owning_decl_id);
   if (first_owning_decl_loc.kind() == SemIR::LocId::Kind::ImportIRInstId) {
     // Emit a declaration of an imported vtable using a(n opaque) pointer type.
     // This doesn't have to match the definition that appears elsewhere, it'll

+ 5 - 4
toolchain/sem_ir/absolute_node_id.cpp

@@ -21,7 +21,7 @@ static auto FollowImportRef(
                "If we get `None` locations here, we may need to more "
                "thoroughly track ImportDecls.");
 
-  auto import_loc_id = cursor_ir->insts().GetLocId(import_ir.decl_id);
+  auto import_loc_id = cursor_ir->insts().GetCanonicalLocId(import_ir.decl_id);
   switch (import_loc_id.kind()) {
     case LocId::Kind::None:
       break;
@@ -33,7 +33,7 @@ static auto FollowImportRef(
           cursor_ir->import_ir_insts().Get(import_loc_id.import_ir_inst_id());
       const auto& implicit_ir =
           cursor_ir->import_irs().Get(implicit_import_ir_inst.ir_id());
-      auto implicit_loc_id = implicit_ir.sem_ir->insts().GetLocId(
+      auto implicit_loc_id = implicit_ir.sem_ir->insts().GetCanonicalLocId(
           implicit_import_ir_inst.inst_id());
       CARBON_CHECK(implicit_loc_id.kind() == LocId::Kind::NodeId,
                    "Should only be one layer of implicit imports");
@@ -96,7 +96,7 @@ static auto GetAbsoluteNodeIdImpl(
     }
 
     // If the parse node has a value, use it for the location.
-    if (auto loc_id = cursor_ir->insts().GetLocId(cursor_inst_id);
+    if (auto loc_id = cursor_ir->insts().GetCanonicalLocId(cursor_inst_id);
         loc_id.has_value()) {
       if (HandleLocId(absolute_node_ids, cursor_ir, cursor_inst_id, loc_id)) {
         return;
@@ -136,7 +136,8 @@ auto GetAbsoluteNodeId(const File* sem_ir, LocId loc_id)
     case LocId::Kind::NodeId: {
       const File* cursor_ir = sem_ir;
       InstId cursor_inst_id = InstId::None;
-      if (HandleLocId(absolute_node_ids, cursor_ir, cursor_inst_id, loc_id)) {
+      if (HandleLocId(absolute_node_ids, cursor_ir, cursor_inst_id,
+                      cursor_ir->insts().GetCanonicalLocId(loc_id))) {
         break;
       }
       CARBON_CHECK(cursor_inst_id.has_value(), "Should be set by HandleLocId");

+ 3 - 0
toolchain/sem_ir/absolute_node_id.h

@@ -21,6 +21,9 @@ struct AbsoluteNodeId {
 // files. The vector will have one entry if there were no imports, and multiple
 // entries when imports are traversed. The final entry is the actual
 // declaration.
+//
+// Note that the `LocId` here is typically not canonical, and it uses that fact
+// for non-canonical locations built from an `ExportDecl` instruction.
 auto GetAbsoluteNodeId(const File* sem_ir, LocId loc_id)
     -> llvm::SmallVector<AbsoluteNodeId>;
 

+ 2 - 2
toolchain/sem_ir/formatter.cpp

@@ -563,7 +563,7 @@ class FormatterImpl {
     // If this entity was imported from a different IR, annotate the name of
     // that IR in the output before the `{` or `;`.
     if (first_owning_decl_id.has_value()) {
-      auto loc_id = sem_ir_->insts().GetLocId(first_owning_decl_id);
+      auto loc_id = sem_ir_->insts().GetCanonicalLocId(first_owning_decl_id);
       if (loc_id.kind() == LocId::Kind::ImportIRInstId) {
         auto import_ir_id =
             sem_ir_->import_ir_insts().Get(loc_id.import_ir_inst_id()).ir_id();
@@ -1159,7 +1159,7 @@ class FormatterImpl {
       // instruction as a last resort.
       const auto& import_ir = sem_ir_->import_irs().Get(import_ir_inst.ir_id());
       auto loc_id =
-          import_ir.sem_ir->insts().GetLocId(import_ir_inst.inst_id());
+          import_ir.sem_ir->insts().GetCanonicalLocId(import_ir_inst.inst_id());
       switch (loc_id.kind()) {
         case LocId::Kind::None: {
           out_ << import_ir_inst.inst_id() << " [no loc]";

+ 26 - 15
toolchain/sem_ir/ids.h

@@ -845,6 +845,12 @@ struct ImportIRInstId : public IdBase<ImportIRInstId> {
 //
 // In addition, two bits are used for flags: `ImplicitBit` and `TokenOnlyBit`.
 // Note that these can only be used with negative, non-`InstId` values.
+//
+// Use `InstStore::GetCanonicalLocId()` to get a canonical `LocId` which will
+// not be backed by an `InstId`. Note that the canonical `LocId` may be `None`
+// even when the original `LocId` was not, so this operation needs to be done
+// before checking `has_value()`. Only canonical locations can be converted with
+// `ToImplicit()` or `ToTokenOnly()`.
 struct LocId : public IdBase<LocId> {
   // The contained index kind.
   enum class Kind {
@@ -864,8 +870,7 @@ struct LocId : public IdBase<LocId> {
                    ? FirstImportIRInstId - import_ir_inst_id.index
                    : NoneIndex) {}
 
-  // NOLINTNEXTLINE(google-explicit-constructor)
-  constexpr LocId(InstId inst_id) : IdBase(inst_id.index) {}
+  explicit constexpr LocId(InstId inst_id) : IdBase(inst_id.index) {}
 
   // NOLINTNEXTLINE(google-explicit-constructor)
   constexpr LocId(Parse::NoneNodeId /*none*/) : IdBase(NoneIndex) {}
@@ -874,12 +879,13 @@ struct LocId : public IdBase<LocId> {
   constexpr LocId(Parse::NodeId node_id)
       : IdBase(FirstNodeId - node_id.index) {}
 
-  // Forms an equivalent LocId for a desugared location.  Requires a
-  // non-`InstId` location.
+  // Forms an equivalent LocId for a desugared location. Requires a
+  // canonical location. See `InstStore::GetCanonicalLocId()`.
+  //
   // TODO: Rename to something like `ToDesugared`.
   auto ToImplicit() const -> LocId {
-    // This should only be called for NodeId or ImportIRInstId, but we only set
-    // the flag for NodeId.
+    // This should only be called for NodeId or ImportIRInstId (i.e. canonical
+    // locations), but we only set the flag for NodeId.
     CARBON_CHECK(kind() != Kind::InstId);
     if (kind() == Kind::NodeId) {
       return LocId(index & ~ImplicitBit);
@@ -887,8 +893,12 @@ struct LocId : public IdBase<LocId> {
     return *this;
   }
 
-  // Forms an equivalent `LocId` for a token-only diagnostic location.  Requires
-  // a non-`InstId` location.
+  // Forms an equivalent `LocId` for a token-only diagnostic location. Requires
+  // a canonical location. See `InstStore::GetCanonicalLocId()`.
+  //
+  // TODO: Consider making this a part of check/ diagnostics instead, as a free
+  // function operation on `LocIdForDiagnostics`?
+  // https://github.com/carbon-language/carbon-lang/pull/5355#discussion_r2064113186
   auto ToTokenOnly() const -> LocId {
     CARBON_CHECK(kind() != Kind::InstId);
     if (has_value()) {
@@ -914,15 +924,16 @@ struct LocId : public IdBase<LocId> {
   // Returns true if the location corresponds to desugared instructions.
   // Requires a non-`InstId` location.
   auto is_implicit() const -> bool {
-    CARBON_CHECK(kind() != Kind::InstId);
     return (kind() == Kind::NodeId) && (index & ImplicitBit) == 0;
   }
 
-  // Returns true if the location is token-only for diagnostics. Requires a
-  // non-`InstId` location.
+  // Returns true if the location is token-only for diagnostics.
+  //
+  // This means the displayed location will include only the location's specific
+  // parse node, instead of also including its descendants. As such, this can
+  // only be true for locations backed by a `NodeId`.
   auto is_token_only() const -> bool {
-    CARBON_CHECK(kind() != Kind::InstId);
-    return (index & TokenOnlyBit) == 0;
+    return kind() != Kind::InstId && (index & TokenOnlyBit) == 0;
   }
 
   // Returns the equivalent `ImportIRInstId` when `kind()` matches or is `None`.
@@ -956,8 +967,8 @@ struct LocId : public IdBase<LocId> {
   // for `NodeId`.
   static constexpr int32_t ImplicitBit = 1 << 30;
 
-  // See `token_only` for the use. This only applies for `NodeId` and
-  // `ImportIRInstId`.
+  // See `is_token_only` for the use. This only applies for canonical locations
+  // (i.e. those containing `NodeId` or `ImportIRInstId`).
   static constexpr int32_t TokenOnlyBit = 1 << 29;
 
   // The value of the 0 index for each of `NodeId` and `ImportIRInstId`.

+ 1 - 1
toolchain/sem_ir/ids_test.cpp

@@ -49,7 +49,7 @@ class IdsTestWithParam
   auto BuildIdAndLocId() -> std::pair<IdT, LocId> {
     auto [implicit, token_only, index] = GetParam();
     IdT id(index);
-    LocId loc_id = id;
+    LocId loc_id(id);
     if (implicit) {
       loc_id = loc_id.ToImplicit();
     }

+ 1 - 1
toolchain/sem_ir/import_ir.cpp

@@ -28,7 +28,7 @@ auto GetCanonicalFileAndInstId(const File* sem_ir, SemIR::InstId inst_id)
   while (true) {
     // Step through an instruction with an imported location to the imported
     // instruction.
-    if (auto loc_id = sem_ir->insts().GetLocId(inst_id);
+    if (auto loc_id = sem_ir->insts().GetCanonicalLocId(inst_id);
         loc_id.kind() == SemIR::LocId::Kind::ImportIRInstId) {
       auto import_ir_inst =
           sem_ir->import_ir_insts().Get(loc_id.import_ir_inst_id());

+ 23 - 3
toolchain/sem_ir/inst.h

@@ -398,7 +398,7 @@ class InstStore {
 
   // Returns the requested instruction and its location ID.
   auto GetWithLocId(InstId inst_id) const -> LocIdAndInst {
-    return LocIdAndInst::UncheckedLoc(GetLocId(inst_id), Get(inst_id));
+    return LocIdAndInst::UncheckedLoc(LocId(inst_id), Get(inst_id));
   }
 
   // Returns whether the requested instruction is the specified type.
@@ -431,11 +431,31 @@ class InstStore {
     return TryGetAs<InstT>(inst_id);
   }
 
-  auto GetLocId(InstId inst_id) const -> LocId {
+  // Returns a resolved LocId, which will point to a parse node, an import, or
+  // be None.
+  //
+  // Unresolved LocIds can be backed by an InstId which may or may not have a
+  // value after being resolved, so this operation needs to be done before using
+  // most operations on LocId.
+  auto GetCanonicalLocId(LocId loc_id) const -> LocId {
+    while (loc_id.kind() == LocId::Kind::InstId) {
+      auto inst_id = loc_id.inst_id();
+      CARBON_CHECK(inst_id.index >= 0, "{0}", inst_id.index);
+      CARBON_CHECK(static_cast<size_t>(inst_id.index) < loc_ids_.size(),
+                   "{0} {1}", inst_id.index, loc_ids_.size());
+      loc_id = loc_ids_[inst_id.index];
+    }
+    return loc_id;
+  }
+
+  // Gets the resolved LocId for an instruction. InstId can directly construct
+  // an unresolved LocId. This skips that step when a resolved LocId is needed.
+  auto GetCanonicalLocId(InstId inst_id) const -> LocId {
     CARBON_CHECK(inst_id.index >= 0, "{0}", inst_id.index);
     CARBON_CHECK(inst_id.index < (int)loc_ids_.size(), "{0} {1}", inst_id.index,
                  loc_ids_.size());
-    return loc_ids_[inst_id.index];
+    auto loc_id = loc_ids_[inst_id.index];
+    return GetCanonicalLocId(loc_id);
   }
 
   // Overwrites a given instruction with a new value.

+ 7 - 6
toolchain/sem_ir/inst_namer.cpp

@@ -101,9 +101,8 @@ InstNamer::InstNamer(const File* sem_ir) : sem_ir_(sem_ir) {
   for (auto [assoc_const_id, assoc_const_info] :
        sem_ir->associated_constants().enumerate()) {
     auto assoc_const_scope = GetScopeFor(assoc_const_id);
-    auto assoc_const_loc = sem_ir->insts().GetLocId(assoc_const_info.decl_id);
     GetScopeInfo(assoc_const_scope).name = globals_.AllocateName(
-        *this, assoc_const_loc,
+        *this, LocId(assoc_const_info.decl_id),
         sem_ir->names().GetIRBaseName(assoc_const_info.name_id).str());
     CollectNamesInGeneric(assoc_const_scope, assoc_const_info.generic_id);
   }
@@ -170,7 +169,7 @@ auto InstNamer::GetNameFor(ScopeId scope_id, InstId inst_id) const
     // This should not happen in valid IR.
     RawStringOstream out;
     out << "<unexpected>." << inst_id;
-    auto loc_id = sem_ir_->insts().GetLocId(inst_id);
+    auto loc_id = sem_ir_->insts().GetCanonicalLocId(inst_id);
     // TODO: Consider handling other kinds.
     if (loc_id.kind() == LocId::Kind::NodeId) {
       const auto& tree = sem_ir_->parse_tree();
@@ -263,6 +262,7 @@ auto InstNamer::Namespace::AllocateName(
 
   // Append location information to try to disambiguate.
   if (auto* loc_id = std::get_if<LocId>(&loc_id_or_fingerprint)) {
+    *loc_id = inst_namer.sem_ir_->insts().GetCanonicalLocId(*loc_id);
     // TODO: Consider handling other kinds.
     if (loc_id->kind() == LocId::Kind::NodeId) {
       const auto& tree = inst_namer.sem_ir_->parse_tree();
@@ -312,7 +312,7 @@ auto InstNamer::AddBlockLabel(
       loc_id && !loc_id->has_value()) {
     if (const auto& block = sem_ir_->inst_blocks().Get(block_id);
         !block.empty()) {
-      loc_id_or_fingerprint = sem_ir_->insts().GetLocId(block.front());
+      loc_id_or_fingerprint = LocId(block.front());
     }
   }
 
@@ -325,6 +325,7 @@ auto InstNamer::AddBlockLabel(
 // represents some kind of branch.
 auto InstNamer::AddBlockLabel(ScopeId scope_id, LocId loc_id, AnyBranch branch)
     -> void {
+  loc_id = sem_ir_->insts().GetCanonicalLocId(loc_id);
   if (!loc_id.node_id().has_value()) {
     AddBlockLabel(scope_id, branch.target_id, "", loc_id);
     return;
@@ -441,7 +442,7 @@ auto InstNamer::CollectNamesInBlock(ScopeId top_scope_id,
         if (scope_id == ScopeId::Constants || scope_id == ScopeId::ImportRefs) {
           loc_id_or_fingerprint = fingerprinter_.GetOrCompute(sem_ir_, inst_id);
         } else {
-          loc_id_or_fingerprint = sem_ir_->insts().GetLocId(inst_id);
+          loc_id_or_fingerprint = LocId(inst_id);
         }
         insts_[inst_id.index] = {
             scope_id,
@@ -497,7 +498,7 @@ auto InstNamer::CollectNamesInBlock(ScopeId top_scope_id,
     };
 
     if (auto branch = untyped_inst.TryAs<AnyBranch>()) {
-      AddBlockLabel(scope_id, sem_ir_->insts().GetLocId(inst_id), *branch);
+      AddBlockLabel(scope_id, LocId(inst_id), *branch);
     }
 
     CARBON_KIND_SWITCH(untyped_inst) {