Explorar el Código

Split parse nodes out from instructions because they're rarely used. (#3590)

The parse nodes are still tracked as part of the same value store
interface in order to ensure parity, but they're split out from Inst
itself in order to reduce the size of Inst -- the expectation is that
they don't need to be passed around quite as much.

This change doesn't actually reduce the passing very much, although
there are hints of it: AddInstAndPush doesn't typically need a separate
parse node from the one on the Inst itself, for example. In a couple
spots I changed code to rely a little more on the InstId until the
ParseNode is needed, but it's very low hanging fruit where done. I think
convert could do more to not eagerly fetch the parse node before its
use, but more cleanup felt it would be easier to handle separately. I'm
currently viewing this as making such cleanup _possible_ rather than
executing on it up-front.

But also, I want to make sure there's a consensus to head in this
direction before pulling the trigger. We speculated that this would
result in the parse node being passed around less, and I do think that's
the case, although it's a bit fuzzy in the change.

---------

Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Jon Ross-Perkins hace 2 años
padre
commit
f197219c10
Se han modificado 37 ficheros con 674 adiciones y 627 borrados
  1. 2 0
      toolchain/check/BUILD
  2. 5 3
      toolchain/check/check.cpp
  3. 77 70
      toolchain/check/context.cpp
  4. 15 4
      toolchain/check/context.h
  5. 82 67
      toolchain/check/convert.cpp
  6. 20 10
      toolchain/check/eval.cpp
  7. 3 4
      toolchain/check/handle_array.cpp
  8. 19 16
      toolchain/check/handle_binding_pattern.cpp
  9. 8 5
      toolchain/check/handle_call_expr.cpp
  10. 22 20
      toolchain/check/handle_class.cpp
  11. 1 1
      toolchain/check/handle_expr_statement.cpp
  12. 9 10
      toolchain/check/handle_function.cpp
  13. 1 1
      toolchain/check/handle_if_statement.cpp
  14. 8 7
      toolchain/check/handle_index.cpp
  15. 4 4
      toolchain/check/handle_interface.cpp
  16. 2 1
      toolchain/check/handle_let.cpp
  17. 18 23
      toolchain/check/handle_literal.cpp
  18. 3 3
      toolchain/check/handle_loop_statement.cpp
  19. 20 22
      toolchain/check/handle_name.cpp
  20. 2 2
      toolchain/check/handle_namespace.cpp
  21. 20 24
      toolchain/check/handle_operator.cpp
  22. 1 1
      toolchain/check/handle_paren.cpp
  23. 13 12
      toolchain/check/handle_struct.cpp
  24. 3 4
      toolchain/check/handle_variable.cpp
  25. 5 4
      toolchain/check/import.cpp
  26. 3 2
      toolchain/check/inst_block_stack.h
  27. 7 4
      toolchain/check/pending_block.h
  28. 9 9
      toolchain/check/return.cpp
  29. 1 0
      toolchain/sem_ir/BUILD
  30. 9 6
      toolchain/sem_ir/file.cpp
  31. 13 11
      toolchain/sem_ir/formatter.cpp
  32. 18 35
      toolchain/sem_ir/inst.h
  33. 8 8
      toolchain/sem_ir/inst_kind.cpp
  34. 17 14
      toolchain/sem_ir/inst_kind.h
  35. 154 180
      toolchain/sem_ir/typed_insts.h
  36. 18 29
      toolchain/sem_ir/typed_insts_test.cpp
  37. 54 11
      toolchain/sem_ir/value_stores.h

+ 2 - 0
toolchain/check/BUILD

@@ -65,6 +65,7 @@ cc_library(
         "//toolchain/sem_ir:ids",
         "//toolchain/sem_ir:inst",
         "//toolchain/sem_ir:inst_kind",
+        "//toolchain/sem_ir:value_stores",
         "@llvm-project//llvm:Support",
     ],
 )
@@ -95,6 +96,7 @@ cc_library(
         "//toolchain/sem_ir:ids",
         "//toolchain/sem_ir:inst",
         "//toolchain/sem_ir:inst_kind",
+        "//toolchain/sem_ir:value_stores",
         "@llvm-project//llvm:Support",
     ],
 )

+ 5 - 3
toolchain/check/check.cpp

@@ -57,6 +57,7 @@ struct UnitInfo {
   Unit* unit;
 
   // Emitter information.
+  // TODO: Augment the translator to translate InstIds for locations.
   Parse::NodeLocationTranslator translator;
   ErrorTrackingDiagnosticConsumer err_tracker;
   DiagnosticEmitter<Parse::NodeLocation> emitter;
@@ -89,9 +90,10 @@ static auto InitPackageScopeAndImports(Context& context, UnitInfo& unit_info)
       SemIR::InstId::PackageNamespace, SemIR::NameScopeId::Invalid);
   CARBON_CHECK(package_scope_id == SemIR::NameScopeId::Package);
 
-  auto package_inst_id = context.AddInst(SemIR::Namespace{
-      Parse::NodeId::Invalid, namespace_type_id,
-      SemIR::NameId::PackageNamespace, SemIR::NameScopeId::Package});
+  auto package_inst_id = context.AddInst(
+      {Parse::NodeId::Invalid,
+       SemIR::Namespace{namespace_type_id, SemIR::NameId::PackageNamespace,
+                        SemIR::NameScopeId::Package}});
   CARBON_CHECK(package_inst_id == SemIR::InstId::PackageNamespace);
 
   // Add imports from the current package.

+ 77 - 70
toolchain/check/context.cpp

@@ -20,6 +20,7 @@
 #include "toolchain/sem_ir/inst.h"
 #include "toolchain/sem_ir/inst_kind.h"
 #include "toolchain/sem_ir/typed_insts.h"
+#include "toolchain/sem_ir/value_stores.h"
 
 namespace Carbon::Check {
 
@@ -61,38 +62,39 @@ auto Context::VerifyOnFinish() -> void {
   CARBON_CHECK(params_or_args_stack_.empty()) << params_or_args_stack_.size();
 }
 
-auto Context::AddInst(SemIR::Inst inst) -> SemIR::InstId {
-  auto inst_id = inst_block_stack_.AddInst(inst);
-  CARBON_VLOG() << "AddInst: " << inst << "\n";
+auto Context::AddInst(SemIR::ParseNodeAndInst parse_node_and_inst)
+    -> SemIR::InstId {
+  auto inst_id = inst_block_stack_.AddInst(parse_node_and_inst);
+  CARBON_VLOG() << "AddInst: " << parse_node_and_inst.inst << "\n";
 
-  auto const_id = TryEvalInst(*this, inst_id, inst);
+  auto const_id = TryEvalInst(*this, inst_id, parse_node_and_inst.inst);
   if (const_id.is_constant()) {
-    CARBON_VLOG() << "Constant: " << inst << " -> " << const_id.inst_id()
-                  << "\n";
+    CARBON_VLOG() << "Constant: " << parse_node_and_inst.inst << " -> "
+                  << const_id.inst_id() << "\n";
     constant_values().Set(inst_id, const_id);
   }
 
   return inst_id;
 }
 
-auto Context::AddConstant(SemIR::Inst inst, bool is_symbolic)
-    -> SemIR::ConstantId {
+auto Context::AddConstant(SemIR::ParseNodeAndInst parse_node_and_inst,
+                          bool is_symbolic) -> SemIR::ConstantId {
   // TODO: Deduplicate constants.
-  auto inst_id = insts().AddInNoBlock(inst);
+  auto inst_id = insts().AddInNoBlock(parse_node_and_inst);
   constants().Add(inst_id);
 
   auto const_id = is_symbolic ? SemIR::ConstantId::ForSymbolicConstant(inst_id)
                               : SemIR::ConstantId::ForTemplateConstant(inst_id);
   constant_values().Set(inst_id, const_id);
 
-  CARBON_VLOG() << "AddConstantInst: " << inst << "\n";
+  CARBON_VLOG() << "AddConstantInst: " << parse_node_and_inst.inst << "\n";
   return const_id;
 }
 
-auto Context::AddInstAndPush(Parse::NodeId parse_node, SemIR::Inst inst)
+auto Context::AddInstAndPush(SemIR::ParseNodeAndInst parse_node_and_inst)
     -> void {
-  auto inst_id = AddInst(inst);
-  node_stack_.Push(parse_node, inst_id);
+  auto inst_id = AddInst(parse_node_and_inst);
+  node_stack_.Push(parse_node_and_inst.parse_node, inst_id);
 }
 
 auto Context::DiagnoseDuplicateName(Parse::NodeId parse_node,
@@ -101,9 +103,8 @@ auto Context::DiagnoseDuplicateName(Parse::NodeId parse_node,
                     "Duplicate name being declared in the same scope.");
   CARBON_DIAGNOSTIC(NameDeclPrevious, Note,
                     "Name is previously declared here.");
-  auto prev_def = insts().Get(prev_def_id);
   emitter_->Build(parse_node, NameDeclDuplicate)
-      .Note(prev_def.parse_node(), NameDeclPrevious)
+      .Note(insts().GetParseNode(prev_def_id), NameDeclPrevious)
       .Emit();
 }
 
@@ -122,10 +123,10 @@ auto Context::NoteIncompleteClass(SemIR::ClassId class_id,
   const auto& class_info = classes().Get(class_id);
   CARBON_CHECK(!class_info.is_defined()) << "Class is not incomplete";
   if (class_info.definition_id.is_valid()) {
-    builder.Note(insts().Get(class_info.definition_id).parse_node(),
+    builder.Note(insts().GetParseNode(class_info.definition_id),
                  ClassIncompleteWithinDefinition);
   } else {
-    builder.Note(insts().Get(class_info.decl_id).parse_node(),
+    builder.Note(insts().GetParseNode(class_info.decl_id),
                  ClassForwardDeclaredHere);
   }
 }
@@ -149,10 +150,10 @@ auto Context::AddPackageImports(Parse::NodeId import_node,
   SemIR::CrossRefIRId last_id(cross_ref_irs().size() - 1);
 
   auto type_id = GetBuiltinType(SemIR::BuiltinKind::NamespaceType);
-  auto inst_id = AddInst(SemIR::Import{.parse_node = import_node,
-                                       .type_id = type_id,
-                                       .first_cross_ref_ir_id = first_id,
-                                       .last_cross_ref_ir_id = last_id});
+  auto inst_id =
+      AddInst({import_node, SemIR::Import{.type_id = type_id,
+                                          .first_cross_ref_ir_id = first_id,
+                                          .last_cross_ref_ir_id = last_id}});
 
   // Add the import to lookup. Should always succeed because imports will be
   // uniquely named.
@@ -162,10 +163,9 @@ auto Context::AddPackageImports(Parse::NodeId import_node,
   // otherwise fits in an Inst.
   auto bind_name_id = bind_names().Add(
       {.name_id = name_id, .enclosing_scope_id = SemIR::NameScopeId::Package});
-  AddInst(SemIR::BindName{.parse_node = import_node,
-                          .type_id = type_id,
-                          .bind_name_id = bind_name_id,
-                          .value_id = inst_id});
+  AddInst({import_node, SemIR::BindName{.type_id = type_id,
+                                        .bind_name_id = bind_name_id,
+                                        .value_id = inst_id}});
 }
 
 auto Context::AddNameToLookup(Parse::NodeId name_node, SemIR::NameId name_id,
@@ -205,7 +205,6 @@ auto Context::ResolveIfLazyImportRef(SemIR::InstId inst_id) -> void {
                            .return_type_id = SemIR::TypeId::Invalid,
                            .return_slot_id = SemIR::InstId::Invalid});
       insts().Set(inst_id, SemIR::FunctionDecl{
-                               Parse::NodeId::Invalid,
                                GetBuiltinType(SemIR::BuiltinKind::FunctionType),
                                function_id});
       constant_values().Set(inst_id,
@@ -220,8 +219,7 @@ auto Context::ResolveIfLazyImportRef(SemIR::InstId inst_id) -> void {
       // error.
       TODO(Parse::NodeId::Invalid,
            (llvm::Twine("TODO: support ") + import_inst.kind().name()).str());
-      insts().Set(inst_id, SemIR::VarStorage{Parse::NodeId::Invalid,
-                                             SemIR::TypeId::Error,
+      insts().Set(inst_id, SemIR::VarStorage{SemIR::TypeId::Error,
                                              SemIR::NameId::PackageNamespace});
       break;
   }
@@ -462,7 +460,7 @@ static auto AddDominatedBlockAndBranchImpl(Context& context,
     return SemIR::InstBlockId::Unreachable;
   }
   auto block_id = context.inst_blocks().AddDefaultValue();
-  context.AddInst(BranchNode{parse_node, block_id, args...});
+  context.AddInst({parse_node, BranchNode{block_id, args...}});
   return block_id;
 }
 
@@ -495,7 +493,7 @@ auto Context::AddConvergenceBlockAndPush(Parse::NodeId parse_node,
       if (new_block_id == SemIR::InstBlockId::Unreachable) {
         new_block_id = inst_blocks().AddDefaultValue();
       }
-      AddInst(SemIR::Branch{parse_node, new_block_id});
+      AddInst({parse_node, SemIR::Branch{new_block_id}});
     }
     inst_block_stack().Pop();
   }
@@ -513,7 +511,7 @@ auto Context::AddConvergenceBlockWithArgAndPush(
       if (new_block_id == SemIR::InstBlockId::Unreachable) {
         new_block_id = inst_blocks().AddDefaultValue();
       }
-      AddInst(SemIR::BranchWithArg{parse_node, new_block_id, arg_id});
+      AddInst({parse_node, SemIR::BranchWithArg{new_block_id, arg_id}});
     }
     inst_block_stack().Pop();
   }
@@ -521,7 +519,7 @@ auto Context::AddConvergenceBlockWithArgAndPush(
 
   // Acquire the result value.
   SemIR::TypeId result_type_id = insts().Get(*block_args.begin()).type_id();
-  return AddInst(SemIR::BlockArg{parse_node, result_type_id, new_block_id});
+  return AddInst({parse_node, SemIR::BlockArg{result_type_id, new_block_id}});
 }
 
 // Add the current code block to the enclosing function.
@@ -640,7 +638,8 @@ class TypeCompleter {
       return true;
     }
 
-    auto inst = context_.types().GetAsInst(type_id);
+    auto inst_id = context_.types().GetInstId(type_id);
+    auto inst = context_.insts().Get(inst_id);
     auto old_work_list_size = work_list_.size();
 
     switch (phase) {
@@ -654,7 +653,8 @@ class TypeCompleter {
         break;
 
       case Phase::BuildValueRepr: {
-        auto value_rep = BuildValueRepr(type_id, inst);
+        auto value_rep = BuildValueRepr(
+            type_id, context_.insts().GetParseNode(inst_id), inst);
         context_.sem_ir().CompleteType(type_id, value_rep);
         CARBON_CHECK(old_work_list_size == work_list_.size())
             << "BuildValueRepr should not change work items";
@@ -829,13 +829,13 @@ class TypeCompleter {
     return MakePointerValueRepr(parse_node, elementwise_rep, aggregate_kind);
   }
 
-  auto BuildStructTypeValueRepr(SemIR::TypeId type_id,
+  auto BuildStructTypeValueRepr(SemIR::TypeId type_id, Parse::NodeId parse_node,
                                 SemIR::StructType struct_type) const
       -> SemIR::ValueRepr {
     // TODO: Share more code with tuples.
     auto fields = context_.inst_blocks().Get(struct_type.fields_id);
     if (fields.empty()) {
-      return MakeEmptyValueRepr(struct_type.parse_node);
+      return MakeEmptyValueRepr(parse_node);
     }
 
     // Find the value representation for each field, and construct a struct
@@ -850,32 +850,34 @@ class TypeCompleter {
         same_as_object_rep = false;
         field.field_type_id = field_value_rep.type_id;
         // TODO: Use `TryEvalInst` to form this value.
-        field_id = context_
-                       .AddConstant(field, context_.constant_values()
-                                               .Get(context_.types().GetInstId(
-                                                   field.field_type_id))
-                                               .is_symbolic())
-                       .inst_id();
+        field_id =
+            context_
+                .AddConstant(
+                    {context_.insts().GetParseNode(field_id), field},
+                    context_.constant_values()
+                        .Get(context_.types().GetInstId(field.field_type_id))
+                        .is_symbolic())
+                .inst_id();
       }
       value_rep_fields.push_back(field_id);
     }
 
-    auto value_rep = same_as_object_rep
-                         ? type_id
-                         : context_.CanonicalizeStructType(
-                               struct_type.parse_node,
-                               context_.inst_blocks().Add(value_rep_fields));
-    return BuildStructOrTupleValueRepr(struct_type.parse_node, fields.size(),
-                                       value_rep, same_as_object_rep);
+    auto value_rep =
+        same_as_object_rep
+            ? type_id
+            : context_.CanonicalizeStructType(
+                  parse_node, context_.inst_blocks().Add(value_rep_fields));
+    return BuildStructOrTupleValueRepr(parse_node, fields.size(), value_rep,
+                                       same_as_object_rep);
   }
 
-  auto BuildTupleTypeValueRepr(SemIR::TypeId type_id,
+  auto BuildTupleTypeValueRepr(SemIR::TypeId type_id, Parse::NodeId parse_node,
                                SemIR::TupleType tuple_type) const
       -> SemIR::ValueRepr {
     // TODO: Share more code with structs.
     auto elements = context_.type_blocks().Get(tuple_type.elements_id);
     if (elements.empty()) {
-      return MakeEmptyValueRepr(tuple_type.parse_node);
+      return MakeEmptyValueRepr(parse_node);
     }
 
     // Find the value representation for each element, and construct a tuple
@@ -891,18 +893,17 @@ class TypeCompleter {
       value_rep_elements.push_back(element_value_rep.type_id);
     }
 
-    auto value_rep = same_as_object_rep
-                         ? type_id
-                         : context_.CanonicalizeTupleType(tuple_type.parse_node,
-                                                          value_rep_elements);
-    return BuildStructOrTupleValueRepr(tuple_type.parse_node, elements.size(),
-                                       value_rep, same_as_object_rep);
+    auto value_rep = same_as_object_rep ? type_id
+                                        : context_.CanonicalizeTupleType(
+                                              parse_node, value_rep_elements);
+    return BuildStructOrTupleValueRepr(parse_node, elements.size(), value_rep,
+                                       same_as_object_rep);
   }
 
   // Builds and returns the value representation for the given type. All nested
   // types, as found by AddNestedIncompleteTypes, are known to be complete.
-  auto BuildValueRepr(SemIR::TypeId type_id, SemIR::Inst inst) const
-      -> SemIR::ValueRepr {
+  auto BuildValueRepr(SemIR::TypeId type_id, Parse::NodeId parse_node,
+                      SemIR::Inst inst) const -> SemIR::ValueRepr {
     // TODO: This can emit new SemIR instructions. Consider emitting them into a
     // dedicated file-scope instruction block where possible, or somewhere else
     // that better reflects the definition of the type, rather than wherever the
@@ -971,15 +972,17 @@ class TypeCompleter {
         // For arrays, it's convenient to always use a pointer representation,
         // even when the array has zero or one element, in order to support
         // indexing.
-        return MakePointerValueRepr(inst.parse_node(), type_id,
+        return MakePointerValueRepr(parse_node, type_id,
                                     SemIR::ValueRepr::ObjectAggregate);
       }
 
       case SemIR::StructType::Kind:
-        return BuildStructTypeValueRepr(type_id, inst.As<SemIR::StructType>());
+        return BuildStructTypeValueRepr(type_id, parse_node,
+                                        inst.As<SemIR::StructType>());
 
       case SemIR::TupleType::Kind:
-        return BuildTupleTypeValueRepr(type_id, inst.As<SemIR::TupleType>());
+        return BuildTupleTypeValueRepr(type_id, parse_node,
+                                       inst.As<SemIR::TupleType>());
 
       case SemIR::ClassType::Kind:
         // The value representation for a class is a pointer to the object
@@ -987,7 +990,7 @@ class TypeCompleter {
         // TODO: Support customized value representations for classes.
         // TODO: Pick a better value representation when possible.
         return MakePointerValueRepr(
-            inst.parse_node(),
+            parse_node,
             context_.classes()
                 .Get(inst.As<SemIR::ClassType>().class_id)
                 .object_repr_id,
@@ -1154,14 +1157,17 @@ static auto ProfileType(Context& semantics_context, SemIR::Inst inst,
   return true;
 }
 
-auto Context::CanonicalizeTypeAndAddInstIfNew(SemIR::Inst inst)
+auto Context::CanonicalizeTypeAndAddInstIfNew(Parse::NodeId parse_node,
+                                              SemIR::Inst inst)
     -> SemIR::TypeId {
   auto profile_node = [&](llvm::FoldingSetNodeID& canonical_id) {
     return ProfileType(*this, inst, canonical_id);
   };
   auto make_inst = [&] {
     // TODO: Properly determine whether types are symbolic.
-    return AddConstant(inst, /*is_symbolic=*/false).inst_id();
+    return AddConstant(SemIR::ParseNodeAndInst::Untyped(parse_node, inst),
+                       /*is_symbolic=*/false)
+        .inst_id();
   };
   return CanonicalizeTypeImpl(inst.kind(), profile_node, make_inst);
 }
@@ -1189,7 +1195,7 @@ auto Context::CanonicalizeStructType(Parse::NodeId parse_node,
                                      SemIR::InstBlockId refs_id)
     -> SemIR::TypeId {
   return CanonicalizeTypeAndAddInstIfNew(
-      SemIR::StructType{parse_node, SemIR::TypeId::TypeType, refs_id});
+      parse_node, SemIR::StructType{SemIR::TypeId::TypeType, refs_id});
 }
 
 auto Context::CanonicalizeTupleType(Parse::NodeId parse_node,
@@ -1202,9 +1208,10 @@ auto Context::CanonicalizeTupleType(Parse::NodeId parse_node,
   };
   auto make_tuple_inst = [&] {
     // TODO: Properly determine when types are symbolic.
-    return AddConstant(SemIR::TupleType{parse_node, SemIR::TypeId::TypeType,
-                                        type_blocks().Add(type_ids)},
-                       /*is_symbolic=*/false)
+    return AddConstant(
+               {parse_node, SemIR::TupleType{SemIR::TypeId::TypeType,
+                                             type_blocks().Add(type_ids)}},
+               /*is_symbolic=*/false)
         .inst_id();
   };
   return CanonicalizeTypeImpl(SemIR::TupleType::Kind, profile_tuple,
@@ -1223,7 +1230,7 @@ auto Context::GetBuiltinType(SemIR::BuiltinKind kind) -> SemIR::TypeId {
 auto Context::GetPointerType(Parse::NodeId parse_node,
                              SemIR::TypeId pointee_type_id) -> SemIR::TypeId {
   return CanonicalizeTypeAndAddInstIfNew(
-      SemIR::PointerType{parse_node, SemIR::TypeId::TypeType, pointee_type_id});
+      parse_node, SemIR::PointerType{SemIR::TypeId::TypeType, pointee_type_id});
 }
 
 auto Context::GetUnqualifiedType(SemIR::TypeId type_id) -> SemIR::TypeId {

+ 15 - 4
toolchain/check/context.h

@@ -19,6 +19,7 @@
 #include "toolchain/sem_ir/file.h"
 #include "toolchain/sem_ir/ids.h"
 #include "toolchain/sem_ir/inst.h"
+#include "toolchain/sem_ir/value_stores.h"
 
 namespace Carbon::Check {
 
@@ -57,14 +58,15 @@ class Context {
   auto VerifyOnFinish() -> void;
 
   // Adds an instruction to the current block, returning the produced ID.
-  auto AddInst(SemIR::Inst inst) -> SemIR::InstId;
+  auto AddInst(SemIR::ParseNodeAndInst parse_node_and_inst) -> SemIR::InstId;
 
   // Adds an instruction to the constants block, returning the produced ID.
-  auto AddConstant(SemIR::Inst inst, bool is_symbolic) -> SemIR::ConstantId;
+  auto AddConstant(SemIR::ParseNodeAndInst parse_node_and_inst,
+                   bool is_symbolic) -> SemIR::ConstantId;
 
   // Pushes a parse tree node onto the stack, storing the SemIR::Inst as the
   // result.
-  auto AddInstAndPush(Parse::NodeId parse_node, SemIR::Inst inst) -> void;
+  auto AddInstAndPush(SemIR::ParseNodeAndInst parse_node_and_inst) -> void;
 
   // Adds a package's imports to name lookup, with all libraries together.
   // sem_irs will all be non-null; has_load_error must be used for any errors.
@@ -135,6 +137,14 @@ class Context {
     return current_scope().scope_id;
   }
 
+  auto GetCurrentScopeParseNode() const -> Parse::NodeId {
+    auto current_scope_inst_id = current_scope().scope_inst_id;
+    if (!current_scope_inst_id.is_valid()) {
+      return Parse::NodeId::Invalid;
+    }
+    return sem_ir_->insts().GetParseNode(current_scope_inst_id);
+  }
+
   // Returns true if currently at file scope.
   auto at_file_scope() const -> bool { return scope_stack_.size() == 1; }
 
@@ -449,7 +459,8 @@ class Context {
 
   // Forms a canonical type ID for a type. If the type is new, adds the
   // instruction to the current block.
-  auto CanonicalizeTypeAndAddInstIfNew(SemIR::Inst inst) -> SemIR::TypeId;
+  auto CanonicalizeTypeAndAddInstIfNew(Parse::NodeId parse_node,
+                                       SemIR::Inst inst) -> SemIR::TypeId;
 
   // If the passed in instruction ID is a LazyImportRef, resolves it for use.
   // Called when name lookup intends to return an inst_id.

+ 82 - 67
toolchain/check/convert.cpp

@@ -94,8 +94,9 @@ static auto FinalizeTemporary(Context& context, SemIR::InstId init_id,
         << "initialized multiple times? Have "
         << sem_ir.insts().Get(return_slot_id);
     auto init = sem_ir.insts().Get(init_id);
-    return context.AddInst(SemIR::Temporary{init.parse_node(), init.type_id(),
-                                            return_slot_id, init_id});
+    return context.AddInst(
+        {sem_ir.insts().GetParseNode(init_id),
+         SemIR::Temporary{init.type_id(), return_slot_id, init_id}});
   }
 
   if (discarded) {
@@ -109,10 +110,11 @@ static auto FinalizeTemporary(Context& context, SemIR::InstId init_id,
   // materialize and initialize a temporary, rather than two separate
   // instructions.
   auto init = sem_ir.insts().Get(init_id);
-  auto temporary_id = context.AddInst(
-      SemIR::TemporaryStorage{init.parse_node(), init.type_id()});
-  return context.AddInst(SemIR::Temporary{init.parse_node(), init.type_id(),
-                                          temporary_id, init_id});
+  auto parse_node = sem_ir.insts().GetParseNode(init_id);
+  auto temporary_id =
+      context.AddInst({parse_node, SemIR::TemporaryStorage{init.type_id()}});
+  return context.AddInst(
+      {parse_node, SemIR::Temporary{init.type_id(), temporary_id, init_id}});
 }
 
 // Materialize a temporary to hold the result of the given expression if it is
@@ -136,14 +138,15 @@ static auto MakeElementAccessInst(Context& context, Parse::NodeId parse_node,
     // TODO: Add a new instruction kind for indexing an array at a constant
     // index so that we don't need an integer literal instruction here, and
     // remove this special case.
-    auto index_id = block.AddInst(SemIR::IntLiteral{
-        parse_node, context.GetBuiltinType(SemIR::BuiltinKind::IntType),
-        context.sem_ir().ints().Add(llvm::APInt(32, i))});
+    auto index_id = block.AddInst(
+        {parse_node,
+         SemIR::IntLiteral{context.GetBuiltinType(SemIR::BuiltinKind::IntType),
+                           context.sem_ir().ints().Add(llvm::APInt(32, i))}});
     return block.AddInst(
-        AccessInstT{parse_node, elem_type_id, aggregate_id, index_id});
+        {parse_node, AccessInstT{elem_type_id, aggregate_id, index_id}});
   } else {
-    return block.AddInst(AccessInstT{parse_node, elem_type_id, aggregate_id,
-                                     SemIR::ElementIndex(i)});
+    return block.AddInst({parse_node, AccessInstT{elem_type_id, aggregate_id,
+                                                  SemIR::ElementIndex(i)}});
   }
 }
 
@@ -243,6 +246,7 @@ static auto ConvertTupleToArray(Context& context, SemIR::TupleType tuple_type,
   auto tuple_elem_types = sem_ir.type_blocks().Get(tuple_type.elements_id);
 
   auto value = sem_ir.insts().Get(value_id);
+  auto value_parse_node = sem_ir.insts().GetParseNode(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
@@ -265,7 +269,7 @@ static auto ConvertTupleToArray(Context& context, SemIR::TupleType tuple_type,
                       "Cannot initialize array of {0} element(s) from tuple "
                       "with {1} element(s).",
                       uint64_t, size_t);
-    context.emitter().Emit(value.parse_node(),
+    context.emitter().Emit(value_parse_node,
                            literal_elems.empty()
                                ? ArrayInitFromExprArgCountMismatch
                                : ArrayInitFromLiteralArgCountMismatch,
@@ -282,7 +286,7 @@ static auto ConvertTupleToArray(Context& context, SemIR::TupleType tuple_type,
   SemIR::InstId return_slot_id = target.init_id;
   if (!target.init_id.is_valid()) {
     return_slot_id = target_block->AddInst(
-        SemIR::TemporaryStorage{value.parse_node(), target.type_id});
+        {value_parse_node, SemIR::TemporaryStorage{target.type_id}});
   }
 
   // Initialize each element of the array from the corresponding element of the
@@ -296,7 +300,7 @@ static auto ConvertTupleToArray(Context& context, SemIR::TupleType tuple_type,
     // approach.
     auto init_id =
         ConvertAggregateElement<SemIR::TupleAccess, SemIR::ArrayIndex>(
-            context, value.parse_node(), value_id, src_type_id, literal_elems,
+            context, value_parse_node, value_id, src_type_id, literal_elems,
             ConversionTarget::FullInitializer, return_slot_id,
             array_type.element_type_id, target_block, i);
     if (init_id == SemIR::InstId::BuiltinError) {
@@ -308,9 +312,10 @@ static auto ConvertTupleToArray(Context& context, SemIR::TupleType tuple_type,
   // Flush the temporary here if we didn't insert it earlier, so we can add a
   // reference to the return slot.
   target_block->InsertHere();
-  return context.AddInst(SemIR::ArrayInit{value.parse_node(), target.type_id,
-                                          sem_ir.inst_blocks().Add(inits),
-                                          return_slot_id});
+  return context.AddInst(
+      {value_parse_node,
+       SemIR::ArrayInit{target.type_id, sem_ir.inst_blocks().Add(inits),
+                        return_slot_id}});
 }
 
 // Performs a conversion from a tuple to a tuple type. This function only
@@ -325,6 +330,7 @@ static auto ConvertTupleToTuple(Context& context, SemIR::TupleType src_type,
   auto dest_elem_types = sem_ir.type_blocks().Get(dest_type.elements_id);
 
   auto value = sem_ir.insts().Get(value_id);
+  auto value_parse_node = sem_ir.insts().GetParseNode(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
@@ -344,7 +350,7 @@ static auto ConvertTupleToTuple(Context& context, SemIR::TupleType src_type,
                       "Cannot initialize tuple of {0} element(s) from tuple "
                       "with {1} element(s).",
                       size_t, size_t);
-    context.emitter().Emit(value.parse_node(), TupleInitElementCountMismatch,
+    context.emitter().Emit(value_parse_node, TupleInitElementCountMismatch,
                            dest_elem_types.size(), src_elem_types.size());
     return SemIR::InstId::BuiltinError;
   }
@@ -371,7 +377,7 @@ static auto ConvertTupleToTuple(Context& context, SemIR::TupleType src_type,
     // approach.
     auto init_id =
         ConvertAggregateElement<SemIR::TupleAccess, SemIR::TupleAccess>(
-            context, value.parse_node(), value_id, src_type_id, literal_elems,
+            context, value_parse_node, value_id, src_type_id, literal_elems,
             inner_kind, target.init_id, dest_type_id, target.init_block, i);
     if (init_id == SemIR::InstId::BuiltinError) {
       return SemIR::InstId::BuiltinError;
@@ -381,11 +387,12 @@ static auto ConvertTupleToTuple(Context& context, SemIR::TupleType src_type,
 
   if (is_init) {
     target.init_block->InsertHere();
-    return context.AddInst(SemIR::TupleInit{value.parse_node(), target.type_id,
-                                            new_block.id(), target.init_id});
+    return context.AddInst(
+        {value_parse_node,
+         SemIR::TupleInit{target.type_id, new_block.id(), target.init_id}});
   } else {
     return context.AddInst(
-        SemIR::TupleValue{value.parse_node(), target.type_id, new_block.id()});
+        {value_parse_node, SemIR::TupleValue{target.type_id, new_block.id()}});
   }
 }
 
@@ -402,6 +409,7 @@ static auto ConvertStructToStructOrClass(Context& context,
   auto dest_elem_fields = sem_ir.inst_blocks().Get(dest_type.fields_id);
 
   auto value = sem_ir.insts().Get(value_id);
+  auto value_parse_node = sem_ir.insts().GetParseNode(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
@@ -424,7 +432,7 @@ static auto ConvertStructToStructOrClass(Context& context,
                       "with {2} field(s).",
                       llvm::StringLiteral, size_t, size_t);
     context.emitter().Emit(
-        value.parse_node(), StructInitElementCountMismatch,
+        value_parse_node, StructInitElementCountMismatch,
         is_class ? llvm::StringLiteral("class") : llvm::StringLiteral("struct"),
         dest_elem_fields.size(), src_elem_fields.size());
     return SemIR::InstId::BuiltinError;
@@ -471,7 +479,7 @@ static auto ConvertStructToStructOrClass(Context& context,
               "Missing value for field `{0}` in struct initialization.",
               std::string);
           context.emitter().Emit(
-              value.parse_node(), StructInitMissingFieldInLiteral,
+              value_parse_node, StructInitMissingFieldInLiteral,
               sem_ir.names().GetFormatted(dest_field.name_id).str());
         } else {
           CARBON_DIAGNOSTIC(StructInitMissingFieldInConversion, Error,
@@ -479,7 +487,7 @@ static auto ConvertStructToStructOrClass(Context& context,
                             "missing field `{2}` in source type.",
                             std::string, std::string, std::string);
           context.emitter().Emit(
-              value.parse_node(), StructInitMissingFieldInConversion,
+              value_parse_node, StructInitMissingFieldInConversion,
               sem_ir.StringifyType(value.type_id()),
               sem_ir.StringifyType(target.type_id),
               sem_ir.names().GetFormatted(dest_field.name_id).str());
@@ -495,7 +503,7 @@ static auto ConvertStructToStructOrClass(Context& context,
     // approach.
     auto init_id =
         ConvertAggregateElement<SemIR::StructAccess, TargetAccessInstT>(
-            context, value.parse_node(), value_id, src_field.field_type_id,
+            context, value_parse_node, value_id, src_field.field_type_id,
             literal_elems, inner_kind, target.init_id, dest_field.field_type_id,
             target.init_block, src_field_index);
     if (init_id == SemIR::InstId::BuiltinError) {
@@ -508,15 +516,17 @@ static auto ConvertStructToStructOrClass(Context& context,
     target.init_block->InsertHere();
     CARBON_CHECK(is_init)
         << "Converting directly to a class value is not supported";
-    return context.AddInst(SemIR::ClassInit{value.parse_node(), target.type_id,
-                                            new_block.id(), target.init_id});
+    return context.AddInst(
+        {value_parse_node,
+         SemIR::ClassInit{target.type_id, new_block.id(), target.init_id}});
   } else if (is_init) {
     target.init_block->InsertHere();
-    return context.AddInst(SemIR::StructInit{value.parse_node(), target.type_id,
-                                             new_block.id(), target.init_id});
+    return context.AddInst(
+        {value_parse_node,
+         SemIR::StructInit{target.type_id, new_block.id(), target.init_id}});
   } else {
     return context.AddInst(
-        SemIR::StructValue{value.parse_node(), target.type_id, new_block.id()});
+        {value_parse_node, SemIR::StructValue{target.type_id, new_block.id()}});
   }
 }
 
@@ -545,7 +555,7 @@ static auto ConvertStructToClass(Context& context, SemIR::StructType src_type,
                       "Cannot construct instance of abstract class. "
                       "Consider using `partial {0}` instead.",
                       std::string);
-    context.emitter().Emit(context.insts().Get(value_id).parse_node(),
+    context.emitter().Emit(context.insts().GetParseNode(value_id),
                            ConstructionOfAbstractClass,
                            context.sem_ir().StringifyType(target.type_id));
     return SemIR::InstId::BuiltinError;
@@ -559,8 +569,9 @@ static auto ConvertStructToClass(Context& context, SemIR::StructType src_type,
   if (need_temporary) {
     target.kind = ConversionTarget::Initializer;
     target.init_block = &target_block;
-    target.init_id = target_block.AddInst(SemIR::TemporaryStorage{
-        context.insts().Get(value_id).parse_node(), target.type_id});
+    target.init_id =
+        target_block.AddInst({context.insts().GetParseNode(value_id),
+                              SemIR::TemporaryStorage{target.type_id}});
   }
 
   auto result_id = ConvertStructToStructOrClass<SemIR::ClassElementAccess>(
@@ -569,8 +580,8 @@ static auto ConvertStructToClass(Context& context, SemIR::StructType src_type,
   if (need_temporary) {
     target_block.InsertHere();
     result_id = context.AddInst(
-        SemIR::Temporary{context.insts().Get(value_id).parse_node(),
-                         target.type_id, target.init_id, result_id});
+        {context.insts().GetParseNode(value_id),
+         SemIR::Temporary{target.type_id, target.init_id, result_id}});
   }
   return result_id;
 }
@@ -625,8 +636,9 @@ static auto ConvertDerivedToBase(Context& context, Parse::NodeId parse_node,
   // Add a series of `.base` accesses.
   for (auto base_id : path) {
     auto base_decl = context.insts().GetAs<SemIR::BaseDecl>(base_id);
-    value_id = context.AddInst(SemIR::ClassElementAccess{
-        parse_node, base_decl.base_type_id, value_id, base_decl.index});
+    value_id = context.AddInst(
+        {parse_node, SemIR::ClassElementAccess{base_decl.base_type_id, value_id,
+                                               base_decl.index}});
   }
   return value_id;
 }
@@ -639,13 +651,13 @@ static auto ConvertDerivedPointerToBasePointer(
   // Form `*p`.
   ptr_id = ConvertToValueExpr(context, ptr_id);
   auto ref_id = context.AddInst(
-      SemIR::Deref{parse_node, src_ptr_type.pointee_id, ptr_id});
+      {parse_node, SemIR::Deref{src_ptr_type.pointee_id, ptr_id}});
 
   // Convert as a reference expression.
   ref_id = ConvertDerivedToBase(context, parse_node, ref_id, path);
 
   // Take the address.
-  return context.AddInst(SemIR::AddrOf{parse_node, dest_ptr_type_id, ref_id});
+  return context.AddInst({parse_node, SemIR::AddrOf{dest_ptr_type_id, ref_id}});
 }
 
 // Returns whether `category` is a valid expression category to produce as a
@@ -728,7 +740,7 @@ static auto PerformBuiltinConversion(Context& context, Parse::NodeId parse_node,
         // value representation is a copy of the object representation, so we
         // already have a value of the right form.
         return context.AddInst(
-            SemIR::ValueOfInitializer{parse_node, value_type_id, value_id});
+            {parse_node, SemIR::ValueOfInitializer{value_type_id, value_id}});
       }
     }
   }
@@ -852,7 +864,8 @@ static auto PerformCopy(Context& context, SemIR::InstId expr_id)
   // copyable, or how to perform the copy.
   CARBON_DIAGNOSTIC(CopyOfUncopyableType, Error,
                     "Cannot copy value of type `{0}`.", std::string);
-  context.emitter().Emit(expr.parse_node(), CopyOfUncopyableType,
+  context.emitter().Emit(context.insts().GetParseNode(expr_id),
+                         CopyOfUncopyableType,
                          context.sem_ir().StringifyType(type_id));
   return SemIR::InstId::BuiltinError;
 }
@@ -875,7 +888,7 @@ auto Convert(Context& context, Parse::NodeId parse_node, SemIR::InstId expr_id,
     // namespace names, and allow use of functions as values.
     CARBON_DIAGNOSTIC(UseOfNonExprAsValue, Error,
                       "Expression cannot be used as a value.");
-    context.emitter().Emit(sem_ir.insts().Get(expr_id).parse_node(),
+    context.emitter().Emit(sem_ir.insts().GetParseNode(expr_id),
                            UseOfNonExprAsValue);
     return SemIR::InstId::BuiltinError;
   }
@@ -931,8 +944,9 @@ auto Convert(Context& context, Parse::NodeId parse_node, SemIR::InstId expr_id,
 
   // Track that we performed a type conversion, if we did so.
   if (orig_expr_id != expr_id) {
-    expr_id = context.AddInst(SemIR::Converted{
-        expr.parse_node(), target.type_id, orig_expr_id, expr_id});
+    expr_id = context.AddInst(
+        {context.insts().GetParseNode(expr_id),
+         SemIR::Converted{target.type_id, orig_expr_id, expr_id}});
   }
 
   // For `as`, don't perform any value category conversions. In particular, an
@@ -982,8 +996,8 @@ auto Convert(Context& context, Parse::NodeId parse_node, 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 = context.AddInst(
-          SemIR::BindValue{expr.parse_node(), expr.type_id(), expr_id});
+      expr_id = context.AddInst({context.insts().GetParseNode(expr_id),
+                                 SemIR::BindValue{expr.type_id(), expr_id}});
       // We now have a value expression.
       [[fallthrough]];
 
@@ -1000,8 +1014,9 @@ auto Convert(Context& context, Parse::NodeId parse_node, SemIR::InstId expr_id,
     if (auto init_rep = SemIR::GetInitRepr(sem_ir, target.type_id);
         init_rep.kind == SemIR::InitRepr::ByCopy) {
       target.init_block->InsertHere();
-      expr_id = context.AddInst(SemIR::InitializeFrom{
-          parse_node, target.type_id, expr_id, target.init_id});
+      expr_id = context.AddInst(
+          {parse_node,
+           SemIR::InitializeFrom{target.type_id, expr_id, target.init_id}});
     }
   }
 
@@ -1021,17 +1036,16 @@ auto Initialize(Context& context, Parse::NodeId parse_node,
 
 auto ConvertToValueExpr(Context& context, SemIR::InstId expr_id)
     -> SemIR::InstId {
-  auto expr = context.sem_ir().insts().Get(expr_id);
-  return Convert(context, expr.parse_node(), expr_id,
-                 {.kind = ConversionTarget::Value, .type_id = expr.type_id()});
+  return Convert(context, context.insts().GetParseNode(expr_id), expr_id,
+                 {.kind = ConversionTarget::Value,
+                  .type_id = context.sem_ir().insts().Get(expr_id).type_id()});
 }
 
 auto ConvertToValueOrRefExpr(Context& context, SemIR::InstId expr_id)
     -> SemIR::InstId {
-  auto expr = context.sem_ir().insts().Get(expr_id);
-  return Convert(
-      context, expr.parse_node(), expr_id,
-      {.kind = ConversionTarget::ValueOrRef, .type_id = expr.type_id()});
+  return Convert(context, context.insts().GetParseNode(expr_id), expr_id,
+                 {.kind = ConversionTarget::ValueOrRef,
+                  .type_id = context.sem_ir().insts().Get(expr_id).type_id()});
 }
 
 auto ConvertToValueOfType(Context& context, Parse::NodeId parse_node,
@@ -1068,8 +1082,8 @@ CARBON_DIAGNOSTIC(InCallToFunction, Note, "Calling function declared here.");
 static auto ConvertSelf(Context& context, Parse::NodeId call_parse_node,
                         Parse::NodeId callee_parse_node,
                         std::optional<SemIR::AddrPattern> addr_pattern,
-                        SemIR::Param self_param, SemIR::InstId self_id)
-    -> SemIR::InstId {
+                        SemIR::InstId self_param_id, SemIR::Param self_param,
+                        SemIR::InstId self_id) -> SemIR::InstId {
   if (!self_id.is_valid()) {
     CARBON_DIAGNOSTIC(MissingObjectInMethodCall, Error,
                       "Missing object argument in method call.");
@@ -1086,7 +1100,8 @@ static auto ConvertSelf(Context& context, Parse::NodeId call_parse_node,
             InCallToFunctionSelf, Note,
             "Initializing `{0}` parameter of method declared here.",
             llvm::StringLiteral);
-        builder.Note(self_param.parse_node, InCallToFunctionSelf,
+        builder.Note(context.insts().GetParseNode(self_param_id),
+                     InCallToFunctionSelf,
                      addr_pattern ? llvm::StringLiteral("addr self")
                                   : llvm::StringLiteral("self"));
       });
@@ -1107,10 +1122,11 @@ static auto ConvertSelf(Context& context, Parse::NodeId call_parse_node,
         context.emitter().Emit(TokenOnly(call_parse_node), AddrSelfIsNonRef);
         return SemIR::InstId::BuiltinError;
     }
+    auto parse_node = context.insts().GetParseNode(self_or_addr_id);
     self_or_addr_id = context.AddInst(
-        SemIR::AddrOf{self.parse_node(),
-                      context.GetPointerType(self.parse_node(), self.type_id()),
-                      self_or_addr_id});
+        {parse_node,
+         SemIR::AddrOf{context.GetPointerType(parse_node, self.type_id()),
+                       self_or_addr_id}});
   }
 
   return ConvertToValueOfType(context, call_parse_node, self_or_addr_id,
@@ -1151,13 +1167,12 @@ auto ConvertCallArgs(Context& context, Parse::NodeId call_parse_node,
   for (auto implicit_param_id : implicit_param_refs) {
     auto addr_pattern =
         context.insts().TryGetAs<SemIR::AddrPattern>(implicit_param_id);
-    auto param = SemIR::Function::GetParamFromParamRefId(context.sem_ir(),
-                                                         implicit_param_id)
-                     .second;
+    auto [param_id, param] = SemIR::Function::GetParamFromParamRefId(
+        context.sem_ir(), implicit_param_id);
     if (param.name_id == SemIR::NameId::SelfValue) {
       auto converted_self_id =
           ConvertSelf(context, call_parse_node, callee_parse_node, addr_pattern,
-                      param, self_id);
+                      param_id, param, self_id);
       if (converted_self_id == SemIR::InstId::BuiltinError) {
         return SemIR::InstBlockId::Invalid;
       }

+ 20 - 10
toolchain/check/eval.cpp

@@ -5,6 +5,7 @@
 #include "toolchain/check/eval.h"
 
 #include "toolchain/sem_ir/typed_insts.h"
+#include "toolchain/sem_ir/value_stores.h"
 
 namespace Carbon::Check {
 
@@ -65,7 +66,8 @@ static auto ReplaceFieldWithConstantValue(Context& context, InstT* inst,
 // replaces the fields with their constant values and builds a corresponding
 // constant value. Otherwise returns `SemIR::InstId::Invalid`.
 template <typename InstT, typename... EachFieldIdT>
-static auto RebuildIfFieldsAreConstant(Context& context, SemIR::Inst inst,
+static auto RebuildIfFieldsAreConstant(Context& context, SemIR::InstId inst_id,
+                                       SemIR::Inst inst,
                                        EachFieldIdT InstT::*... each_field_id)
     -> SemIR::ConstantId {
   // Build a constant instruction by replacing each non-constant operand with
@@ -75,7 +77,10 @@ static auto RebuildIfFieldsAreConstant(Context& context, SemIR::Inst inst,
   if ((ReplaceFieldWithConstantValue(context, &typed_inst, each_field_id,
                                      &is_symbolic) &&
        ...)) {
-    return context.AddConstant(typed_inst, is_symbolic);
+    return context.AddConstant(
+        SemIR::ParseNodeAndInst::Untyped(context.insts().GetParseNode(inst_id),
+                                         typed_inst),
+        is_symbolic);
   }
   return SemIR::ConstantId::NotConstant;
 }
@@ -90,23 +95,23 @@ auto TryEvalInst(Context& context, SemIR::InstId inst_id, SemIR::Inst inst)
   switch (inst.kind()) {
     // These cases are constants if their operands are.
     case SemIR::AddrOf::Kind:
-      return RebuildIfFieldsAreConstant(context, inst,
+      return RebuildIfFieldsAreConstant(context, inst_id, inst,
                                         &SemIR::AddrOf::lvalue_id);
     case SemIR::ArrayType::Kind:
-      return RebuildIfFieldsAreConstant(context, inst,
+      return RebuildIfFieldsAreConstant(context, inst_id, inst,
                                         &SemIR::ArrayType::bound_id);
     case SemIR::BoundMethod::Kind:
-      return RebuildIfFieldsAreConstant(context, inst,
+      return RebuildIfFieldsAreConstant(context, inst_id, inst,
                                         &SemIR::BoundMethod::object_id,
                                         &SemIR::BoundMethod::function_id);
     case SemIR::StructValue::Kind:
-      return RebuildIfFieldsAreConstant(context, inst,
+      return RebuildIfFieldsAreConstant(context, inst_id, inst,
                                         &SemIR::StructValue::elements_id);
     case SemIR::Temporary::Kind:
-      return RebuildIfFieldsAreConstant(context, inst,
+      return RebuildIfFieldsAreConstant(context, inst_id, inst,
                                         &SemIR::Temporary::init_id);
     case SemIR::TupleValue::Kind:
-      return RebuildIfFieldsAreConstant(context, inst,
+      return RebuildIfFieldsAreConstant(context, inst_id, inst,
                                         &SemIR::TupleValue::elements_id);
 
     // These cases are constants already.
@@ -132,7 +137,10 @@ auto TryEvalInst(Context& context, SemIR::InstId inst_id, SemIR::Inst inst)
     case SemIR::RealLiteral::Kind:
     case SemIR::StringLiteral::Kind:
       // Promote literals to the constant block.
-      return context.AddConstant(inst, /*is_symbolic=*/false);
+      return context.AddConstant(
+          SemIR::ParseNodeAndInst::Untyped(
+              context.insts().GetParseNode(inst_id), inst),
+          /*is_symbolic=*/false);
 
     // TODO: These need special handling.
     case SemIR::ArrayIndex::Kind:
@@ -186,7 +194,9 @@ auto TryEvalInst(Context& context, SemIR::InstId inst_id, SemIR::Inst inst)
       value.value =
           (value.value == SemIR::BoolValue::False ? SemIR::BoolValue::True
                                                   : SemIR::BoolValue::False);
-      return context.AddConstant(value, /*is_symbolic=*/false);
+      return context.AddConstant(
+          {context.insts().GetParseNode(const_id.inst_id()), value},
+          /*is_symbolic=*/false);
     }
 
     // These cases are either not expressions or not constant.

+ 3 - 4
toolchain/check/handle_array.cpp

@@ -38,10 +38,9 @@ auto HandleArrayExpr(Context& context, Parse::ArrayExprId parse_node) -> bool {
     // TODO: Produce an error if the array type is too large.
     if (bound_value.getActiveBits() <= 64) {
       context.AddInstAndPush(
-          parse_node,
-          SemIR::ArrayType{
-              parse_node, SemIR::TypeId::TypeType, bound_inst_id,
-              ExprAsType(context, parse_node, element_type_inst_id)});
+          {parse_node, SemIR::ArrayType{SemIR::TypeId::TypeType, bound_inst_id,
+                                        ExprAsType(context, parse_node,
+                                                   element_type_inst_id)}});
       return true;
     }
   }

+ 19 - 16
toolchain/check/handle_binding_pattern.cpp

@@ -6,6 +6,7 @@
 #include "toolchain/check/convert.h"
 #include "toolchain/check/return.h"
 #include "toolchain/sem_ir/inst.h"
+#include "toolchain/sem_ir/value_stores.h"
 
 namespace Carbon::Check {
 
@@ -24,17 +25,17 @@ auto HandleAnyBindingPattern(Context& context, Parse::NodeId parse_node,
   // Create the appropriate kind of binding for this pattern.
   auto make_bind_name = [&, name_node = name_node, name_id = name_id](
                             SemIR::TypeId type_id,
-                            SemIR::InstId value_id) -> SemIR::Inst {
+                            SemIR::InstId value_id) -> SemIR::ParseNodeAndInst {
     // TODO: Eventually the name will need to support associations with other
     // scopes, but right now we don't support qualified names here.
     auto bind_name_id = context.bind_names().Add(
         {.name_id = name_id, .enclosing_scope_id = context.current_scope_id()});
     if (is_generic) {
       // TODO: Create a `BindTemplateName` instead inside a `template` pattern.
-      return SemIR::BindSymbolicName{name_node, type_id, bind_name_id,
-                                     value_id};
+      return {name_node,
+              SemIR::BindSymbolicName{type_id, bind_name_id, value_id}};
     } else {
-      return SemIR::BindName{name_node, type_id, bind_name_id, value_id};
+      return {name_node, SemIR::BindName{type_id, bind_name_id, value_id}};
     }
   };
 
@@ -90,22 +91,25 @@ auto HandleAnyBindingPattern(Context& context, Parse::NodeId parse_node,
       } else if (enclosing_class_decl) {
         auto& class_info =
             context.classes().Get(enclosing_class_decl->class_id);
-        auto field_type_inst_id = context.AddInst(SemIR::UnboundElementType{
-            binding_id, context.GetBuiltinType(SemIR::BuiltinKind::TypeType),
-            class_info.self_type_id, cast_type_id});
+        auto field_type_inst_id = context.AddInst(
+            {binding_id,
+             SemIR::UnboundElementType{
+                 context.GetBuiltinType(SemIR::BuiltinKind::TypeType),
+                 class_info.self_type_id, cast_type_id}});
         value_type_id = context.CanonicalizeType(field_type_inst_id);
         value_id = context.AddInst(
-            SemIR::FieldDecl{binding_id, value_type_id, name_id,
+            {binding_id, SemIR::FieldDecl{
+                             value_type_id, name_id,
                              SemIR::ElementIndex(context.args_type_info_stack()
                                                      .PeekCurrentBlockContents()
-                                                     .size())});
+                                                     .size())}});
 
         // Add a corresponding field to the object representation of the class.
         context.args_type_info_stack().AddInst(
-            SemIR::StructTypeField{binding_id, name_id, cast_type_id});
+            {binding_id, SemIR::StructTypeField{name_id, cast_type_id}});
       } else {
         value_id = context.AddInst(
-            SemIR::VarStorage{name_node, value_type_id, name_id});
+            {name_node, SemIR::VarStorage{value_type_id, name_id}});
       }
       auto bind_id = context.AddInst(make_bind_name(value_type_id, value_id));
       context.node_stack().Push(parse_node, bind_id);
@@ -123,9 +127,9 @@ auto HandleAnyBindingPattern(Context& context, Parse::NodeId parse_node,
       // TODO: A tuple pattern can appear in other places than function
       // parameters.
       auto param_id =
-          context.AddInst(SemIR::Param{name_node, cast_type_id, name_id});
-      context.AddInstAndPush(parse_node,
-                             make_bind_name(cast_type_id, param_id));
+          context.AddInst({name_node, SemIR::Param{cast_type_id, name_id}});
+      auto bind_id = context.AddInst(make_bind_name(cast_type_id, param_id));
+      context.node_stack().Push(parse_node, bind_id);
       break;
     }
 
@@ -175,8 +179,7 @@ auto HandleAddr(Context& context, Parse::AddrId parse_node) -> bool {
     // TODO: The type of an `addr_pattern` should probably be the non-pointer
     // type, because that's the type that the pattern matches.
     context.AddInstAndPush(
-        parse_node,
-        SemIR::AddrPattern{parse_node, self_param->type_id, self_param_id});
+        {parse_node, SemIR::AddrPattern{self_param->type_id, self_param_id}});
   } else {
     CARBON_DIAGNOSTIC(AddrOnNonSelfParam, Error,
                       "`addr` can only be applied to a `self` parameter.");

+ 8 - 5
toolchain/check/handle_call_expr.cpp

@@ -81,18 +81,21 @@ auto HandleCallExpr(Context& context, Parse::CallExprId parse_node) -> bool {
   if (callable.return_slot_id.is_valid()) {
     // Tentatively put storage for a temporary in the function's return slot.
     // This will be replaced if necessary when we perform initialization.
-    return_storage_id = context.AddInst(
-        SemIR::TemporaryStorage{call_expr_parse_node, callable.return_type_id});
+    return_storage_id =
+        context.AddInst({call_expr_parse_node,
+                         SemIR::TemporaryStorage{callable.return_type_id}});
   }
 
   // Convert the arguments to match the parameters.
   auto converted_args_id =
       ConvertCallArgs(context, call_expr_parse_node, self_id,
                       context.params_or_args_stack().PeekCurrentBlockContents(),
-                      return_storage_id, function_decl->parse_node,
+                      return_storage_id,
+                      context.insts().GetParseNode(function_decl_id.inst_id()),
                       callable.implicit_param_refs_id, callable.param_refs_id);
-  auto call_inst_id = context.AddInst(
-      SemIR::Call{call_expr_parse_node, type_id, callee_id, converted_args_id});
+  auto call_inst_id =
+      context.AddInst({call_expr_parse_node,
+                       SemIR::Call{type_id, callee_id, converted_args_id}});
 
   context.node_stack().Push(parse_node, call_inst_id);
   return true;

+ 22 - 20
toolchain/check/handle_class.cpp

@@ -51,9 +51,8 @@ static auto BuildClassDecl(Context& context, Parse::AnyClassDeclId parse_node)
   auto decl_block_id = context.inst_block_stack().Pop();
 
   // Add the class declaration.
-  auto class_decl =
-      SemIR::ClassDecl{parse_node, SemIR::ClassId::Invalid, decl_block_id};
-  auto class_decl_id = context.AddInst(class_decl);
+  auto class_decl = SemIR::ClassDecl{SemIR::ClassId::Invalid, decl_block_id};
+  auto class_decl_id = context.AddInst({parse_node, class_decl});
 
   // Check whether this is a redeclaration.
   auto existing_id =
@@ -74,7 +73,7 @@ static auto BuildClassDecl(Context& context, Parse::AnyClassDeclId parse_node)
                           "Previously declared here.");
         context.emitter()
             .Build(parse_node, ClassRedeclarationDifferentIntroducer)
-            .Note(existing_class_decl->parse_node,
+            .Note(context.insts().GetParseNode(existing_id),
                   ClassRedeclarationDifferentIntroducerPrevious)
             .Emit();
       }
@@ -102,10 +101,10 @@ static auto BuildClassDecl(Context& context, Parse::AnyClassDeclId parse_node)
 
     // Build the `Self` type.
     auto& class_info = context.classes().Get(class_decl.class_id);
-    class_info.self_type_id =
-        context.CanonicalizeType(context.AddInst(SemIR::ClassType{
-            parse_node, context.GetBuiltinType(SemIR::BuiltinKind::TypeType),
-            class_decl.class_id}));
+    class_info.self_type_id = context.CanonicalizeType(context.AddInst(
+        {parse_node,
+         SemIR::ClassType{context.GetBuiltinType(SemIR::BuiltinKind::TypeType),
+                          class_decl.class_id}}));
   }
 
   // Write the class ID into the ClassDecl.
@@ -135,7 +134,7 @@ auto HandleClassDefinitionStart(Context& context,
     context.emitter()
         .Build(parse_node, ClassRedefinition,
                context.names().GetFormatted(class_info.name_id).str())
-        .Note(context.insts().Get(class_info.definition_id).parse_node(),
+        .Note(context.insts().GetParseNode(class_info.definition_id),
               ClassPreviousDefinition)
         .Emit();
   } else {
@@ -281,8 +280,7 @@ auto HandleBaseDecl(Context& context, Parse::BaseDeclId parse_node) -> bool {
                       "Previous `base` declaration is here.");
     context.emitter()
         .Build(parse_node, BaseRepeated)
-        .Note(context.insts().Get(class_info.base_id).parse_node(),
-              BasePrevious)
+        .Note(context.insts().GetParseNode(class_info.base_id), BasePrevious)
         .Emit();
     return true;
   }
@@ -291,19 +289,23 @@ auto HandleBaseDecl(Context& context, Parse::BaseDeclId parse_node) -> bool {
 
   // The `base` value in the class scope has an unbound element type. Instance
   // binding will be performed when it's found by name lookup into an instance.
-  auto field_type_inst_id = context.AddInst(SemIR::UnboundElementType{
-      parse_node, context.GetBuiltinType(SemIR::BuiltinKind::TypeType),
-      class_info.self_type_id, base_info.type_id});
+  auto field_type_inst_id = context.AddInst(
+      {parse_node, SemIR::UnboundElementType{
+                       context.GetBuiltinType(SemIR::BuiltinKind::TypeType),
+                       class_info.self_type_id, base_info.type_id}});
   auto field_type_id = context.CanonicalizeType(field_type_inst_id);
-  class_info.base_id = context.AddInst(SemIR::BaseDecl{
-      parse_node, field_type_id, base_info.type_id,
-      SemIR::ElementIndex(
-          context.args_type_info_stack().PeekCurrentBlockContents().size())});
+  class_info.base_id = context.AddInst(
+      {parse_node,
+       SemIR::BaseDecl{field_type_id, base_info.type_id,
+                       SemIR::ElementIndex(context.args_type_info_stack()
+                                               .PeekCurrentBlockContents()
+                                               .size())}});
 
   // Add a corresponding field to the object representation of the class.
   // TODO: Consider whether we want to use `partial T` here.
-  context.args_type_info_stack().AddInst(SemIR::StructTypeField{
-      parse_node, SemIR::NameId::Base, base_info.type_id});
+  context.args_type_info_stack().AddInst(
+      {parse_node,
+       SemIR::StructTypeField{SemIR::NameId::Base, base_info.type_id}});
 
   // Bind the name `base` in the class to the base field.
   context.decl_name_stack().AddNameToLookup(

+ 1 - 1
toolchain/check/handle_expr_statement.cpp

@@ -15,7 +15,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, expr.parse_node(), expr_id,
+  Convert(context, context.insts().GetParseNode(expr_id), expr_id,
           {.kind = ConversionTarget::Discarded, .type_id = expr.type_id()});
 
   // TODO: This will eventually need to do some "do not discard" analysis.

+ 9 - 10
toolchain/check/handle_function.cpp

@@ -33,8 +33,7 @@ auto HandleReturnType(Context& context, Parse::ReturnTypeId parse_node)
   auto type_id = ExprAsType(context, type_parse_node, type_inst_id);
   // TODO: Use a dedicated instruction rather than VarStorage here.
   context.AddInstAndPush(
-      parse_node,
-      SemIR::VarStorage{parse_node, type_id, SemIR::NameId::ReturnSlot});
+      {parse_node, SemIR::VarStorage{type_id, SemIR::NameId::ReturnSlot}});
   return true;
 }
 
@@ -52,12 +51,12 @@ static auto DiagnoseModifiers(Context& context) -> KeywordModifierSet {
     if (inheritance_kind == SemIR::Class::Final) {
       ForbidModifiersOnDecl(context, KeywordModifierSet::Virtual, decl_kind,
                             " in a non-abstract non-base `class` definition",
-                            class_decl->parse_node);
+                            context.GetCurrentScopeParseNode());
     }
     if (inheritance_kind != SemIR::Class::Abstract) {
       ForbidModifiersOnDecl(context, KeywordModifierSet::Abstract, decl_kind,
                             " in a non-abstract `class` definition",
-                            class_decl->parse_node);
+                            context.GetCurrentScopeParseNode());
     }
   } else {
     ForbidModifiersOnDecl(context, KeywordModifierSet::Method, decl_kind,
@@ -137,9 +136,9 @@ static auto BuildFunctionDecl(Context& context,
 
   // Add the function declaration.
   auto function_decl = SemIR::FunctionDecl{
-      parse_node, context.GetBuiltinType(SemIR::BuiltinKind::FunctionType),
+      context.GetBuiltinType(SemIR::BuiltinKind::FunctionType),
       SemIR::FunctionId::Invalid};
-  auto function_decl_id = context.AddInst(function_decl);
+  auto function_decl_id = context.AddInst({parse_node, function_decl});
 
   // Check whether this is a redeclaration.
   auto existing_id =
@@ -225,7 +224,7 @@ auto HandleFunctionDefinitionStart(Context& context,
     context.emitter()
         .Build(parse_node, FunctionRedefinition,
                context.names().GetFormatted(function.name_id).str())
-        .Note(context.insts().Get(function.definition_id).parse_node(),
+        .Note(context.insts().GetParseNode(function.definition_id),
               FunctionPreviousDefinition)
         .Emit();
   } else {
@@ -258,13 +257,13 @@ auto HandleFunctionDefinitionStart(Context& context,
           "Parameter has incomplete type `{0}` in function definition.",
           std::string);
       return context.emitter().Build(
-          param.parse_node(), IncompleteTypeInFunctionParam,
+          context.insts().GetParseNode(param_id), IncompleteTypeInFunctionParam,
           context.sem_ir().StringifyType(param.type_id()));
     });
 
     if (auto fn_param = param.TryAs<SemIR::AnyBindName>()) {
       context.AddNameToLookup(
-          fn_param->parse_node,
+          context.insts().GetParseNode(param_id),
           context.bind_names().Get(fn_param->bind_name_id).name_id, param_id);
     } else {
       CARBON_FATAL() << "Unexpected kind of parameter in function definition "
@@ -290,7 +289,7 @@ auto HandleFunctionDefinition(Context& context,
           "Missing `return` at end of function with declared return type.");
       context.emitter().Emit(TokenOnly(parse_node), MissingReturnStatement);
     } else {
-      context.AddInst(SemIR::Return{parse_node});
+      context.AddInst({parse_node, SemIR::Return{}});
     }
   }
 

+ 1 - 1
toolchain/check/handle_if_statement.cpp

@@ -55,7 +55,7 @@ auto HandleIfStatement(Context& context, Parse::IfStatementId parse_node)
       // block.
       auto else_block_id =
           context.node_stack().Pop<Parse::NodeKind::IfCondition>();
-      context.AddInst(SemIR::Branch{parse_node, else_block_id});
+      context.AddInst({parse_node, SemIR::Branch{else_block_id}});
       context.inst_block_stack().Pop();
       context.inst_block_stack().Push(else_block_id);
       break;

+ 8 - 7
toolchain/check/handle_index.cpp

@@ -47,6 +47,7 @@ auto HandleIndexExpr(Context& context, Parse::IndexExprId parse_node) -> bool {
   switch (operand_type_inst.kind()) {
     case SemIR::ArrayType::Kind: {
       auto array_type = operand_type_inst.As<SemIR::ArrayType>();
+      auto index_parse_node = context.insts().GetParseNode(index_inst_id);
       // We can check whether integers are in-bounds, although it doesn't affect
       // the IR for an array.
       if (auto index_literal = index_inst.TryAs<SemIR::IntLiteral>();
@@ -57,7 +58,7 @@ auto HandleIndexExpr(Context& context, Parse::IndexExprId parse_node) -> bool {
         index_inst_id = SemIR::InstId::BuiltinError;
       }
       auto cast_index_id = ConvertToValueOfType(
-          context, index_inst.parse_node(), index_inst_id,
+          context, index_parse_node, index_inst_id,
           context.GetBuiltinType(SemIR::BuiltinKind::IntType));
       auto array_cat =
           SemIR::GetExprCategory(context.sem_ir(), operand_inst_id);
@@ -65,11 +66,11 @@ auto HandleIndexExpr(Context& context, Parse::IndexExprId parse_node) -> bool {
         // If the operand is an array value, convert it to an ephemeral
         // reference to an array so we can perform a primitive indexing into it.
         operand_inst_id = context.AddInst(
-            SemIR::ValueAsRef{parse_node, operand_type_id, operand_inst_id});
+            {parse_node, SemIR::ValueAsRef{operand_type_id, operand_inst_id}});
       }
       auto elem_id = context.AddInst(
-          SemIR::ArrayIndex{parse_node, array_type.element_type_id,
-                            operand_inst_id, cast_index_id});
+          {parse_node, SemIR::ArrayIndex{array_type.element_type_id,
+                                         operand_inst_id, cast_index_id}});
       if (array_cat != SemIR::ExprCategory::DurableRef) {
         // Indexing a durable reference gives a durable reference expression.
         // Indexing anything else gives a value expression.
@@ -98,9 +99,9 @@ auto HandleIndexExpr(Context& context, Parse::IndexExprId parse_node) -> bool {
         context.emitter().Emit(parse_node, TupleIndexIntLiteral);
         index_inst_id = SemIR::InstId::BuiltinError;
       }
-      context.AddInstAndPush(parse_node,
-                             SemIR::TupleIndex{parse_node, element_type_id,
-                                               operand_inst_id, index_inst_id});
+      context.AddInstAndPush(
+          {parse_node,
+           SemIR::TupleIndex{element_type_id, operand_inst_id, index_inst_id}});
       return true;
     }
     default: {

+ 4 - 4
toolchain/check/handle_interface.cpp

@@ -47,9 +47,9 @@ static auto BuildInterfaceDecl(Context& context,
   auto decl_block_id = context.inst_block_stack().Pop();
 
   // Add the interface declaration.
-  auto interface_decl = SemIR::InterfaceDecl{
-      parse_node, SemIR::InterfaceId::Invalid, decl_block_id};
-  auto interface_decl_id = context.AddInst(interface_decl);
+  auto interface_decl =
+      SemIR::InterfaceDecl{SemIR::InterfaceId::Invalid, decl_block_id};
+  auto interface_decl_id = context.AddInst({parse_node, interface_decl});
 
   // Check whether this is a redeclaration.
   auto existing_id = context.decl_name_stack().LookupOrAddName(
@@ -109,7 +109,7 @@ auto HandleInterfaceDefinitionStart(
     context.emitter()
         .Build(parse_node, InterfaceRedefinition,
                context.names().GetFormatted(interface_info.name_id).str())
-        .Note(context.insts().Get(interface_info.definition_id).parse_node(),
+        .Note(context.insts().GetParseNode(interface_info.definition_id),
               InterfacePreviousDefinition)
         .Emit();
   } else {

+ 2 - 1
toolchain/check/handle_let.cpp

@@ -65,7 +65,8 @@ auto HandleLetDecl(Context& context, Parse::LetDeclId parse_node) -> bool {
 
   // Add the name of the binding to the current scope.
   auto name_id = context.bind_names().Get(bind_name.bind_name_id).name_id;
-  context.AddNameToLookup(pattern.parse_node(), name_id, pattern_id);
+  context.AddNameToLookup(context.insts().GetParseNode(pattern_id), name_id,
+                          pattern_id);
   return true;
 }
 

+ 18 - 23
toolchain/check/handle_literal.cpp

@@ -9,53 +9,48 @@ namespace Carbon::Check {
 auto HandleBoolLiteralFalse(Context& context,
                             Parse::BoolLiteralFalseId parse_node) -> bool {
   context.AddInstAndPush(
-      parse_node,
-      SemIR::BoolLiteral{parse_node,
-                         context.GetBuiltinType(SemIR::BuiltinKind::BoolType),
-                         SemIR::BoolValue::False});
+      {parse_node,
+       SemIR::BoolLiteral{context.GetBuiltinType(SemIR::BuiltinKind::BoolType),
+                          SemIR::BoolValue::False}});
   return true;
 }
 
 auto HandleBoolLiteralTrue(Context& context,
                            Parse::BoolLiteralTrueId parse_node) -> bool {
   context.AddInstAndPush(
-      parse_node,
-      SemIR::BoolLiteral{parse_node,
-                         context.GetBuiltinType(SemIR::BuiltinKind::BoolType),
-                         SemIR::BoolValue::True});
+      {parse_node,
+       SemIR::BoolLiteral{context.GetBuiltinType(SemIR::BuiltinKind::BoolType),
+                          SemIR::BoolValue::True}});
   return true;
 }
 
 auto HandleIntLiteral(Context& context, Parse::IntLiteralId parse_node)
     -> bool {
   context.AddInstAndPush(
-      parse_node,
-      SemIR::IntLiteral{parse_node,
-                        context.GetBuiltinType(SemIR::BuiltinKind::IntType),
-                        context.tokens().GetIntLiteral(
-                            context.parse_tree().node_token(parse_node))});
+      {parse_node,
+       SemIR::IntLiteral{context.GetBuiltinType(SemIR::BuiltinKind::IntType),
+                         context.tokens().GetIntLiteral(
+                             context.parse_tree().node_token(parse_node))}});
   return true;
 }
 
 auto HandleRealLiteral(Context& context, Parse::RealLiteralId parse_node)
     -> bool {
   context.AddInstAndPush(
-      parse_node,
-      SemIR::RealLiteral{parse_node,
-                         context.GetBuiltinType(SemIR::BuiltinKind::FloatType),
-                         context.tokens().GetRealLiteral(
-                             context.parse_tree().node_token(parse_node))});
+      {parse_node,
+       SemIR::RealLiteral{context.GetBuiltinType(SemIR::BuiltinKind::FloatType),
+                          context.tokens().GetRealLiteral(
+                              context.parse_tree().node_token(parse_node))}});
   return true;
 }
 
 auto HandleStringLiteral(Context& context, Parse::StringLiteralId parse_node)
     -> bool {
   context.AddInstAndPush(
-      parse_node,
-      SemIR::StringLiteral{
-          parse_node, context.GetBuiltinType(SemIR::BuiltinKind::StringType),
-          context.tokens().GetStringLiteralValue(
-              context.parse_tree().node_token(parse_node))});
+      {parse_node, SemIR::StringLiteral{
+                       context.GetBuiltinType(SemIR::BuiltinKind::StringType),
+                       context.tokens().GetStringLiteralValue(
+                           context.parse_tree().node_token(parse_node))}});
   return true;
 }
 

+ 3 - 3
toolchain/check/handle_loop_statement.cpp

@@ -59,7 +59,7 @@ auto HandleWhileStatement(Context& context, Parse::WhileStatementId parse_node)
   context.break_continue_stack().pop_back();
 
   // Add the loop backedge.
-  context.AddInst(SemIR::Branch{parse_node, loop_header_id});
+  context.AddInst({parse_node, SemIR::Branch{loop_header_id}});
   context.inst_block_stack().Pop();
 
   // Start emitting the loop exit block.
@@ -102,7 +102,7 @@ auto HandleBreakStatementStart(Context& context,
                       "`break` can only be used in a loop.");
     context.emitter().Emit(parse_node, BreakOutsideLoop);
   } else {
-    context.AddInst(SemIR::Branch{parse_node, stack.back().break_target});
+    context.AddInst({parse_node, SemIR::Branch{stack.back().break_target}});
   }
 
   context.inst_block_stack().Pop();
@@ -127,7 +127,7 @@ auto HandleContinueStatementStart(Context& context,
                       "`continue` can only be used in a loop.");
     context.emitter().Emit(parse_node, ContinueOutsideLoop);
   } else {
-    context.AddInst(SemIR::Branch{parse_node, stack.back().continue_target});
+    context.AddInst({parse_node, SemIR::Branch{stack.back().continue_target}});
   }
 
   context.inst_block_stack().Pop();

+ 20 - 22
toolchain/check/handle_name.cpp

@@ -26,7 +26,7 @@ static auto GetAsNameScope(Context& context, SemIR::InstId base_id)
                         "Member access into incomplete class `{0}`.",
                         std::string);
       auto builder =
-          context.emitter().Build(context.insts().Get(base_id).parse_node(),
+          context.emitter().Build(context.insts().GetParseNode(base_id),
                                   QualifiedExprInIncompleteClassScope,
                                   context.sem_ir().StringifyTypeExpr(base_id));
       context.NoteIncompleteClass(base_as_class->class_id, builder);
@@ -108,8 +108,7 @@ auto HandleMemberAccessExpr(Context& context,
     auto inst = context.insts().Get(inst_id);
     // TODO: Track that this instruction was named within `base_id`.
     context.AddInstAndPush(
-        parse_node,
-        SemIR::NameRef{parse_node, inst.type_id(), name_id, inst_id});
+        {parse_node, SemIR::NameRef{inst.type_id(), name_id, inst_id}});
     return true;
   }
 
@@ -120,8 +119,7 @@ auto HandleMemberAccessExpr(Context& context,
                           "Member access into object of incomplete type `{0}`.",
                           std::string);
         return context.emitter().Build(
-            context.insts().Get(base_id).parse_node(),
-            IncompleteTypeInMemberAccess,
+            context.insts().GetParseNode(base_id), IncompleteTypeInMemberAccess,
             context.sem_ir().StringifyType(base_type_id));
       })) {
     context.node_stack().Push(parse_node, SemIR::InstId::BuiltinError);
@@ -159,8 +157,10 @@ auto HandleMemberAccessExpr(Context& context,
             << "Non-constant value " << context.insts().Get(member_id)
             << " of unbound element type";
         auto index = GetClassElementIndex(context, element_id.inst_id());
-        auto access_id = context.AddInst(SemIR::ClassElementAccess{
-            parse_node, unbound_element_type->element_type_id, base_id, index});
+        auto access_id = context.AddInst(
+            {parse_node,
+             SemIR::ClassElementAccess{unbound_element_type->element_type_id,
+                                       base_id, index}});
         if (SemIR::GetExprCategory(context.sem_ir(), base_id) ==
                 SemIR::ExprCategory::Value &&
             SemIR::GetExprCategory(context.sem_ir(), access_id) !=
@@ -191,11 +191,10 @@ auto HandleMemberAccessExpr(Context& context,
             << " of function type";
         if (IsInstanceMethod(context.sem_ir(), function_decl->function_id)) {
           context.AddInstAndPush(
-              parse_node,
-              SemIR::BoundMethod{
-                  parse_node,
-                  context.GetBuiltinType(SemIR::BuiltinKind::BoundMethodType),
-                  base_id, member_id});
+              {parse_node,
+               SemIR::BoundMethod{
+                   context.GetBuiltinType(SemIR::BuiltinKind::BoundMethodType),
+                   base_id, member_id}});
           return true;
         }
       }
@@ -203,8 +202,7 @@ auto HandleMemberAccessExpr(Context& context,
       // For a non-instance member, the result is that member.
       // TODO: Track that this was named within `base_id`.
       context.AddInstAndPush(
-          parse_node,
-          SemIR::NameRef{parse_node, member_type_id, name_id, member_id});
+          {parse_node, SemIR::NameRef{member_type_id, name_id, member_id}});
       return true;
     }
     case SemIR::StructType::Kind: {
@@ -215,8 +213,8 @@ auto HandleMemberAccessExpr(Context& context,
         auto field = context.insts().GetAs<SemIR::StructTypeField>(ref_id);
         if (name_id == field.name_id) {
           context.AddInstAndPush(
-              parse_node, SemIR::StructAccess{parse_node, field.field_type_id,
-                                              base_id, SemIR::ElementIndex(i)});
+              {parse_node, SemIR::StructAccess{field.field_type_id, base_id,
+                                               SemIR::ElementIndex(i)}});
           return true;
         }
       }
@@ -274,8 +272,8 @@ static auto HandleNameAsExpr(Context& context, Parse::NodeId parse_node,
     return context.TODO(parse_node, "Unimplemented use of interface");
   }
   auto value = context.insts().Get(value_id);
-  context.AddInstAndPush(parse_node, SemIR::NameRef{parse_node, value.type_id(),
-                                                    name_id, value_id});
+  context.AddInstAndPush(
+      {parse_node, SemIR::NameRef{value.type_id(), name_id, value_id}});
   return true;
 }
 
@@ -355,10 +353,10 @@ auto HandleQualifiedName(Context& context, Parse::QualifiedNameId parse_node)
 auto HandlePackageExpr(Context& context, Parse::PackageExprId parse_node)
     -> bool {
   context.AddInstAndPush(
-      parse_node,
-      SemIR::NameRef{
-          parse_node, context.GetBuiltinType(SemIR::BuiltinKind::NamespaceType),
-          SemIR::NameId::PackageNamespace, SemIR::InstId::PackageNamespace});
+      {parse_node,
+       SemIR::NameRef{context.GetBuiltinType(SemIR::BuiltinKind::NamespaceType),
+                      SemIR::NameId::PackageNamespace,
+                      SemIR::InstId::PackageNamespace}});
   return true;
 }
 

+ 2 - 2
toolchain/check/handle_namespace.cpp

@@ -23,9 +23,9 @@ auto HandleNamespace(Context& context, Parse::NamespaceId parse_node) -> bool {
   LimitModifiersOnDecl(context, KeywordModifierSet::None,
                        Lex::TokenKind::Namespace);
   auto namespace_inst = SemIR::Namespace{
-      parse_node, context.GetBuiltinType(SemIR::BuiltinKind::NamespaceType),
+      context.GetBuiltinType(SemIR::BuiltinKind::NamespaceType),
       name_context.name_id_for_new_inst(), SemIR::NameScopeId::Invalid};
-  auto namespace_id = context.AddInst(namespace_inst);
+  auto namespace_id = context.AddInst({parse_node, namespace_inst});
   namespace_inst.name_scope_id = context.name_scopes().Add(
       namespace_id, name_context.enclosing_scope_id_for_new_inst());
   context.insts().Set(namespace_id, namespace_inst);

+ 20 - 24
toolchain/check/handle_operator.cpp

@@ -57,7 +57,7 @@ auto HandleInfixOperatorEqual(Context& context,
   // TODO: Destroy the old value before reinitializing. This will require
   // building the destruction code before we build the RHS subexpression.
   rhs_id = Initialize(context, parse_node, lhs_id, rhs_id);
-  context.AddInst(SemIR::Assign{parse_node, lhs_id, rhs_id});
+  context.AddInst({parse_node, SemIR::Assign{lhs_id, rhs_id}});
   // We model assignment as an expression, so we need to push a value for
   // it, even though it doesn't produce a value.
   // TODO: Consider changing our parse tree to model assignment as a
@@ -199,8 +199,7 @@ auto HandlePostfixOperatorStar(Context& context,
   auto value_id = context.node_stack().PopExpr();
   auto inner_type_id = ExprAsType(context, parse_node, value_id);
   context.AddInstAndPush(
-      parse_node,
-      SemIR::PointerType{parse_node, SemIR::TypeId::TypeType, inner_type_id});
+      {parse_node, SemIR::PointerType{SemIR::TypeId::TypeType, inner_type_id}});
   return true;
 }
 
@@ -224,11 +223,10 @@ auto HandlePrefixOperatorAmp(Context& context,
       break;
   }
   context.AddInstAndPush(
-      parse_node,
-      SemIR::AddrOf{parse_node,
-                    context.GetPointerType(
-                        parse_node, context.insts().Get(value_id).type_id()),
-                    value_id});
+      {parse_node,
+       SemIR::AddrOf{context.GetPointerType(
+                         parse_node, context.insts().Get(value_id).type_id()),
+                     value_id}});
   return true;
 }
 
@@ -254,8 +252,7 @@ auto HandlePrefixOperatorConst(Context& context,
   }
   auto inner_type_id = ExprAsType(context, parse_node, value_id);
   context.AddInstAndPush(
-      parse_node,
-      SemIR::ConstType{parse_node, SemIR::TypeId::TypeType, inner_type_id});
+      {parse_node, SemIR::ConstType{SemIR::TypeId::TypeType, inner_type_id}});
   return true;
 }
 
@@ -275,9 +272,8 @@ auto HandlePrefixOperatorNot(Context& context,
   auto value_id = context.node_stack().PopExpr();
   value_id = ConvertToBoolValue(context, parse_node, value_id);
   context.AddInstAndPush(
-      parse_node,
-      SemIR::UnaryOperatorNot{
-          parse_node, context.insts().Get(value_id).type_id(), value_id});
+      {parse_node, SemIR::UnaryOperatorNot{
+                       context.insts().Get(value_id).type_id(), value_id}});
   return true;
 }
 
@@ -313,8 +309,7 @@ auto HandlePrefixOperatorStar(Context& context,
     }
     builder.Emit();
   }
-  context.AddInstAndPush(parse_node,
-                         SemIR::Deref{parse_node, result_type_id, value_id});
+  context.AddInstAndPush({parse_node, SemIR::Deref{result_type_id, value_id}});
   return true;
 }
 
@@ -329,12 +324,14 @@ static auto HandleShortCircuitOperand(Context& context,
 
   // Compute the branch value: the condition for `and`, inverted for `or`.
   SemIR::InstId branch_value_id =
-      is_or ? context.AddInst(SemIR::UnaryOperatorNot{parse_node, bool_type_id,
-                                                      cond_value_id})
+      is_or ? context.AddInst(
+                  {parse_node,
+                   SemIR::UnaryOperatorNot{bool_type_id, cond_value_id}})
             : cond_value_id;
-  auto short_circuit_result_id = context.AddInst(SemIR::BoolLiteral{
-      parse_node, bool_type_id,
-      is_or ? SemIR::BoolValue::True : SemIR::BoolValue::False});
+  auto short_circuit_result_id = context.AddInst(
+      {parse_node,
+       SemIR::BoolLiteral{bool_type_id, is_or ? SemIR::BoolValue::True
+                                              : SemIR::BoolValue::False}});
 
   // Create a block for the right-hand side and for the continuation.
   auto rhs_block_id =
@@ -380,15 +377,14 @@ static auto HandleShortCircuitOperator(Context& context,
   // When the second operand is evaluated, the result of `and` and `or` is
   // its value.
   auto resume_block_id = context.inst_block_stack().PeekOrAdd(/*depth=*/1);
-  context.AddInst(SemIR::BranchWithArg{parse_node, resume_block_id, rhs_id});
+  context.AddInst({parse_node, SemIR::BranchWithArg{resume_block_id, rhs_id}});
   context.inst_block_stack().Pop();
   context.AddCurrentCodeBlockToFunction();
 
   // Collect the result from either the first or second operand.
   context.AddInstAndPush(
-      parse_node,
-      SemIR::BlockArg{parse_node, context.insts().Get(rhs_id).type_id(),
-                      resume_block_id});
+      {parse_node, SemIR::BlockArg{context.insts().Get(rhs_id).type_id(),
+                                   resume_block_id}});
   return true;
 }
 

+ 1 - 1
toolchain/check/handle_paren.cpp

@@ -46,7 +46,7 @@ auto HandleTupleLiteral(Context& context, Parse::TupleLiteralId parse_node)
   auto type_id = context.CanonicalizeTupleType(parse_node, type_ids);
 
   auto value_id =
-      context.AddInst(SemIR::TupleLiteral{parse_node, type_id, refs_id});
+      context.AddInst({parse_node, SemIR::TupleLiteral{type_id, refs_id}});
   context.node_stack().Push(parse_node, value_id);
   return true;
 }

+ 13 - 12
toolchain/check/handle_struct.cpp

@@ -40,8 +40,9 @@ auto HandleStructFieldValue(Context& context,
   auto [name_node, name_id] = context.node_stack().PopNameWithParseNode();
 
   // Store the name for the type.
-  context.args_type_info_stack().AddInst(SemIR::StructTypeField{
-      name_node, name_id, context.insts().Get(value_inst_id).type_id()});
+  context.args_type_info_stack().AddInst(
+      {name_node, SemIR::StructTypeField{
+                      name_id, context.insts().Get(value_inst_id).type_id()}});
 
   // Push the value back on the stack as an argument.
   context.node_stack().Push(parse_node, value_inst_id);
@@ -55,8 +56,9 @@ auto HandleStructFieldType(Context& context,
 
   auto [name_node, name_id] = context.node_stack().PopNameWithParseNode();
 
-  context.AddInstAndPush(
-      parse_node, SemIR::StructTypeField{name_node, name_id, cast_type_id});
+  auto inst_id = context.AddInst(
+      {name_node, SemIR::StructTypeField{name_id, cast_type_id}});
+  context.node_stack().Push(parse_node, inst_id);
   return true;
 }
 
@@ -65,12 +67,11 @@ static auto DiagnoseDuplicateNames(Context& context,
                                    llvm::StringRef construct) -> bool {
   auto& sem_ir = context.sem_ir();
   auto fields = sem_ir.inst_blocks().Get(type_block_id);
-  llvm::SmallDenseMap<SemIR::NameId, Parse::NodeId> names;
+  llvm::SmallDenseMap<SemIR::NameId, SemIR::InstId> names;
   auto& insts = sem_ir.insts();
   for (SemIR::InstId field_inst_id : fields) {
     auto field_inst = insts.GetAs<SemIR::StructTypeField>(field_inst_id);
-    auto [it, added] =
-        names.insert({field_inst.name_id, field_inst.parse_node});
+    auto [it, added] = names.insert({field_inst.name_id, field_inst_id});
     if (!added) {
       CARBON_DIAGNOSTIC(StructNameDuplicate, Error,
                         "Duplicated field name `{1}` in {0}.", std::string,
@@ -78,9 +79,10 @@ static auto DiagnoseDuplicateNames(Context& context,
       CARBON_DIAGNOSTIC(StructNamePrevious, Note,
                         "Field with the same name here.");
       context.emitter()
-          .Build(field_inst.parse_node, StructNameDuplicate, construct.str(),
+          .Build(context.insts().GetParseNode(field_inst_id),
+                 StructNameDuplicate, construct.str(),
                  sem_ir.names().GetFormatted(field_inst.name_id).str())
-          .Note(it->second, StructNamePrevious)
+          .Note(context.insts().GetParseNode(it->second), StructNamePrevious)
           .Emit();
       return true;
     }
@@ -106,7 +108,7 @@ auto HandleStructLiteral(Context& context, Parse::StructLiteralId parse_node)
   auto type_id = context.CanonicalizeStructType(parse_node, type_block_id);
 
   auto value_id =
-      context.AddInst(SemIR::StructLiteral{parse_node, type_id, refs_id});
+      context.AddInst({parse_node, SemIR::StructLiteral{type_id, refs_id}});
   context.node_stack().Push(parse_node, value_id);
   return true;
 }
@@ -131,8 +133,7 @@ auto HandleStructTypeLiteral(Context& context,
     return true;
   }
   context.AddInstAndPush(
-      parse_node,
-      SemIR::StructType{parse_node, SemIR::TypeId::TypeType, refs_id});
+      {parse_node, SemIR::StructType{SemIR::TypeId::TypeType, refs_id}});
   return true;
 }
 

+ 3 - 4
toolchain/check/handle_variable.cpp

@@ -44,12 +44,11 @@ auto HandleVariableDecl(Context& context, Parse::VariableDeclId parse_node)
 
   // Extract the name binding.
   auto value_id = context.node_stack().PopPattern();
-  if (auto bind_name =
-          context.insts().Get(value_id).TryAs<SemIR::AnyBindName>()) {
+  if (auto bind_name = context.insts().TryGetAs<SemIR::AnyBindName>(value_id)) {
     // Form a corresponding name in the current context, and bind the name to
     // the variable.
     auto name_context = context.decl_name_stack().MakeUnqualifiedName(
-        bind_name->parse_node,
+        context.insts().GetParseNode(value_id),
         context.bind_names().Get(bind_name->bind_name_id).name_id);
     context.decl_name_stack().AddNameToLookup(name_context, value_id);
     value_id = bind_name->value_id;
@@ -70,7 +69,7 @@ auto HandleVariableDecl(Context& context, Parse::VariableDeclId parse_node)
       init_id = Initialize(context, parse_node, value_id, *init_id);
       // TODO: Consider using different instruction kinds for assignment versus
       // initialization.
-      context.AddInst(SemIR::Assign{parse_node, value_id, *init_id});
+      context.AddInst({parse_node, SemIR::Assign{value_id, *init_id}});
     }
   }
 

+ 5 - 4
toolchain/check/import.cpp

@@ -69,9 +69,9 @@ static auto AddNamespace(Context& context,
                          SemIR::NameId name_id, SemIR::TypeId namespace_type_id)
     -> std::pair<SemIR::InstId, SemIR::NameScopeId> {
   // Use the invalid node because there's no node to associate with.
-  auto inst = SemIR::Namespace{Parse::NodeId::Invalid, namespace_type_id,
-                               name_id, SemIR::NameScopeId::Invalid};
-  auto id = context.AddInst(inst);
+  auto inst =
+      SemIR::Namespace{namespace_type_id, name_id, SemIR::NameScopeId::Invalid};
+  auto id = context.AddInst({Parse::NodeId::Invalid, inst});
   inst.name_scope_id = context.name_scopes().Add(id, enclosing_scope_id);
   context.insts().Set(id, inst);
   return {id, inst.name_scope_id};
@@ -195,7 +195,8 @@ auto Import(Context& context, SemIR::TypeId namespace_type_id,
     } else {
       // Leave a placeholder that the inst comes from the other IR.
       auto target_id = context.AddInst(
-          SemIR::LazyImportRef{.ir_id = ir_id, .inst_id = import_inst_id});
+          {Parse::NodeId::Invalid,
+           SemIR::LazyImportRef{.ir_id = ir_id, .inst_id = import_inst_id}});
       // TODO: When importing from other packages, the scope's names should
       // be changed to allow for ambiguous names. When importing from the
       // current package, as is currently being done, we should issue a

+ 3 - 2
toolchain/check/inst_block_stack.h

@@ -8,6 +8,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "toolchain/sem_ir/file.h"
 #include "toolchain/sem_ir/inst.h"
+#include "toolchain/sem_ir/value_stores.h"
 
 namespace Carbon::Check {
 
@@ -49,8 +50,8 @@ class InstBlockStack {
 
   // Adds the given instruction to the block at the top of the stack and returns
   // its ID.
-  auto AddInst(SemIR::Inst inst) -> SemIR::InstId {
-    auto inst_id = sem_ir_->insts().AddInNoBlock(inst);
+  auto AddInst(SemIR::ParseNodeAndInst parse_node_and_inst) -> SemIR::InstId {
+    auto inst_id = sem_ir_->insts().AddInNoBlock(parse_node_and_inst);
     AddInstId(inst_id);
     return inst_id;
   }

+ 7 - 4
toolchain/check/pending_block.h

@@ -7,6 +7,7 @@
 
 #include "llvm/ADT/SmallVector.h"
 #include "toolchain/check/context.h"
+#include "toolchain/sem_ir/value_stores.h"
 
 namespace Carbon::Check {
 
@@ -39,8 +40,8 @@ class PendingBlock {
     size_t size_;
   };
 
-  auto AddInst(SemIR::Inst inst) -> SemIR::InstId {
-    auto inst_id = context_.insts().AddInNoBlock(inst);
+  auto AddInst(SemIR::ParseNodeAndInst parse_node_and_inst) -> SemIR::InstId {
+    auto inst_id = context_.insts().AddInNoBlock(parse_node_and_inst);
     insts_.push_back(inst_id);
     return inst_id;
   }
@@ -64,7 +65,7 @@ class PendingBlock {
       // 1) The block is empty. Replace `target_id` with an empty splice
       // pointing at `value_id`.
       context_.insts().Set(
-          target_id, SemIR::SpliceBlock{value.parse_node(), value.type_id(),
+          target_id, SemIR::SpliceBlock{value.type_id(),
                                         SemIR::InstBlockId::Empty, value_id});
     } else if (insts_.size() == 1 && insts_[0] == value_id) {
       // 2) The block is {value_id}. Replace `target_id` with the instruction
@@ -74,9 +75,11 @@ class PendingBlock {
       // 3) Anything else: splice it into the IR, replacing `target_id`.
       context_.insts().Set(
           target_id,
-          SemIR::SpliceBlock{value.parse_node(), value.type_id(),
+          SemIR::SpliceBlock{value.type_id(),
                              context_.inst_blocks().Add(insts_), value_id});
     }
+    context_.insts().SetParseNode(target_id,
+                                  context_.insts().GetParseNode(value_id));
 
     // Prepare to stash more pending instructions.
     insts_.clear();

+ 9 - 9
toolchain/check/return.cpp

@@ -34,7 +34,7 @@ static auto NoteNoReturnTypeProvided(Context& context,
                                      const SemIR::Function& function) {
   CARBON_DIAGNOSTIC(ReturnTypeOmittedNote, Note,
                     "There was no return type provided.");
-  diag.Note(context.insts().Get(function.decl_id).parse_node(),
+  diag.Note(context.insts().GetParseNode(function.decl_id),
             ReturnTypeOmittedNote);
 }
 
@@ -43,7 +43,7 @@ static auto NoteReturnType(Context& context, Context::DiagnosticBuilder& diag,
                            const SemIR::Function& function) {
   // TODO: This is the location of the `fn` keyword. Find the location of the
   // return type.
-  auto type_parse_node = context.insts().Get(function.decl_id).parse_node();
+  auto type_parse_node = context.insts().GetParseNode(function.decl_id);
   CARBON_DIAGNOSTIC(ReturnTypeHereNote, Note,
                     "Return type of function is `{0}`.", std::string);
   diag.Note(type_parse_node, ReturnTypeHereNote,
@@ -54,7 +54,7 @@ static auto NoteReturnType(Context& context, Context::DiagnosticBuilder& diag,
 static auto NoteReturnedVar(Context& context, Context::DiagnosticBuilder& diag,
                             SemIR::InstId returned_var_id) {
   CARBON_DIAGNOSTIC(ReturnedVarHere, Note, "`returned var` was declared here.");
-  diag.Note(context.insts().Get(returned_var_id).parse_node(), ReturnedVarHere);
+  diag.Note(context.insts().GetParseNode(returned_var_id), ReturnedVarHere);
 }
 
 auto CheckReturnedVar(Context& context, Parse::NodeId returned_node,
@@ -92,7 +92,7 @@ auto CheckReturnedVar(Context& context, Parse::NodeId returned_node,
   if (function.return_slot_id.is_valid()) {
     return function.return_slot_id;
   }
-  return context.AddInst(SemIR::VarStorage{name_node, type_id, name_id});
+  return context.AddInst({name_node, SemIR::VarStorage{type_id, name_id}});
 }
 
 auto RegisterReturnedVar(Context& context, SemIR::InstId bind_id) -> void {
@@ -101,8 +101,8 @@ auto RegisterReturnedVar(Context& context, SemIR::InstId bind_id) -> void {
     CARBON_DIAGNOSTIC(ReturnedVarShadowed, Error,
                       "Cannot declare a `returned var` in the scope of "
                       "another `returned var`.");
-    auto diag = context.emitter().Build(
-        context.insts().Get(bind_id).parse_node(), ReturnedVarShadowed);
+    auto diag = context.emitter().Build(context.insts().GetParseNode(bind_id),
+                                        ReturnedVarShadowed);
     NoteReturnedVar(context, diag, existing_id);
     diag.Emit();
   }
@@ -120,7 +120,7 @@ auto BuildReturnWithNoExpr(Context& context,
     diag.Emit();
   }
 
-  context.AddInst(SemIR::Return{parse_node});
+  context.AddInst({parse_node, SemIR::Return{}});
 }
 
 auto BuildReturnWithExpr(Context& context, Parse::ReturnStatementId parse_node,
@@ -152,7 +152,7 @@ auto BuildReturnWithExpr(Context& context, Parse::ReturnStatementId parse_node,
                                    function.return_type_id);
   }
 
-  context.AddInst(SemIR::ReturnExpr{parse_node, expr_id});
+  context.AddInst({parse_node, SemIR::ReturnExpr{expr_id}});
 }
 
 auto BuildReturnVar(Context& context, Parse::ReturnStatementId parse_node)
@@ -173,7 +173,7 @@ auto BuildReturnVar(Context& context, Parse::ReturnStatementId parse_node)
     returned_var_id = ConvertToValueExpr(context, returned_var_id);
   }
 
-  context.AddInst(SemIR::ReturnExpr{parse_node, returned_var_id});
+  context.AddInst({parse_node, SemIR::ReturnExpr{returned_var_id}});
 }
 
 }  // namespace Carbon::Check

+ 1 - 0
toolchain/sem_ir/BUILD

@@ -123,6 +123,7 @@ cc_library(
         "//toolchain/base:value_store",
         "//toolchain/base:yaml",
         "//toolchain/lex:token_kind",
+        "//toolchain/parse:node_kind",
         "@llvm-project//llvm:Support",
     ],
 )

+ 9 - 6
toolchain/sem_ir/file.cpp

@@ -74,11 +74,12 @@ File::File(SharedValueStores& value_stores)
   // Error uses a self-referential type so that it's not accidentally treated as
   // a normal type. Every other builtin is a type, including the
   // self-referential TypeType.
-#define CARBON_SEM_IR_BUILTIN_KIND(Name, ...)                         \
-  insts_.AddInNoBlock(Builtin{BuiltinKind::Name == BuiltinKind::Error \
-                                  ? TypeId::Error                     \
-                                  : TypeId::TypeType,                 \
-                              BuiltinKind::Name});
+#define CARBON_SEM_IR_BUILTIN_KIND(Name, ...)                              \
+  insts_.AddInNoBlock(                                                     \
+      {Parse::NodeId::Invalid,                                             \
+       Builtin{BuiltinKind::Name == BuiltinKind::Error ? TypeId::Error     \
+                                                       : TypeId::TypeType, \
+               BuiltinKind::Name}});
 #include "toolchain/sem_ir/builtin_kind.def"
 
   CARBON_CHECK(insts_.size() == BuiltinKind::ValidCount)
@@ -102,7 +103,9 @@ File::File(SharedValueStores& value_stores, std::string filename,
   static constexpr auto BuiltinIR = CrossRefIRId(0);
   for (auto [i, inst] : llvm::enumerate(builtins->insts_.array_ref())) {
     // We can reuse builtin type IDs because they're special-cased values.
-    insts_.AddInNoBlock(CrossRef{inst.type_id(), BuiltinIR, SemIR::InstId(i)});
+    insts_.AddInNoBlock(
+        {Parse::NodeId::Invalid,
+         CrossRef{inst.type_id(), BuiltinIR, SemIR::InstId(i)}});
   }
 }
 

+ 13 - 11
toolchain/sem_ir/formatter.cpp

@@ -69,10 +69,9 @@ class InstNamer {
       CollectNamesInBlock(fn_scope, fn.param_refs_id);
       if (fn.return_slot_id.is_valid()) {
         insts[fn.return_slot_id.index] = {
-            fn_scope,
-            GetScopeInfo(fn_scope).insts.AllocateName(
-                *this, sem_ir.insts().Get(fn.return_slot_id).parse_node(),
-                "return")};
+            fn_scope, GetScopeInfo(fn_scope).insts.AllocateName(
+                          *this, sem_ir.insts().GetParseNode(fn.return_slot_id),
+                          "return")};
       }
       if (!fn.body_block_ids.empty()) {
         AddBlockLabel(fn_scope, fn.body_block_ids.front(), "entry", fn_loc);
@@ -341,7 +340,7 @@ class InstNamer {
     if (!parse_node.is_valid()) {
       if (const auto& block = sem_ir_.inst_blocks().Get(block_id);
           !block.empty()) {
-        parse_node = sem_ir_.insts().Get(block.front()).parse_node();
+        parse_node = sem_ir_.insts().GetParseNode(block.front());
       }
     }
 
@@ -352,9 +351,10 @@ class InstNamer {
 
   // Finds and adds a suitable block label for the given SemIR instruction that
   // represents some kind of branch.
-  auto AddBlockLabel(ScopeIndex scope_idx, AnyBranch branch) -> void {
+  auto AddBlockLabel(ScopeIndex scope_idx, Parse::NodeId parse_node,
+                     AnyBranch branch) -> void {
     llvm::StringRef name;
-    switch (parse_tree_.node_kind(branch.parse_node)) {
+    switch (parse_tree_.node_kind(parse_node)) {
       case Parse::NodeKind::IfExprIf:
         switch (branch.kind) {
           case BranchIf::Kind:
@@ -416,7 +416,7 @@ class InstNamer {
         break;
     }
 
-    AddBlockLabel(scope_idx, branch.target_id, name.str(), branch.parse_node);
+    AddBlockLabel(scope_idx, branch.target_id, name.str(), parse_node);
   }
 
   auto CollectNamesInBlock(ScopeIndex scope_idx, InstBlockId block_id) -> void {
@@ -437,8 +437,9 @@ class InstNamer {
 
       auto inst = sem_ir_.insts().Get(inst_id);
       auto add_inst_name = [&](std::string name) {
-        insts[inst_id.index] = {scope_idx, scope.insts.AllocateName(
-                                               *this, inst.parse_node(), name)};
+        insts[inst_id.index] = {
+            scope_idx, scope.insts.AllocateName(
+                           *this, sem_ir_.insts().GetParseNode(inst_id), name)};
       };
       auto add_inst_name_id = [&](NameId name_id, llvm::StringRef suffix = "") {
         add_inst_name(
@@ -446,7 +447,8 @@ class InstNamer {
       };
 
       if (auto branch = inst.TryAs<AnyBranch>()) {
-        AddBlockLabel(scope_idx, *branch);
+        AddBlockLabel(scope_idx, sem_ir_.insts().GetParseNode(inst_id),
+                      *branch);
       }
 
       switch (inst.kind()) {

+ 18 - 35
toolchain/sem_ir/inst.h

@@ -37,9 +37,8 @@ struct InstLikeTypeInfoBase {
   using Tuple =
       decltype(StructReflection::AsTuple(std::declval<InstLikeType>()));
 
-  static constexpr int FirstArgField = HasKindMemberAsField<InstLikeType> +
-                                       HasParseNodeMember<InstLikeType> +
-                                       HasTypeIdMember<InstLikeType>;
+  static constexpr int FirstArgField =
+      HasKindMemberAsField<InstLikeType> + HasTypeIdMember<InstLikeType>;
 
   static constexpr int NumArgs = std::tuple_size_v<Tuple> - FirstArgField;
   static_assert(NumArgs <= 2,
@@ -57,8 +56,11 @@ struct InstLikeTypeInfoBase {
 // A particular type of instruction is instruction-like.
 template <typename TypedInst>
 struct InstLikeTypeInfo<
-    TypedInst, static_cast<bool>(std::is_same_v<const InstKind::Definition,
-                                                decltype(TypedInst::Kind)>)>
+    TypedInst,
+    static_cast<bool>(
+        std::is_same_v<const InstKind::Definition<
+                           typename decltype(TypedInst::Kind)::TypedNodeId>,
+                       decltype(TypedInst::Kind)>)>
     : InstLikeTypeInfoBase<TypedInst> {
   static_assert(!HasKindMemberAsField<TypedInst>,
                 "Instruction type should not have a kind field");
@@ -106,7 +108,6 @@ struct InstLikeTypeInfo<
 // provides access to common fields present on most or all kinds of
 // instructions:
 //
-// - `parse_node` for error placement.
 // - `kind` for run-time logic when the input Kind is unknown.
 // - `type_id` for quick type checking.
 //
@@ -127,15 +128,11 @@ class Inst : public Printable<Inst> {
             typename Info = typename InstLikeTypeInfo<TypedInst>::Self>
   // NOLINTNEXTLINE(google-explicit-constructor)
   Inst(TypedInst typed_inst)
-      : parse_node_(Parse::NodeId::Invalid),
-        // Always overwritten below.
-        kind_(InstKind::Create({})),
+      // kind_ is always overwritten below.
+      : kind_(InstKind::Create({})),
         type_id_(TypeId::Invalid),
         arg0_(InstId::InvalidIndex),
         arg1_(InstId::InvalidIndex) {
-    if constexpr (HasParseNodeMember<TypedInst>) {
-      parse_node_ = typed_inst.parse_node;
-    }
     if constexpr (HasKindMemberAsField<TypedInst>) {
       kind_ = typed_inst.kind;
     } else {
@@ -164,20 +161,11 @@ class Inst : public Printable<Inst> {
   auto As() const -> TypedInst {
     CARBON_CHECK(Is<TypedInst>()) << "Casting inst of kind " << kind()
                                   << " to wrong kind " << Info::DebugName();
-    auto build_with_parse_node_onwards = [&](auto... parse_node_onwards) {
-      if constexpr (HasKindMemberAsField<TypedInst>) {
-        return TypedInst{kind(), parse_node_onwards...};
-      } else {
-        return TypedInst{parse_node_onwards...};
-      }
-    };
-
     auto build_with_type_id_onwards = [&](auto... type_id_onwards) {
-      if constexpr (HasParseNodeMember<TypedInst>) {
-        return build_with_parse_node_onwards(
-            decltype(TypedInst::parse_node)(parse_node()), type_id_onwards...);
+      if constexpr (HasKindMemberAsField<TypedInst>) {
+        return TypedInst{kind(), type_id_onwards...};
       } else {
-        return build_with_parse_node_onwards(type_id_onwards...);
+        return TypedInst{type_id_onwards...};
       }
     };
 
@@ -212,7 +200,6 @@ class Inst : public Printable<Inst> {
     }
   }
 
-  auto parse_node() const -> Parse::NodeId { return parse_node_; }
   auto kind() const -> InstKind { return kind_; }
 
   // Gets the type of the value produced by evaluating this instruction.
@@ -224,13 +211,8 @@ class Inst : public Printable<Inst> {
   friend class InstTestHelper;
 
   // Raw constructor, used for testing.
-  explicit Inst(InstKind kind, Parse::NodeId parse_node, TypeId type_id,
-                int32_t arg0, int32_t arg1)
-      : parse_node_(parse_node),
-        kind_(kind),
-        type_id_(type_id),
-        arg0_(arg0),
-        arg1_(arg1) {}
+  explicit Inst(InstKind kind, TypeId type_id, int32_t arg0, int32_t arg1)
+      : kind_(kind), type_id_(type_id), arg0_(arg0), arg1_(arg1) {}
 
   // Convert a field to its raw representation, used as `arg0_` / `arg1_`.
   static constexpr auto ToRaw(IdBase base) -> int32_t { return base.index; }
@@ -248,7 +230,6 @@ class Inst : public Printable<Inst> {
     return BuiltinKind::FromInt(raw);
   }
 
-  Parse::NodeId parse_node_;
   InstKind kind_;
   TypeId type_id_;
 
@@ -257,10 +238,12 @@ class Inst : public Printable<Inst> {
   int32_t arg1_;
 };
 
-// TODO: This is currently 20 bytes because we sometimes have 2 arguments for a
+// TODO: This is currently 16 bytes because we sometimes have 2 arguments for a
 // pair of Insts. However, InstKind is 1 byte; if args were 3.5 bytes, we could
 // potentially shrink Inst by 4 bytes. This may be worth investigating further.
-static_assert(sizeof(Inst) == 20, "Unexpected Inst size");
+// Note though that 16 bytes is an ideal size for registers, we may want more
+// flags, and 12 bytes would be a more marginal improvement.
+static_assert(sizeof(Inst) == 16, "Unexpected Inst size");
 
 // Instruction-like types can be printed by converting them to instructions.
 template <typename TypedInst,

+ 8 - 8
toolchain/sem_ir/inst_kind.cpp

@@ -14,7 +14,11 @@ CARBON_DEFINE_ENUM_CLASS_NAMES(InstKind) = {
 };
 
 auto InstKind::ir_name() const -> llvm::StringLiteral {
-  return definition().ir_name();
+  static constexpr const llvm::StringLiteral Table[] = {
+#define CARBON_SEM_IR_INST_KIND(Name) SemIR::Name::Kind.ir_name(),
+#include "toolchain/sem_ir/inst_kind.def"
+  };
+  return Table[AsInt()];
 }
 
 auto InstKind::value_kind() const -> InstValueKind {
@@ -27,15 +31,11 @@ auto InstKind::value_kind() const -> InstValueKind {
 }
 
 auto InstKind::terminator_kind() const -> TerminatorKind {
-  return definition().terminator_kind();
-}
-
-auto InstKind::definition() const -> const Definition& {
-  static constexpr const Definition* Table[] = {
-#define CARBON_SEM_IR_INST_KIND(Name) &SemIR::Name::Kind,
+  static constexpr const TerminatorKind Table[] = {
+#define CARBON_SEM_IR_INST_KIND(Name) SemIR::Name::Kind.terminator_kind(),
 #include "toolchain/sem_ir/inst_kind.def"
   };
-  return *Table[AsInt()];
+  return Table[AsInt()];
 }
 
 }  // namespace Carbon::SemIR

+ 17 - 14
toolchain/sem_ir/inst_kind.h

@@ -48,6 +48,17 @@ class InstKind : public CARBON_ENUM_BASE(InstKind) {
 #define CARBON_SEM_IR_INST_KIND(Name) CARBON_ENUM_CONSTANT_DECL(Name)
 #include "toolchain/sem_ir/inst_kind.def"
 
+  template <typename TypedNodeId>
+  class Definition;
+
+  // Provides a definition for this instruction kind. Should only be called
+  // once, to construct the kind as part of defining it in `typed_insts.h`.
+  template <typename TypedNodeId>
+  constexpr auto Define(
+      llvm::StringLiteral ir_name,
+      TerminatorKind terminator_kind = TerminatorKind::NotTerminator) const
+      -> Definition<TypedNodeId>;
+
   using EnumBase::Create;
 
   // Returns the name to use for this instruction kind in Semantics IR.
@@ -66,18 +77,6 @@ class InstKind : public CARBON_ENUM_BASE(InstKind) {
   // Compute a fingerprint for this instruction kind, allowing its use as part
   // of the key in a `FoldingSet`.
   void Profile(llvm::FoldingSetNodeID& id) { id.AddInteger(AsInt()); }
-
-  class Definition;
-
-  // Provides a definition for this instruction kind. Should only be called
-  // once, to construct the kind as part of defining it in `typed_insts.h`.
-  constexpr auto Define(llvm::StringLiteral ir_name,
-                        TerminatorKind terminator_kind =
-                            TerminatorKind::NotTerminator) const -> Definition;
-
- private:
-  // Looks up the definition for this instruction kind.
-  auto definition() const -> const Definition&;
 };
 
 #define CARBON_SEM_IR_INST_KIND(Name) \
@@ -92,8 +91,11 @@ static_assert(sizeof(InstKind) == 1, "Kind objects include padding!");
 // are not copyable, and only one instance of this type is expected to exist per
 // instruction kind, specifically `TypedInst::Kind`. Use `InstKind` instead as a
 // thin wrapper around an instruction kind index.
+template <typename TypedNodeIdArg>
 class InstKind::Definition : public InstKind {
  public:
+  using TypedNodeId = TypedNodeIdArg;
+
   // Not copyable.
   Definition(const Definition&) = delete;
   auto operator=(const Definition&) -> Definition& = delete;
@@ -118,10 +120,11 @@ class InstKind::Definition : public InstKind {
   TerminatorKind terminator_kind_;
 };
 
+template <typename TypedNodeId>
 constexpr auto InstKind::Define(llvm::StringLiteral ir_name,
                                 TerminatorKind terminator_kind) const
-    -> Definition {
-  return Definition(*this, ir_name, terminator_kind);
+    -> Definition<TypedNodeId> {
+  return Definition<TypedNodeId>(*this, ir_name, terminator_kind);
 }
 
 }  // namespace Carbon::SemIR

+ 154 - 180
toolchain/sem_ir/typed_insts.h

@@ -17,20 +17,15 @@
 //
 // - Either a `Kind` constant, or a `Kinds` constant and an `InstKind kind;`
 //   member. These are described below.
-// - Optionally, a `Parse::NodeId parse_node;` member, for instructions with an
-//   associated location. Almost all instructions should have this, with
-//   exceptions being things that are generated internally, without any relation
-//   to source syntax, such as predeclared builtins.
-//   TODO: Make these typed parse node id types.
 // - Optionally, a `TypeId type_id;` member, for instructions that produce a
 //   value. This includes instructions that produce an abstract value, such as a
 //   `Namespace`, for which a placeholder type should be used.
 // - Up to two `[...]Id` members describing the contents of the struct.
 //
 // The field names here matter -- the fields must have the names specified
-// above, when present. When converting to a `SemIR::Inst`, the `kind`,
-// `parse_node`, and `type_id` fields will become the kind, parse node, and
-// type associated with the type-erased instruction.
+// above, when present. When converting to a `SemIR::Inst`, the `kind` and
+// `type_id` fields will become the kind and type associated with the
+// type-erased instruction.
 //
 // Each type that describes a single kind of instructions provides a constant
 // `Kind` that associates the type with a particular member of the `InstKind`
@@ -51,28 +46,28 @@
 namespace Carbon::SemIR {
 
 struct AddrOf {
-  static constexpr auto Kind = InstKind::AddrOf.Define("addr_of");
+  // TODO: Make Parse::NodeId more specific.
+  static constexpr auto Kind =
+      InstKind::AddrOf.Define<Parse::NodeId>("addr_of");
 
-  // TODO: Make this more specific.
-  Parse::NodeId parse_node;
   TypeId type_id;
   InstId lvalue_id;
 };
 
 struct AddrPattern {
-  static constexpr auto Kind = InstKind::AddrPattern.Define("addr_pattern");
+  static constexpr auto Kind =
+      InstKind::AddrPattern.Define<Parse::AddrId>("addr_pattern");
 
-  Parse::AddrId parse_node;
   TypeId type_id;
   // The `self` binding.
   InstId inner_id;
 };
 
 struct ArrayIndex {
-  static constexpr auto Kind = InstKind::ArrayIndex.Define("array_index");
+  // TODO: Make Parse::NodeId more specific.
+  static constexpr auto Kind =
+      InstKind::ArrayIndex.Define<Parse::NodeId>("array_index");
 
-  // TODO: Make this more specific.
-  Parse::NodeId parse_node;
   TypeId type_id;
   InstId array_id;
   InstId index_id;
@@ -82,19 +77,19 @@ struct ArrayIndex {
 // expression. `inits_id` contains one initializer per array element.
 // `dest_id` is the destination array object for the initialization.
 struct ArrayInit {
-  static constexpr auto Kind = InstKind::ArrayInit.Define("array_init");
+  // TODO: Make Parse::NodeId more specific.
+  static constexpr auto Kind =
+      InstKind::ArrayInit.Define<Parse::NodeId>("array_init");
 
-  // TODO: Make this more specific.
-  Parse::NodeId parse_node;
   TypeId type_id;
   InstBlockId inits_id;
   InstId dest_id;
 };
 
 struct ArrayType {
-  static constexpr auto Kind = InstKind::ArrayType.Define("array_type");
+  static constexpr auto Kind =
+      InstKind::ArrayType.Define<Parse::ArrayExprId>("array_type");
 
-  Parse::ArrayExprId parse_node;
   TypeId type_id;
   InstId bound_id;
   TypeId element_type_id;
@@ -104,10 +99,10 @@ struct ArrayType {
 // `rhs_id`. This finishes initialization of `lhs_id` in the same way as
 // `InitializeFrom`.
 struct Assign {
-  static constexpr auto Kind = InstKind::Assign.Define("assign");
+  static constexpr auto Kind = InstKind::Assign.Define<
+      Parse::NodeIdOneOf<Parse::InfixOperatorEqualId, Parse::VariableDeclId>>(
+      "assign");
 
-  Parse::NodeIdOneOf<Parse::InfixOperatorEqualId, Parse::VariableDeclId>
-      parse_node;
   // Assignments are statements, and so have no type.
   InstId lhs_id;
   InstId rhs_id;
@@ -117,9 +112,9 @@ struct Assign {
 // element of the derived class, and the type of the `BaseDecl` instruction is
 // an `UnboundElementType`.
 struct BaseDecl {
-  static constexpr auto Kind = InstKind::BaseDecl.Define("base_decl");
+  static constexpr auto Kind =
+      InstKind::BaseDecl.Define<Parse::BaseDeclId>("base_decl");
 
-  Parse::BaseDeclId parse_node;
   TypeId type_id;
   TypeId base_type_id;
   ElementIndex index;
@@ -132,7 +127,6 @@ struct AnyBindName {
                                        InstKind::BindSymbolicName};
 
   InstKind kind;
-  Parse::NodeId parse_node;
   TypeId type_id;
   BindNameId bind_name_id;
   InstId value_id;
@@ -140,19 +134,18 @@ struct AnyBindName {
 
 struct BindSymbolicName {
   static constexpr auto Kind =
-      InstKind::BindSymbolicName.Define("bind_symbolic_name");
+      InstKind::BindSymbolicName.Define<Parse::NodeId>("bind_symbolic_name");
 
-  Parse::NodeId parse_node;
   TypeId type_id;
   BindNameId bind_name_id;
   InstId value_id;
 };
 
 struct BindName {
-  static constexpr auto Kind = InstKind::BindName.Define("bind_name");
+  // TODO: Make Parse::NodeId more specific.
+  static constexpr auto Kind =
+      InstKind::BindName.Define<Parse::NodeId>("bind_name");
 
-  // TODO: Make this more specific.
-  Parse::NodeId parse_node;
   TypeId type_id;
   BindNameId bind_name_id;
   // The value is inline in the inst so that value access doesn't require an
@@ -161,28 +154,27 @@ struct BindName {
 };
 
 struct BindValue {
-  static constexpr auto Kind = InstKind::BindValue.Define("bind_value");
+  // TODO: Make Parse::NodeId more specific.
+  static constexpr auto Kind =
+      InstKind::BindValue.Define<Parse::NodeId>("bind_value");
 
-  // TODO: Make this more specific.
-  Parse::NodeId parse_node;
   TypeId type_id;
   InstId value_id;
 };
 
 struct BlockArg {
-  static constexpr auto Kind = InstKind::BlockArg.Define("block_arg");
+  // TODO: Make Parse::NodeId more specific.
+  static constexpr auto Kind =
+      InstKind::BlockArg.Define<Parse::NodeId>("block_arg");
 
-  // TODO: Make this more specific.
-  Parse::NodeId parse_node;
   TypeId type_id;
   InstBlockId block_id;
 };
 
 struct BoolLiteral {
-  static constexpr auto Kind = InstKind::BoolLiteral.Define("bool_literal");
+  static constexpr auto Kind =
+      InstKind::BoolLiteral.Define<Parse::NodeId>("bool_literal");
 
-  // TODO: Make this more specific.
-  Parse::NodeId parse_node;
   TypeId type_id;
   BoolValue value;
 };
@@ -190,9 +182,9 @@ struct BoolLiteral {
 // A bound method, that combines a function with the value to use for its
 // `self` parameter, such as `object.MethodName`.
 struct BoundMethod {
-  static constexpr auto Kind = InstKind::BoundMethod.Define("bound_method");
+  static constexpr auto Kind =
+      InstKind::BoundMethod.Define<Parse::MemberAccessExprId>("bound_method");
 
-  Parse::MemberAccessExprId parse_node;
   TypeId type_id;
   // The object argument in the bound method, which will be used to initialize
   // `self`, or whose address will be used to initialize `self` for an `addr
@@ -207,7 +199,6 @@ struct AnyBranch {
                                        InstKind::BranchWithArg};
 
   InstKind kind;
-  Parse::NodeId parse_node;
   // Branches don't produce a value, so have no type.
   InstBlockId target_id;
   // Kind-specific data.
@@ -215,49 +206,47 @@ struct AnyBranch {
 };
 
 struct Branch {
+  // TODO: Make Parse::NodeId more specific.
   static constexpr auto Kind =
-      InstKind::Branch.Define("br", TerminatorKind::Terminator);
+      InstKind::Branch.Define<Parse::NodeId>("br", TerminatorKind::Terminator);
 
-  // TODO: Make this more specific.
-  Parse::NodeId parse_node;
   // Branches don't produce a value, so have no type.
   InstBlockId target_id;
 };
 
 struct BranchIf {
-  static constexpr auto Kind =
-      InstKind::BranchIf.Define("br", TerminatorKind::TerminatorSequence);
+  // TODO: Make Parse::NodeId more specific.
+  static constexpr auto Kind = InstKind::BranchIf.Define<Parse::NodeId>(
+      "br", TerminatorKind::TerminatorSequence);
 
-  // TODO: Make this more specific.
-  Parse::NodeId parse_node;
   // Branches don't produce a value, so have no type.
   InstBlockId target_id;
   InstId cond_id;
 };
 
 struct BranchWithArg {
-  static constexpr auto Kind =
-      InstKind::BranchWithArg.Define("br", TerminatorKind::Terminator);
+  // TODO: Make Parse::NodeId more specific.
+  static constexpr auto Kind = InstKind::BranchWithArg.Define<Parse::NodeId>(
+      "br", TerminatorKind::Terminator);
 
-  // TODO: Make this more specific.
-  Parse::NodeId parse_node;
   // Branches don't produce a value, so have no type.
   InstBlockId target_id;
   InstId arg_id;
 };
 
 struct Builtin {
-  static constexpr auto Kind = InstKind::Builtin.Define("builtin");
-
   // Builtins don't have a parse node associated with them.
+  static constexpr auto Kind =
+      InstKind::Builtin.Define<Parse::InvalidNodeId>("builtin");
+
   TypeId type_id;
   BuiltinKind builtin_kind;
 };
 
 struct Call {
-  static constexpr auto Kind = InstKind::Call.Define("call");
+  static constexpr auto Kind =
+      InstKind::Call.Define<Parse::CallExprStartId>("call");
 
-  Parse::CallExprStartId parse_node;
   TypeId type_id;
   InstId callee_id;
   // The arguments block contains IDs for the following arguments, in order:
@@ -268,9 +257,9 @@ struct Call {
 };
 
 struct ClassDecl {
-  static constexpr auto Kind = InstKind::ClassDecl.Define("class_decl");
+  static constexpr auto Kind =
+      InstKind::ClassDecl.Define<Parse::AnyClassDeclId>("class_decl");
 
-  Parse::AnyClassDeclId parse_node;
   // No type: a class declaration is not itself a value. The name of a class
   // declaration becomes a class type value.
   // TODO: For a generic class declaration, the name of the class declaration
@@ -282,48 +271,47 @@ struct ClassDecl {
 };
 
 struct ClassElementAccess {
+  // TODO: Make Parse::NodeId more specific.
   static constexpr auto Kind =
-      InstKind::ClassElementAccess.Define("class_element_access");
+      InstKind::ClassElementAccess.Define<Parse::NodeId>(
+          "class_element_access");
 
-  // TODO: Make this more specific.
-  Parse::NodeId parse_node;
   TypeId type_id;
   InstId base_id;
   ElementIndex index;
 };
 
 struct ClassInit {
-  static constexpr auto Kind = InstKind::ClassInit.Define("class_init");
+  // TODO: Make Parse::NodeId more specific.
+  static constexpr auto Kind =
+      InstKind::ClassInit.Define<Parse::NodeId>("class_init");
 
-  // TODO: Make this more specific.
-  Parse::NodeId parse_node;
   TypeId type_id;
   InstBlockId elements_id;
   InstId dest_id;
 };
 
 struct ClassType {
-  static constexpr auto Kind = InstKind::ClassType.Define("class_type");
+  static constexpr auto Kind =
+      InstKind::ClassType.Define<Parse::AnyClassDeclId>("class_type");
 
-  Parse::AnyClassDeclId parse_node;
   TypeId type_id;
   ClassId class_id;
   // TODO: Once we support generic classes, include the class's arguments here.
 };
 
 struct ConstType {
-  static constexpr auto Kind = InstKind::ConstType.Define("const_type");
+  static constexpr auto Kind =
+      InstKind::ConstType.Define<Parse::PrefixOperatorConstId>("const_type");
 
-  Parse::PrefixOperatorConstId parse_node;
   TypeId type_id;
   TypeId inner_id;
 };
 
 struct Converted {
-  static constexpr auto Kind = InstKind::Converted.Define("converted");
+  static constexpr auto Kind =
+      InstKind::Converted.Define<Parse::NodeId>("converted");
 
-  // TODO: Make this more specific.
-  Parse::NodeId parse_node;
   TypeId type_id;
   InstId original_id;
   InstId result_id;
@@ -331,7 +319,7 @@ struct Converted {
 
 // A cross-reference between IRs.
 struct CrossRef {
-  static constexpr auto Kind = InstKind::CrossRef.Define("xref");
+  static constexpr auto Kind = InstKind::CrossRef.Define<Parse::NodeId>("xref");
 
   // No parse node: an instruction's parse tree node must refer to a node in the
   // current parse tree. This cannot use the cross-referenced instruction's
@@ -342,10 +330,9 @@ struct CrossRef {
 };
 
 struct Deref {
-  static constexpr auto Kind = InstKind::Deref.Define("deref");
+  // TODO: Make Parse::NodeId more specific.
+  static constexpr auto Kind = InstKind::Deref.Define<Parse::NodeId>("deref");
 
-  // TODO: Make this more specific.
-  Parse::NodeId parse_node;
   TypeId type_id;
   InstId pointer_id;
 };
@@ -353,18 +340,18 @@ struct Deref {
 // A field in a class, of the form `var field: field_type;`. The type of the
 // `FieldDecl` instruction is an `UnboundElementType`.
 struct FieldDecl {
-  static constexpr auto Kind = InstKind::FieldDecl.Define("field_decl");
+  static constexpr auto Kind =
+      InstKind::FieldDecl.Define<Parse::BindingPatternId>("field_decl");
 
-  Parse::BindingPatternId parse_node;
   TypeId type_id;
   NameId name_id;
   ElementIndex index;
 };
 
 struct FunctionDecl {
-  static constexpr auto Kind = InstKind::FunctionDecl.Define("fn_decl");
+  static constexpr auto Kind =
+      InstKind::FunctionDecl.Define<Parse::AnyFunctionDeclId>("fn_decl");
 
-  Parse::AnyFunctionDeclId parse_node;
   TypeId type_id;
   FunctionId function_id;
 };
@@ -374,10 +361,9 @@ struct FunctionDecl {
 // there was an import error, first_cross_ref_ir_id will reference a
 // nullptr IR; there should only ever be one nullptr in the range.
 struct Import {
-  static constexpr auto Kind = InstKind::Import.Define("import");
-
   // TODO: Should always be an ImportDirectiveId?
-  Parse::NodeId parse_node;
+  static constexpr auto Kind = InstKind::Import.Define<Parse::NodeId>("import");
+
   TypeId type_id;
   CrossRefIRId first_cross_ref_ir_id;
   CrossRefIRId last_cross_ref_ir_id;
@@ -387,20 +373,20 @@ struct Import {
 // `src_id`, by performing a final copy from source to destination, for types
 // whose initialization is not in-place.
 struct InitializeFrom {
+  // TODO: Make Parse::NodeId more specific.
   static constexpr auto Kind =
-      InstKind::InitializeFrom.Define("initialize_from");
+      InstKind::InitializeFrom.Define<Parse::NodeId>("initialize_from");
 
-  // TODO: Make this more specific.
-  Parse::NodeId parse_node;
   TypeId type_id;
   InstId src_id;
   InstId dest_id;
 };
 
 struct InterfaceDecl {
-  static constexpr auto Kind = InstKind::InterfaceDecl.Define("interface_decl");
+  static constexpr auto Kind =
+      InstKind::InterfaceDecl.Define<Parse::AnyInterfaceDeclId>(
+          "interface_decl");
 
-  Parse::AnyInterfaceDeclId parse_node;
   // No type: an interface declaration is not itself a value. The name of an
   // interface declaration becomes a facet type value.
   // TODO: For a generic interface declaration, the name of the interface
@@ -412,10 +398,10 @@ struct InterfaceDecl {
 };
 
 struct IntLiteral {
-  static constexpr auto Kind = InstKind::IntLiteral.Define("int_literal");
+  // TODO: Make Parse::NodeId more specific.
+  static constexpr auto Kind =
+      InstKind::IntLiteral.Define<Parse::NodeId>("int_literal");
 
-  // TODO: Make this more specific.
-  Parse::NodeId parse_node;
   TypeId type_id;
   IntId int_id;
 };
@@ -423,142 +409,139 @@ struct IntLiteral {
 // This instruction is not intended for direct use. Instead, it should be loaded
 // if it's referenced through name resolution.
 struct LazyImportRef {
-  static constexpr auto Kind =
-      InstKind::LazyImportRef.Define("lazy_import_ref");
-
   // No parse node: an instruction's parse tree node must refer to a node in the
   // current parse tree. This cannot use the cross-referenced instruction's
   // parse tree node because it will be in a different parse tree.
+  static constexpr auto Kind =
+      InstKind::LazyImportRef.Define<Parse::InvalidNodeId>("lazy_import_ref");
 
   CrossRefIRId ir_id;
   InstId inst_id;
 };
 
 struct NameRef {
-  static constexpr auto Kind = InstKind::NameRef.Define("name_ref");
+  // TODO: Make Parse::NodeId more specific.
+  static constexpr auto Kind =
+      InstKind::NameRef.Define<Parse::NodeId>("name_ref");
 
-  // TODO: Make this more specific.
-  Parse::NodeId parse_node;
   TypeId type_id;
   NameId name_id;
   InstId value_id;
 };
 
 struct Namespace {
-  static constexpr auto Kind = InstKind::Namespace.Define("namespace");
+  static constexpr auto Kind =
+      InstKind::Namespace.Define<Parse::NamespaceId>("namespace");
 
-  Parse::NamespaceId parse_node;
   TypeId type_id;
   NameId name_id;
   NameScopeId name_scope_id;
 };
 
 struct Param {
-  static constexpr auto Kind = InstKind::Param.Define("param");
+  // TODO: Make Parse::NodeId more specific.
+  static constexpr auto Kind = InstKind::Param.Define<Parse::NodeId>("param");
 
-  // TODO: Make this more specific.
-  Parse::NodeId parse_node;
   TypeId type_id;
   NameId name_id;
 };
 
 struct PointerType {
-  static constexpr auto Kind = InstKind::PointerType.Define("ptr_type");
+  // TODO: Make Parse::NodeId more specific.
+  static constexpr auto Kind =
+      InstKind::PointerType.Define<Parse::NodeId>("ptr_type");
 
-  // TODO: Make this more specific.
-  Parse::NodeId parse_node;
   TypeId type_id;
   TypeId pointee_id;
 };
 
 struct RealLiteral {
-  static constexpr auto Kind = InstKind::RealLiteral.Define("real_literal");
+  static constexpr auto Kind =
+      InstKind::RealLiteral.Define<Parse::RealLiteralId>("real_literal");
 
-  Parse::RealLiteralId parse_node;
   TypeId type_id;
   RealId real_id;
 };
 
 struct Return {
   static constexpr auto Kind =
-      InstKind::Return.Define("return", TerminatorKind::Terminator);
+      InstKind::Return.Define<Parse::NodeIdOneOf<Parse::FunctionDefinitionId,
+                                                 Parse::ReturnStatementId>>(
+          "return", TerminatorKind::Terminator);
 
-  Parse::NodeIdOneOf<Parse::FunctionDefinitionId, Parse::ReturnStatementId>
-      parse_node;
   // This is a statement, so has no type.
 };
 
 struct ReturnExpr {
   static constexpr auto Kind =
-      InstKind::ReturnExpr.Define("return", TerminatorKind::Terminator);
+      InstKind::ReturnExpr.Define<Parse::ReturnStatementId>(
+          "return", TerminatorKind::Terminator);
 
-  Parse::ReturnStatementId parse_node;
   // This is a statement, so has no type.
   InstId expr_id;
 };
 
 struct SpliceBlock {
-  static constexpr auto Kind = InstKind::SpliceBlock.Define("splice_block");
+  // TODO: Can we make Parse::NodeId more specific?
+  static constexpr auto Kind =
+      InstKind::SpliceBlock.Define<Parse::NodeId>("splice_block");
 
-  // TODO: Can we make this more specific?
-  Parse::NodeId parse_node;
   TypeId type_id;
   InstBlockId block_id;
   InstId result_id;
 };
 
 struct StringLiteral {
-  static constexpr auto Kind = InstKind::StringLiteral.Define("string_literal");
+  static constexpr auto Kind =
+      InstKind::StringLiteral.Define<Parse::StringLiteralId>("string_literal");
 
-  Parse::StringLiteralId parse_node;
   TypeId type_id;
   StringLiteralValueId string_literal_id;
 };
 
 struct StructAccess {
-  static constexpr auto Kind = InstKind::StructAccess.Define("struct_access");
+  // TODO: Make Parse::NodeId more specific.
+  static constexpr auto Kind =
+      InstKind::StructAccess.Define<Parse::NodeId>("struct_access");
 
-  // TODO: Make this more specific.
-  Parse::NodeId parse_node;
   TypeId type_id;
   InstId struct_id;
   ElementIndex index;
 };
 
 struct StructInit {
-  static constexpr auto Kind = InstKind::StructInit.Define("struct_init");
+  // TODO: Make Parse::NodeId more specific.
+  static constexpr auto Kind =
+      InstKind::StructInit.Define<Parse::NodeId>("struct_init");
 
-  // TODO: Make this more specific.
-  Parse::NodeId parse_node;
   TypeId type_id;
   InstBlockId elements_id;
   InstId dest_id;
 };
 
 struct StructLiteral {
-  static constexpr auto Kind = InstKind::StructLiteral.Define("struct_literal");
+  static constexpr auto Kind =
+      InstKind::StructLiteral.Define<Parse::StructLiteralId>("struct_literal");
 
-  Parse::StructLiteralId parse_node;
   TypeId type_id;
   InstBlockId elements_id;
 };
 
 struct StructType {
-  static constexpr auto Kind = InstKind::StructType.Define("struct_type");
-
   // TODO: Make this more specific. It can be one of: ClassDefinitionId,
   // StructLiteralId, StructTypeLiteralId
-  Parse::NodeId parse_node;
+  static constexpr auto Kind =
+      InstKind::StructType.Define<Parse::NodeId>("struct_type");
+
   TypeId type_id;
   InstBlockId fields_id;
 };
 
 struct StructTypeField {
+  // TODO: Make Parse::NodeId more specific.
   static constexpr auto Kind =
-      InstKind::StructTypeField.Define("struct_type_field");
+      InstKind::StructTypeField.Define<Parse::NodeId>("struct_type_field");
 
-  // TODO: Make this more specific.
-  Parse::NodeId parse_node;
   // This instruction is an implementation detail of `StructType`, and doesn't
   // produce a value, so has no type, even though it declares a field with a
   // type.
@@ -567,93 +550,92 @@ struct StructTypeField {
 };
 
 struct StructValue {
-  static constexpr auto Kind = InstKind::StructValue.Define("struct_value");
+  // TODO: Make Parse::NodeId more specific.
+  static constexpr auto Kind =
+      InstKind::StructValue.Define<Parse::NodeId>("struct_value");
 
-  // TODO: Make this more specific.
-  Parse::NodeId parse_node;
   TypeId type_id;
   InstBlockId elements_id;
 };
 
 struct Temporary {
-  static constexpr auto Kind = InstKind::Temporary.Define("temporary");
+  // TODO: Make Parse::NodeId more specific.
+  static constexpr auto Kind =
+      InstKind::Temporary.Define<Parse::NodeId>("temporary");
 
-  // TODO: Make this more specific.
-  Parse::NodeId parse_node;
   TypeId type_id;
   InstId storage_id;
   InstId init_id;
 };
 
 struct TemporaryStorage {
+  // TODO: Make Parse::NodeId more specific.
   static constexpr auto Kind =
-      InstKind::TemporaryStorage.Define("temporary_storage");
+      InstKind::TemporaryStorage.Define<Parse::NodeId>("temporary_storage");
 
-  // TODO: Make this more specific.
-  Parse::NodeId parse_node;
   TypeId type_id;
 };
 
 struct TupleAccess {
-  static constexpr auto Kind = InstKind::TupleAccess.Define("tuple_access");
+  // TODO: Make Parse::NodeId more specific.
+  static constexpr auto Kind =
+      InstKind::TupleAccess.Define<Parse::NodeId>("tuple_access");
 
-  // TODO: Make this more specific.
-  Parse::NodeId parse_node;
   TypeId type_id;
   InstId tuple_id;
   ElementIndex index;
 };
 
 struct TupleIndex {
-  static constexpr auto Kind = InstKind::TupleIndex.Define("tuple_index");
+  static constexpr auto Kind =
+      InstKind::TupleIndex.Define<Parse::IndexExprId>("tuple_index");
 
-  Parse::IndexExprId parse_node;
   TypeId type_id;
   InstId tuple_id;
   InstId index_id;
 };
 
 struct TupleInit {
-  static constexpr auto Kind = InstKind::TupleInit.Define("tuple_init");
+  // TODO: Make Parse::NodeId more specific.
+  static constexpr auto Kind =
+      InstKind::TupleInit.Define<Parse::NodeId>("tuple_init");
 
-  // TODO: Make this more specific.
-  Parse::NodeId parse_node;
   TypeId type_id;
   InstBlockId elements_id;
   InstId dest_id;
 };
 
 struct TupleLiteral {
-  static constexpr auto Kind = InstKind::TupleLiteral.Define("tuple_literal");
+  static constexpr auto Kind =
+      InstKind::TupleLiteral.Define<Parse::TupleLiteralId>("tuple_literal");
 
-  Parse::TupleLiteralId parse_node;
   TypeId type_id;
   InstBlockId elements_id;
 };
 
 struct TupleType {
-  static constexpr auto Kind = InstKind::TupleType.Define("tuple_type");
+  // TODO: Make Parse::NodeId more specific.
+  static constexpr auto Kind =
+      InstKind::TupleType.Define<Parse::NodeId>("tuple_type");
 
-  // TODO: Make this more specific.
-  Parse::NodeId parse_node;
   TypeId type_id;
   TypeBlockId elements_id;
 };
 
 struct TupleValue {
-  static constexpr auto Kind = InstKind::TupleValue.Define("tuple_value");
+  // TODO: Make Parse::NodeId more specific.
+  static constexpr auto Kind =
+      InstKind::TupleValue.Define<Parse::NodeId>("tuple_value");
 
-  // TODO: Make this more specific.
-  Parse::NodeId parse_node;
   TypeId type_id;
   InstBlockId elements_id;
 };
 
 struct UnaryOperatorNot {
-  static constexpr auto Kind = InstKind::UnaryOperatorNot.Define("not");
+  // TODO: Make Parse::NodeId more specific.
+  static constexpr auto Kind =
+      InstKind::UnaryOperatorNot.Define<Parse::NodeId>("not");
 
-  // TODO: Make this more specific.
-  Parse::NodeId parse_node;
   TypeId type_id;
   InstId operand_id;
 };
@@ -662,10 +644,10 @@ struct UnaryOperatorNot {
 // `Class.field`. This can be used as the operand of a compound member access
 // expression, such as `instance.(Class.field)`.
 struct UnboundElementType {
-  static constexpr auto Kind =
-      InstKind::UnboundElementType.Define("unbound_element_type");
+  static constexpr auto Kind = InstKind::UnboundElementType.Define<
+      Parse::NodeIdOneOf<Parse::BaseDeclId, Parse::BindingPatternId>>(
+      "unbound_element_type");
 
-  Parse::NodeIdOneOf<Parse::BaseDeclId, Parse::BindingPatternId> parse_node;
   TypeId type_id;
   // The class that a value of this type is an element of.
   TypeId class_type_id;
@@ -674,28 +656,28 @@ struct UnboundElementType {
 };
 
 struct ValueAsRef {
-  static constexpr auto Kind = InstKind::ValueAsRef.Define("value_as_ref");
+  static constexpr auto Kind =
+      InstKind::ValueAsRef.Define<Parse::IndexExprId>("value_as_ref");
 
-  Parse::IndexExprId parse_node;
   TypeId type_id;
   InstId value_id;
 };
 
 struct ValueOfInitializer {
+  // TODO: Make Parse::NodeId more specific.
   static constexpr auto Kind =
-      InstKind::ValueOfInitializer.Define("value_of_initializer");
+      InstKind::ValueOfInitializer.Define<Parse::NodeId>(
+          "value_of_initializer");
 
-  // TODO: Make this more specific.
-  Parse::NodeId parse_node;
   TypeId type_id;
   InstId init_id;
 };
 
 struct VarStorage {
-  static constexpr auto Kind = InstKind::VarStorage.Define("var");
+  // TODO: Make Parse::NodeId more specific.
+  static constexpr auto Kind =
+      InstKind::VarStorage.Define<Parse::NodeId>("var");
 
-  // TODO: Make this more specific.
-  Parse::NodeId parse_node;
   TypeId type_id;
   NameId name_id;
 };
@@ -707,14 +689,6 @@ inline constexpr bool HasKindMemberAsField = false;
 template <typename T>
 inline constexpr bool HasKindMemberAsField<T, decltype(&T::kind)> = true;
 
-// HasParseNodeMember<T> is true if T has a `U parse_node` field,
-// where `U` extends `Parse::NodeId`.
-template <typename T, bool Enabled = true>
-inline constexpr bool HasParseNodeMember = false;
-template <typename T>
-inline constexpr bool HasParseNodeMember<
-    T, bool(std::is_base_of_v<Parse::NodeId, decltype(T::parse_node)>)> = true;
-
 // HasTypeIdMember<T> is true if T has a `TypeId type_id` field.
 template <typename T, typename TypeIdType = TypeId T::*>
 inline constexpr bool HasTypeIdMember = false;

+ 18 - 29
toolchain/sem_ir/typed_insts_test.cpp

@@ -14,9 +14,9 @@ namespace Carbon::SemIR {
 // A friend of `SemIR::Inst` that is used to pierce the abstraction.
 class InstTestHelper {
  public:
-  static auto MakeInst(InstKind inst_kind, Parse::NodeId parse_node,
-                       TypeId type_id, int32_t arg0, int32_t arg1) -> Inst {
-    return Inst(inst_kind, parse_node, type_id, arg0, arg1);
+  static auto MakeInst(InstKind inst_kind, TypeId type_id, int32_t arg0,
+                       int32_t arg1) -> Inst {
+    return Inst(inst_kind, type_id, arg0, arg1);
   }
 };
 
@@ -32,10 +32,9 @@ namespace {
 #include "toolchain/sem_ir/inst_kind.def"
 
 auto MakeInstWithNumberedFields(InstKind kind) -> Inst {
-  Inst inst = InstTestHelper::MakeInst(kind, Parse::NodeId(1), TypeId(2), 3, 4);
+  Inst inst = InstTestHelper::MakeInst(kind, TypeId(1), 2, 3);
   EXPECT_EQ(inst.kind(), kind);
-  EXPECT_EQ(inst.parse_node(), Parse::NodeId(1));
-  EXPECT_EQ(inst.type_id(), TypeId(2));
+  EXPECT_EQ(inst.type_id(), TypeId(1));
   return inst;
 }
 
@@ -43,12 +42,8 @@ template <typename TypedInst>
 auto CommonFieldOrder() -> void {
   Inst inst = MakeInstWithNumberedFields(TypedInst::Kind);
   auto typed = inst.As<TypedInst>();
-  if constexpr (HasParseNodeMember<TypedInst>) {
-    EXPECT_EQ(typed.parse_node,
-              decltype(TypedInst::parse_node)(Parse::NodeId(1)));
-  }
   if constexpr (HasTypeIdMember<TypedInst>) {
-    EXPECT_EQ(typed.type_id, TypeId(2));
+    EXPECT_EQ(typed.type_id, TypeId(1));
   }
 }
 
@@ -61,12 +56,9 @@ TEST(TypedInstTest, CommonFieldOrder) {
 #include "toolchain/sem_ir/inst_kind.def"
 }
 
-auto ExpectEqInsts(const Inst& inst1, const Inst& inst2,
-                   bool compare_parse_node, bool compare_type_id) -> void {
+auto ExpectEqInsts(const Inst& inst1, const Inst& inst2, bool compare_type_id)
+    -> void {
   EXPECT_EQ(inst1.kind(), inst2.kind());
-  if (compare_parse_node) {
-    EXPECT_EQ(inst1.parse_node(), inst2.parse_node());
-  }
   if (compare_type_id) {
     EXPECT_EQ(inst1.type_id(), inst2.type_id());
   }
@@ -78,8 +70,7 @@ auto RoundTrip() -> void {
   auto typed1 = inst1.As<TypedInst>();
   Inst inst2 = typed1;
 
-  ExpectEqInsts(inst1, inst2, HasParseNodeMember<TypedInst>,
-                HasTypeIdMember<TypedInst>);
+  ExpectEqInsts(inst1, inst2, HasTypeIdMember<TypedInst>);
 
   // If the typed instruction has no padding, we should get exactly the same
   // thing if we convert back from an instruction.
@@ -92,7 +83,7 @@ auto RoundTrip() -> void {
   // because the fields not carried by the typed instruction are lost. But they
   // should be stable if we round-trip again.
   Inst inst3 = typed2;
-  ExpectEqInsts(inst2, inst3, true, true);
+  ExpectEqInsts(inst2, inst3, true);
 }
 
 TEST(TypedInstTest, RoundTrip) {
@@ -105,21 +96,18 @@ TEST(TypedInstTest, RoundTrip) {
 }
 
 auto StructLayoutHelper(void* typed_inst, std::size_t typed_inst_size,
-                        bool has_parse_node, bool has_type_id) -> void {
+                        bool has_type_id) -> void {
   // Check that the memory representation of the typed instruction is what we
   // expect.
   // TODO: Struct layout is not guaranteed, and this test could fail in some
   // build environment. If so, we should disable it.
-  int32_t fields[4] = {};
+  int32_t fields[3] = {};
   int field = 0;
-  if (has_parse_node) {
-    fields[field++] = 1;
-  }
   if (has_type_id) {
-    fields[field++] = 2;
+    fields[field++] = 1;
   }
+  fields[field++] = 2;
   fields[field++] = 3;
-  fields[field++] = 4;
 
   ASSERT_LE(typed_inst_size, sizeof(int32_t) * field);
   EXPECT_EQ(std::memcmp(&fields, typed_inst, typed_inst_size), 0);
@@ -131,8 +119,7 @@ auto StructLayout() -> void {
   if constexpr (std::has_unique_object_representations_v<TypedInst>) {
     auto typed =
         MakeInstWithNumberedFields(TypedInst::Kind).template As<TypedInst>();
-    StructLayoutHelper(&typed, sizeof(typed), HasParseNodeMember<TypedInst>,
-                       HasTypeIdMember<TypedInst>);
+    StructLayoutHelper(&typed, sizeof(typed), HasTypeIdMember<TypedInst>);
   }
 }
 
@@ -145,7 +132,9 @@ TEST(TypedInstTest, StructLayout) {
 #include "toolchain/sem_ir/inst_kind.def"
 }
 
-auto InstKindMatches(const InstKind::Definition& def, InstKind kind) {
+template <typename TypedNodeId>
+auto InstKindMatches(const InstKind::Definition<TypedNodeId>& def,
+                     InstKind kind) {
   EXPECT_EQ(def.ir_name(), kind.ir_name());
   EXPECT_EQ(def.terminator_kind(), kind.terminator_kind());
 }

+ 54 - 11
toolchain/sem_ir/value_stores.h

@@ -8,11 +8,40 @@
 #include "llvm/ADT/DenseMap.h"
 #include "toolchain/base/value_store.h"
 #include "toolchain/base/yaml.h"
+#include "toolchain/parse/node_ids.h"
 #include "toolchain/sem_ir/inst.h"
 #include "toolchain/sem_ir/type_info.h"
 
 namespace Carbon::SemIR {
 
+// Associates a NodeId and Inst in order to provide type-checking that the
+// TypedNodeId corresponds to the InstT.
+struct ParseNodeAndInst {
+  // In cases where the NodeId is untyped or the InstT is unknown, the check
+  // can't be done at compile time.
+  // TODO: Consider runtime validation that InstT::Kind::TypedNodeId
+  // corresponds.
+  static auto Untyped(Parse::NodeId parse_node, Inst inst) -> ParseNodeAndInst {
+    return ParseNodeAndInst(parse_node, inst, /*is_untyped=*/true);
+  }
+
+  // For the common case, support construction as:
+  //   context.AddInst({parse_node, SemIR::MyInst{...}});
+  template <typename InstT>
+  // NOLINTNEXTLINE(google-explicit-constructor)
+  ParseNodeAndInst(typename decltype(InstT::Kind)::TypedNodeId parse_node,
+                   InstT inst)
+      : parse_node(parse_node), inst(inst) {}
+
+  Parse::NodeId parse_node;
+  Inst inst;
+
+ private:
+  explicit ParseNodeAndInst(Parse::NodeId parse_node, Inst inst,
+                            bool /*is_untyped*/)
+      : parse_node(parse_node), inst(inst) {}
+};
+
 // Provides a ValueStore wrapper for an API specific to instructions.
 class InstStore {
  public:
@@ -21,7 +50,10 @@ class InstStore {
   // instruction block. Check::Context::AddInst or InstBlockStack::AddInst
   // should usually be used instead, to add the instruction to the current
   // block.
-  auto AddInNoBlock(Inst inst) -> InstId { return values_.Add(inst); }
+  auto AddInNoBlock(ParseNodeAndInst parse_node_and_inst) -> InstId {
+    parse_nodes_.push_back(parse_node_and_inst.parse_node);
+    return values_.Add(parse_node_and_inst.inst);
+  }
 
   // Returns the requested instruction.
   auto Get(InstId inst_id) const -> Inst { return values_.Get(inst_id); }
@@ -43,13 +75,25 @@ class InstStore {
   // Overwrites a given instruction with a new value.
   auto Set(InstId inst_id, Inst inst) -> void { values_.Get(inst_id) = inst; }
 
+  auto GetParseNode(InstId inst_id) const -> Parse::NodeId {
+    return parse_nodes_[inst_id.index];
+  }
+
+  auto SetParseNode(InstId inst_id, Parse::NodeId parse_node) -> void {
+    parse_nodes_[inst_id.index] = parse_node;
+  }
+
   // Reserves space.
-  auto Reserve(size_t size) -> void { values_.Reserve(size); }
+  auto Reserve(size_t size) -> void {
+    parse_nodes_.reserve(size);
+    values_.Reserve(size);
+  }
 
   auto array_ref() const -> llvm::ArrayRef<Inst> { return values_.array_ref(); }
   auto size() const -> int { return values_.size(); }
 
  private:
+  llvm::SmallVector<Parse::NodeId> parse_nodes_;
   ValueStore<InstId> values_;
 };
 
@@ -178,8 +222,7 @@ class TypeStore : public ValueStore<TypeId> {
 // A name is either an identifier name or a special name such as `self` that
 // does not correspond to an identifier token. Identifier names are represented
 // as `NameId`s with the same non-negative index as the `IdentifierId` of the
-// identifier. Special names are represented as `NameId`s with a negative
-// index.
+// identifier. Special names are represented as `NameId`s with a negative index.
 //
 // `SemIR::NameId` values should be obtained by using `NameId::ForIdentifier`
 // or the named constants such as `NameId::SelfValue`.
@@ -206,10 +249,10 @@ class NameStoreWrapper {
   // `"r#name"` if `name` is a keyword.
   auto GetFormatted(NameId name_id) const -> llvm::StringRef;
 
-  // Returns a best-effort name to use as the basis for SemIR and LLVM IR
-  // names. This is always identifier-shaped, but may be ambiguous, for example
-  // if there is both a `self` and an `r#self` in the same scope. Returns ""
-  // for an invalid name.
+  // Returns a best-effort name to use as the basis for SemIR and LLVM IR names.
+  // This is always identifier-shaped, but may be ambiguous, for example if
+  // there is both a `self` and an `r#self` in the same scope. Returns "" for an
+  // invalid name.
   auto GetIRBaseName(NameId name_id) const -> llvm::StringRef;
 
  private:
@@ -248,9 +291,9 @@ struct NameScope : Printable<NameScope> {
 
   // Whether we have diagnosed an error in a construct that would have added
   // names to this scope. For example, this can happen if an `import` failed or
-  // an `extend` declaration was ill-formed. If true, the `names` map is
-  // assumed to be missing names as a result of the error, and no further
-  // errors are produced for lookup failures in this scope.
+  // an `extend` declaration was ill-formed. If true, the `names` map is assumed
+  // to be missing names as a result of the error, and no further errors are
+  // produced for lookup failures in this scope.
   bool has_error = false;
 };