Add MakeVerifiedLocIdAndInst for runtime validation (#6942)

This follows up on a discussion about wanting to use `Any*` inst
clusters to handle boilerplate construction, with the issue that
`UncheckedLoc` use removes validation. Some context is at
https://github.com/carbon-language/carbon-lang/pull/6930#discussion_r2963157428.

This folds in `MakeImportedLocIdAndInst` because the logic is related,
particularly for `LocId` values which are `ImportIRInstId`, and it
eliminates questions of what the right function is to use.

This uncovers an error in the `NodeKind` associated with
`FormBindingPattern`. For now I'm just adding a TODO regarding that.

Assisted-by: Google Antigravity with Gemini

toolchain/check/check_unit.cpp

@@ -355,11 +355,12 @@ auto CheckUnit::ImportOtherPackages(SemIR::TypeId namespace_type_id) -> void {
         auto import_ir_inst_id =
             context_.import_ir_insts().Add(SemIR::ImportIRInst(
                 SemIR::ImportIRId::ApiForImpl, api_imports->import_decl_id));
-        import_decl_id =
-            AddInst(context_, MakeImportedLocIdAndInst<SemIR::ImportDecl>(
-                                  context_, import_ir_inst_id,
-                                  {.package_id = SemIR::NameId::ForPackageName(
-                                       api_imports_entry.first)}));
+        import_decl_id = AddInst(
+            context_,
+            SemIR::LocIdAndInst::RuntimeVerified(
+                context_.sem_ir(), import_ir_inst_id,
+                SemIR::ImportDecl{.package_id = SemIR::NameId::ForPackageName(
+                                      api_imports_entry.first)}));
         package_id = api_imports_entry.first;
       }
       has_load_error |= api_imports->has_load_error;

toolchain/check/convert.cpp

@@ -2276,8 +2276,8 @@ auto ReturnExprAsForm(Context& context, SemIR::LocId loc_id,
     }
     form_inst_id = AddInst(
         context,
-        SemIR::LocIdAndInst::UncheckedLoc(
-            loc_id,
+        SemIR::LocIdAndInst::RuntimeVerified(
+            context.sem_ir(), loc_id,
             SemIR::RefForm{.type_id = SemIR::FormType::TypeId,
                            .type_component_inst_id =
                                context.types().GetAsTypeInstId(type_inst_id)}));
@@ -2292,12 +2292,12 @@ auto ReturnExprAsForm(Context& context, SemIR::LocId loc_id,
       return Context::FormExpr::Error;
     }
     form_inst_id = AddInst(
-        context,
-        SemIR::LocIdAndInst::UncheckedLoc(
-            loc_id, SemIR::InitForm{
-                        .type_id = SemIR::FormType::TypeId,
-                        .type_component_inst_id =
-                            context.types().GetAsTypeInstId(type_inst_id)}));
+        context, SemIR::LocIdAndInst::RuntimeVerified(
+                     context.sem_ir(), loc_id,
+                     SemIR::InitForm{
+                         .type_id = SemIR::FormType::TypeId,
+                         .type_component_inst_id =
+                             context.types().GetAsTypeInstId(type_inst_id)}));
   }
 
   auto type_const_id = context.constant_values().Get(type_inst_id);

toolchain/check/cpp/import.cpp

@@ -318,8 +318,8 @@ static auto BuildClassDecl(Context& context,
                                      .class_id = SemIR::ClassId::None,
                                      .decl_block_id = SemIR::InstBlockId::None};
   auto class_decl_id = AddPlaceholderImportedInstInNoBlock(
-      context,
-      MakeImportedLocIdAndInst(context, import_ir_inst_id, class_decl));
+      context, SemIR::LocIdAndInst::RuntimeVerified(
+                   context.sem_ir(), import_ir_inst_id, class_decl));
 
   SemIR::Class class_info = {
       {.name_id = name_id,
@@ -438,8 +438,8 @@ static auto ImportClassObjectRepr(Context& context, SemIR::ClassId class_id,
       clang_def->isAggregate()) {
     return context.types().GetAsTypeInstId(AddInst(
         context,
-        MakeImportedLocIdAndInst(
-            context, import_ir_inst_id,
+        SemIR::LocIdAndInst::RuntimeVerified(
+            context.sem_ir(), import_ir_inst_id,
             SemIR::StructType{.type_id = SemIR::TypeType::TypeId,
                               .fields_id = SemIR::StructTypeFieldsId::Empty})));
   }
@@ -491,8 +491,8 @@ static auto ImportClassObjectRepr(Context& context, SemIR::ClassId class_id,
 
     auto base_decl_id = AddInst(
         context,
-        MakeImportedLocIdAndInst(
-            context, import_ir_inst_id,
+        SemIR::LocIdAndInst::RuntimeVerified(
+            context.sem_ir(), import_ir_inst_id,
             SemIR::BaseDecl{.type_id = GetUnboundElementType(
                                 context, class_type_inst_id, base_type_inst_id),
                             .base_type_inst_id = base_type_inst_id,
@@ -574,8 +574,8 @@ static auto ImportClassObjectRepr(Context& context, SemIR::ClassId class_id,
     // Create a field now, as we know the index to use.
     // TODO: Consider doing this lazily instead.
     auto field_decl_id = AddInst(
-        context, MakeImportedLocIdAndInst(
-                     context, import_ir_inst_id,
+        context, SemIR::LocIdAndInst::RuntimeVerified(
+                     context.sem_ir(), import_ir_inst_id,
                      SemIR::FieldDecl{
                          .type_id = GetUnboundElementType(
                              context, class_type_inst_id, field_type_inst_id),
@@ -607,12 +607,13 @@ static auto ImportClassObjectRepr(Context& context, SemIR::ClassId class_id,
 
   // TODO: Add a field to prevent tail padding reuse if necessary.
 
-  return AddTypeInst(context,
-                     MakeImportedLocIdAndInst<SemIR::CustomLayoutType>(
-                         context, import_ir_inst_id,
-                         {.type_id = SemIR::TypeType::TypeId,
-                          .fields_id = context.struct_type_fields().Add(fields),
-                          .layout_id = context.custom_layouts().Add(layout)}));
+  return AddTypeInst(
+      context, SemIR::LocIdAndInst::RuntimeVerified(
+                   context.sem_ir(), import_ir_inst_id,
+                   SemIR::CustomLayoutType{
+                       .type_id = SemIR::TypeType::TypeId,
+                       .fields_id = context.struct_type_fields().Add(fields),
+                       .layout_id = context.custom_layouts().Add(layout)}));
 }
 
 // Creates a Carbon class definition based on the information in the given Clang
@@ -633,12 +634,13 @@ static auto BuildClassDefinition(Context& context,
   // Compute the class's object representation.
   auto object_repr_id = ImportClassObjectRepr(
       context, class_id, import_ir_inst_id, class_inst_id, clang_def);
-  class_info.complete_type_witness_id = AddInst(
-      context,
-      MakeImportedLocIdAndInst<SemIR::CompleteTypeWitness>(
-          context, import_ir_inst_id,
-          {.type_id = GetSingletonType(context, SemIR::WitnessType::TypeInstId),
-           .object_repr_type_inst_id = object_repr_id}));
+  class_info.complete_type_witness_id =
+      AddInst(context, SemIR::LocIdAndInst::RuntimeVerified(
+                           context.sem_ir(), import_ir_inst_id,
+                           SemIR::CompleteTypeWitness{
+                               .type_id = GetSingletonType(
+                                   context, SemIR::WitnessType::TypeInstId),
+                               .object_repr_type_inst_id = object_repr_id}));
 
   class_info.body_block_id = context.inst_block_stack().Pop();
 }
@@ -656,12 +658,13 @@ static auto ImportEnumObjectRepresentation(
   auto int_kind = int_type->isSignedIntegerType() ? SemIR::IntKind::Signed
                                                   : SemIR::IntKind::Unsigned;
   auto bit_width_id = GetOrAddInst(
-      context, MakeImportedLocIdAndInst<SemIR::IntValue>(
-                   context, import_ir_inst_id,
-                   {.type_id = GetSingletonType(
-                        context, SemIR::IntLiteralType::TypeInstId),
-                    .int_id = context.ints().AddUnsigned(llvm::APInt(
-                        64, context.ast_context().getIntWidth(int_type)))}));
+      context, SemIR::LocIdAndInst::RuntimeVerified(
+                   context.sem_ir(), import_ir_inst_id,
+                   SemIR::IntValue{
+                       .type_id = GetSingletonType(
+                           context, SemIR::IntLiteralType::TypeInstId),
+                       .int_id = context.ints().AddUnsigned(llvm::APInt(
+                           64, context.ast_context().getIntWidth(int_type)))}));
   return context.types().GetAsTypeInstId(
       GetOrAddInst(context, SemIR::LocIdAndInst::NoLoc(SemIR::IntType{
                                 .type_id = SemIR::TypeType::TypeId,
@@ -690,15 +693,16 @@ static auto BuildEnumDefinition(Context& context,
   auto object_repr_id =
       ImportEnumObjectRepresentation(context, import_ir_inst_id, enum_decl);
   class_info.adapt_id = AddInst(
-      context, MakeImportedLocIdAndInst(
-                   context, import_ir_inst_id,
+      context, SemIR::LocIdAndInst::RuntimeVerified(
+                   context.sem_ir(), import_ir_inst_id,
                    SemIR::AdaptDecl{.adapted_type_inst_id = object_repr_id}));
-  class_info.complete_type_witness_id = AddInst(
-      context,
-      MakeImportedLocIdAndInst<SemIR::CompleteTypeWitness>(
-          context, import_ir_inst_id,
-          {.type_id = GetSingletonType(context, SemIR::WitnessType::TypeInstId),
-           .object_repr_type_inst_id = object_repr_id}));
+  class_info.complete_type_witness_id =
+      AddInst(context, SemIR::LocIdAndInst::RuntimeVerified(
+                           context.sem_ir(), import_ir_inst_id,
+                           SemIR::CompleteTypeWitness{
+                               .type_id = GetSingletonType(
+                                   context, SemIR::WitnessType::TypeInstId),
+                               .object_repr_type_inst_id = object_repr_id}));
 
   class_info.body_block_id = context.inst_block_stack().Pop();
 }
@@ -721,9 +725,9 @@ static auto ImportEnumConstantDecl(Context& context,
   auto import_ir_inst_id =
       AddImportIRInst(context.sem_ir(), enumerator_decl->getLocation());
   auto inst_id = AddInstInNoBlock(
-      context,
-      MakeImportedLocIdAndInst<SemIR::IntValue>(
-          context, import_ir_inst_id, {.type_id = type_id, .int_id = int_id}));
+      context, SemIR::LocIdAndInst::RuntimeVerified(
+                   context.sem_ir(), import_ir_inst_id,
+                   SemIR::IntValue{.type_id = type_id, .int_id = int_id}));
   context.imports().push_back(inst_id);
   context.clang_decls().Add({.key = key, .inst_id = inst_id});
   return inst_id;
@@ -1207,10 +1211,10 @@ static auto MakeParamPatternsBlockId(Context& context, SemIR::LocId loc_id,
       auto tuple_pattern_type_id =
           GetPatternType(context, GetTupleType(context, param_type_ids));
       SemIR::InstId pattern_id = AddPatternInst(
-          context,
-          SemIR::LocIdAndInst::UncheckedLoc(
-              loc_id, SemIR::TuplePattern{.type_id = tuple_pattern_type_id,
-                                          .elements_id = param_block_id}));
+          context, SemIR::LocIdAndInst::RuntimeVerified(
+                       context.sem_ir(), loc_id,
+                       SemIR::TuplePattern{.type_id = tuple_pattern_type_id,
+                                           .elements_id = param_block_id}));
       param_ids = {pattern_id};
       break;
     }
@@ -1320,15 +1324,15 @@ static auto GetReturnInfo(Context& context, SemIR::LocId loc_id,
   if (auto init_form =
           context.insts().TryGetAs<SemIR::InitForm>(form_inst_id)) {
     auto param_pattern_id = AddPatternInst(
-        context, MakeImportedLocIdAndInst(
-                     context, return_type_import_ir_inst_id,
+        context, SemIR::LocIdAndInst::RuntimeVerified(
+                     context.sem_ir(), return_type_import_ir_inst_id,
                      SemIR::OutParamPattern(
                          {.type_id = pattern_type_id,
                           .pretty_name_id = SemIR::NameId::ReturnSlot})));
     SemIR::InstId return_slot_pattern_id = AddPatternInst(
         context,
-        MakeImportedLocIdAndInst(
-            context, return_type_import_ir_inst_id,
+        SemIR::LocIdAndInst::RuntimeVerified(
+            context.sem_ir(), return_type_import_ir_inst_id,
             SemIR::ReturnSlotPattern({.type_id = pattern_type_id,
                                       .subpattern_id = param_pattern_id,
                                       .type_inst_id = type_inst_id})));
@@ -1773,7 +1777,8 @@ static auto ImportTemplateDecl(Context& context,
   SemIR::StructValue value = {.type_id = SemIR::TypeId::None,
                               .elements_id = SemIR::InstBlockId::Empty};
   auto inst_id = AddPlaceholderImportedInstInNoBlock(
-      context, MakeImportedLocIdAndInst(context, import_loc_id, value));
+      context, SemIR::LocIdAndInst::RuntimeVerified(context.sem_ir(),
+                                                    import_loc_id, value));
 
   // Create a type for the constant value.
   auto name_id = context.entity_names().Add(

toolchain/check/cpp/operators.cpp

@@ -247,11 +247,12 @@ static auto MakeCppStdInitializerListMake(Context& context, SemIR::LocId loc_id,
                                    context, SemIR::IntLiteralType::TypeInstId),
                                .int_id = context.ints().Add(size)}));
   auto array_type_inst_id = AddTypeInst(
-      context, SemIR::LocIdAndInst::UncheckedLoc(
-                   loc_id, SemIR::ArrayType{
-                               .type_id = SemIR::TypeType::TypeId,
-                               .bound_id = bound_id,
-                               .element_type_inst_id = element_type_inst_id}));
+      context,
+      SemIR::LocIdAndInst::RuntimeVerified(
+          context.sem_ir(), loc_id,
+          SemIR::ArrayType{.type_id = SemIR::TypeType::TypeId,
+                           .bound_id = bound_id,
+                           .element_type_inst_id = element_type_inst_id}));
   auto array_type_id =
       context.types().GetTypeIdForTypeInstId(array_type_inst_id);
 

toolchain/check/function.cpp

@@ -454,7 +454,8 @@ auto MakeFunctionDecl(Context& context, SemIR::LocId loc_id,
   SemIR::FunctionDecl function_decl = {SemIR::TypeId::None,
                                        SemIR::FunctionId::None, decl_block_id};
   auto decl_id = AddPlaceholderInstInNoBlock(
-      context, SemIR::LocIdAndInst::UncheckedLoc(loc_id, function_decl));
+      context, SemIR::LocIdAndInst::RuntimeVerified(context.sem_ir(), loc_id,
+                                                    function_decl));
   function.first_owning_decl_id = decl_id;
   if (is_definition) {
     function.definition_id = decl_id;

toolchain/check/generic.cpp

@@ -134,10 +134,9 @@ class RebuildGenericConstantInEvalBlockCallbacks : public SubstInstCallbacks {
       // TODO: Add a function on `Context` to add the instruction without
       // inserting it into the dependent instructions list or computing a
       // constant value for it.
-      // TODO: Is the location we pick here always appropriate for the new
-      // instruction?
       auto inst_id = context().sem_ir().insts().AddInNoBlock(
-          SemIR::LocIdAndInst::UncheckedLoc(loc_id_, new_inst));
+          SemIR::LocIdAndInst::RuntimeVerified(context().sem_ir(), loc_id_,
+                                               new_inst));
       auto const_id = AddGenericConstantInstToEvalBlock(
           context(), const_inst_id, inst_id, dependence);
       context().constant_values().Set(inst_id, const_id);

toolchain/check/import.cpp

@@ -112,7 +112,8 @@ static auto MakeImportedNamespaceLocIdAndInst(Context& context,
   // If the import was itself imported, use its location.
   if (auto import_ir_inst_id = context.insts().GetImportSource(import_id);
       import_ir_inst_id.has_value()) {
-    return MakeImportedLocIdAndInst(context, import_ir_inst_id, namespace_inst);
+    return SemIR::LocIdAndInst::RuntimeVerified(
+        context.sem_ir(), import_ir_inst_id, namespace_inst);
   }
 
   // Otherwise we should have a node location for some kind of namespace
@@ -237,11 +238,12 @@ static auto CopySingleNameScopeFromImportIR(
     auto import_ir_inst_id = context.import_ir_insts().Add(
         SemIR::ImportIRInst(ir_id, import_inst_id));
     auto inst_id = AddInstInNoBlock(
-        context, MakeImportedLocIdAndInst<SemIR::ImportRefLoaded>(
-                     context, import_ir_inst_id,
-                     {.type_id = namespace_type_id,
-                      .import_ir_inst_id = import_ir_inst_id,
-                      .entity_name_id = entity_name_id}));
+        context,
+        SemIR::LocIdAndInst::RuntimeVerified(
+            context.sem_ir(), import_ir_inst_id,
+            SemIR::ImportRefLoaded{.type_id = namespace_type_id,
+                                   .import_ir_inst_id = import_ir_inst_id,
+                                   .entity_name_id = entity_name_id}));
     context.imports().push_back(inst_id);
     return inst_id;
   };
@@ -712,44 +714,4 @@ auto ImportNameFromOtherPackage(
   return result_id;
 }
 
-// Returns whether a parse node associated with an imported instruction of kind
-// `imported_kind` is usable as the location of a corresponding local
-// instruction of kind `local_kind`.
-static auto HasCompatibleImportedNodeKind(SemIR::InstKind imported_kind,
-                                          SemIR::InstKind local_kind) -> bool {
-  if (imported_kind == local_kind) {
-    return true;
-  }
-  if (imported_kind == SemIR::ImportDecl::Kind &&
-      local_kind == SemIR::Namespace::Kind) {
-    static_assert(
-        std::is_convertible_v<decltype(SemIR::ImportDecl::Kind)::TypedNodeId,
-                              decltype(SemIR::Namespace::Kind)::TypedNodeId>);
-    return true;
-  }
-  return false;
-}
-
-namespace Internal {
-
-auto CheckCompatibleImportedNodeKind(Context& context,
-                                     SemIR::ImportIRInstId imported_loc_id,
-                                     SemIR::InstKind kind) -> void {
-  auto& import_ir_inst = context.import_ir_insts().Get(imported_loc_id);
-  if (import_ir_inst.ir_id() == SemIR::ImportIRId::Cpp) {
-    // We don't require a matching node kind if the location is in C++, because
-    // there isn't a node.
-    return;
-  }
-  const auto* import_ir =
-      context.import_irs().Get(import_ir_inst.ir_id()).sem_ir;
-  auto imported_kind = import_ir->insts().Get(import_ir_inst.inst_id()).kind();
-  CARBON_CHECK(
-      HasCompatibleImportedNodeKind(imported_kind, kind),
-      "Node of kind {0} created with location of imported node of kind {1}",
-      kind, imported_kind);
-}
-
-}  // namespace Internal
-
 }  // namespace Carbon::Check

toolchain/check/import.h

@@ -93,30 +93,6 @@ auto ImportNameFromOtherPackage(
         import_ir_scopes,
     SemIR::NameId name_id) -> SemIR::InstId;
 
-namespace Internal {
-
-// Checks that the provided imported location has a node kind that is
-// compatible with that of the given instruction.
-auto CheckCompatibleImportedNodeKind(Context& context,
-                                     SemIR::ImportIRInstId imported_loc_id,
-                                     SemIR::InstKind kind) -> void;
-}  // namespace Internal
-
-// Returns a LocIdAndInst for an instruction with an imported location. Checks
-// that the imported location is compatible with the kind of instruction being
-// created.
-template <typename InstT>
-  requires SemIR::Internal::HasNodeId<InstT>
-auto MakeImportedLocIdAndInst(Context& context,
-                              SemIR::ImportIRInstId imported_loc_id, InstT inst)
-    -> SemIR::LocIdAndInst {
-  if constexpr (!SemIR::Internal::HasUntypedNodeId<InstT>) {
-    Internal::CheckCompatibleImportedNodeKind(context, imported_loc_id,
-                                              InstT::Kind);
-  }
-  return SemIR::LocIdAndInst::UncheckedLoc(SemIR::LocId(imported_loc_id), inst);
-}
-
 }  // namespace Carbon::Check
 
 #endif  // CARBON_TOOLCHAIN_CHECK_IMPORT_H_

toolchain/check/import_ref.cpp

@@ -94,8 +94,8 @@ auto AddImportRef(Context& context, SemIR::ImportIRInst import_ir_inst,
   auto import_ir_inst_id = context.import_ir_insts().Add(import_ir_inst);
   auto import_ref_id = AddPlaceholderImportedInstInNoBlock(
       context,
-      MakeImportedLocIdAndInst(
-          context, import_ir_inst_id,
+      SemIR::LocIdAndInst::RuntimeVerified(
+          context.sem_ir(), import_ir_inst_id,
           SemIR::ImportRefUnloaded{.import_ir_inst_id = import_ir_inst_id,
                                    .entity_name_id = entity_name_id}));
   return import_ref_id;
@@ -512,8 +512,8 @@ static auto AddLoadedImportRef(ImportContext& context,
                                  .entity_name_id = SemIR::EntityNameId::None};
   auto inst_id = AddPlaceholderImportedInstInNoBlock(
       context.local_context(),
-      MakeImportedLocIdAndInst(context.local_context(), import_ir_inst_id,
-                               inst));
+      SemIR::LocIdAndInst::RuntimeVerified(context.local_context().sem_ir(),
+                                           import_ir_inst_id, inst));
 
   context.local_constant_values().Set(inst_id, local_const_id);
   context.local_constant_values_for_import_insts().Set(import_inst_id,
@@ -558,8 +558,8 @@ static auto AddPlaceholderImportedInst(ImportContext& context,
   auto import_ir_inst_id = AddImportIRInst(context, import_inst_id);
   return AddPlaceholderImportedInstInNoBlock(
       context.local_context(),
-      MakeImportedLocIdAndInst(context.local_context(), import_ir_inst_id,
-                               inst));
+      SemIR::LocIdAndInst::RuntimeVerified(context.local_context().sem_ir(),
+                                           import_ir_inst_id, inst));
 }
 
 // Replace an imported instruction that was added by

toolchain/check/inst.cpp

@@ -8,6 +8,7 @@
 #include "toolchain/check/context.h"
 #include "toolchain/check/eval.h"
 #include "toolchain/check/generic.h"
+#include "toolchain/check/import.h"
 #include "toolchain/sem_ir/constant.h"
 #include "toolchain/sem_ir/ids.h"
 #include "toolchain/sem_ir/inst_kind.h"

toolchain/check/pattern.cpp

@@ -106,15 +106,16 @@ auto AddBindingPattern(Context& context, SemIR::LocId name_loc,
   auto type_id = SemIR::ExtractScrutineeType(context.sem_ir(), pattern.type_id);
 
   auto bind_id = AddInstInNoBlock(
-      context,
-      SemIR::LocIdAndInst::UncheckedLoc(
-          name_loc, SemIR::AnyBinding{.kind = bind_name_kind,
-                                      .type_id = type_id,
-                                      .entity_name_id = pattern.entity_name_id,
-                                      .value_id = SemIR::InstId::None}));
+      context, SemIR::LocIdAndInst::RuntimeVerified(
+                   context.sem_ir(), name_loc,
+                   SemIR::AnyBinding{.kind = bind_name_kind,
+                                     .type_id = type_id,
+                                     .entity_name_id = pattern.entity_name_id,
+                                     .value_id = SemIR::InstId::None}));
 
-  auto binding_pattern_id = AddPatternInst(
-      context, SemIR::LocIdAndInst::UncheckedLoc(name_loc, pattern));
+  auto binding_pattern_id =
+      AddPatternInst(context, SemIR::LocIdAndInst::RuntimeVerified(
+                                  context.sem_ir(), name_loc, pattern));
 
   if (pattern.kind == SemIR::SymbolicBindingPattern::Kind) {
     context.scope_stack().PushCompileTimeBinding(bind_id);
@@ -173,11 +174,11 @@ auto AddParamPattern(Context& context, SemIR::LocId loc_id,
   const auto& param_pattern_kind =
       is_ref ? SemIR::RefParamPattern::Kind : SemIR::ValueParamPattern::Kind;
   auto pattern_id = AddPatternInst(
-      context,
-      SemIR::LocIdAndInst::UncheckedLoc(
-          loc_id, SemIR::AnyLeafParamPattern{.kind = param_pattern_kind,
-                                             .type_id = pattern_type_id,
-                                             .pretty_name_id = name_id}));
+      context, SemIR::LocIdAndInst::RuntimeVerified(
+                   context.sem_ir(), loc_id,
+                   SemIR::AnyLeafParamPattern{.kind = param_pattern_kind,
+                                              .type_id = pattern_type_id,
+                                              .pretty_name_id = name_id}));
 
   auto entity_name_id =
       AddBindingEntityName(context, name_id,

toolchain/check/subst.cpp

@@ -23,8 +23,9 @@ auto SubstInstCallbacks::RebuildType(SemIR::TypeInstId type_inst_id) const
 auto SubstInstCallbacks::RebuildNewInst(SemIR::LocId loc_id,
                                         SemIR::Inst new_inst) const
     -> SemIR::InstId {
-  auto const_id = EvalOrAddInst(
-      context(), SemIR::LocIdAndInst::UncheckedLoc(loc_id, new_inst));
+  auto const_id =
+      EvalOrAddInst(context(), SemIR::LocIdAndInst::RuntimeVerified(
+                                   context().sem_ir(), loc_id, new_inst));
   CARBON_CHECK(const_id.has_value(),
                "Substitution into constant produced non-constant");
   CARBON_CHECK(const_id.is_constant(),

toolchain/check/thunk.cpp

@@ -41,8 +41,9 @@ static auto RebuildPatternInst(Context& context, SemIR::InstId orig_inst_id,
   CARBON_CHECK(context.insts().Get(orig_inst_id).kind() == new_inst.kind(),
                "Rebuilt pattern with the wrong kind: {0} -> {1}",
                context.insts().Get(orig_inst_id), new_inst);
-  return AddPatternInst(context, SemIR::LocIdAndInst::UncheckedLoc(
-                                     SemIR::LocId(orig_inst_id), new_inst));
+  return AddPatternInst(
+      context, SemIR::LocIdAndInst::RuntimeVerified(
+                   context.sem_ir(), SemIR::LocId(orig_inst_id), new_inst));
 }
 
 // Wrapper to allow the type to be specified as a template argument for API

toolchain/sem_ir/inst.cpp

@@ -61,4 +61,66 @@ auto InstStore::GetUnattachedType(TypeId type_id) const -> TypeId {
   return file_->types().GetUnattachedType(type_id);
 }
 
+// Returns whether a parse node associated with an imported instruction of kind
+// `imported_kind` is usable as the location of a corresponding local
+// instruction of kind `local_kind`.
+static auto HasCompatibleImportedNodeKind(InstKind imported_kind,
+                                          InstKind local_kind) -> bool {
+  if (imported_kind == local_kind) {
+    return true;
+  }
+  if (imported_kind == ImportDecl::Kind && local_kind == Namespace::Kind) {
+    static_assert(
+        std::is_convertible_v<decltype(ImportDecl::Kind)::TypedNodeId,
+                              decltype(Namespace::Kind)::TypedNodeId>);
+    return true;
+  }
+  return false;
+}
+
+auto LocIdAndInst::RuntimeVerified(const File& file, LocId loc_id, Inst inst)
+    -> LocIdAndInst {
+  switch (loc_id.kind()) {
+    case LocId::Kind::ImportIRInstId: {
+      CARBON_CHECK(!inst.kind().disallow_all_node_kinds(),
+                   "Should never import builtins/singletons: {0}", inst);
+
+      if (inst.kind().allow_all_node_kinds()) {
+        break;
+      }
+
+      const auto& import_ir_inst =
+          file.import_ir_insts().Get(loc_id.import_ir_inst_id());
+      // We don't require a matching node kind if the location is in C++,
+      // because there isn't a node.
+      if (import_ir_inst.ir_id() == ImportIRId::Cpp) {
+        break;
+      }
+      const auto* import_ir =
+          file.import_irs().Get(import_ir_inst.ir_id()).sem_ir;
+      auto imported_kind =
+          import_ir->insts().Get(import_ir_inst.inst_id()).kind();
+      CARBON_CHECK(HasCompatibleImportedNodeKind(imported_kind, inst.kind()),
+                   "Unexpected imported `InstKind` {0} for {1}", imported_kind,
+                   inst);
+      break;
+    }
+
+    case LocId::Kind::InstId:
+      // TODO: Figure out right verification.
+      break;
+
+    case LocId::Kind::NodeId: {
+      auto node_kind = file.parse_tree().node_kind(loc_id.node_id());
+      CARBON_CHECK(inst.kind().IsAllowedNodeKind(node_kind),
+                   "Unexpected `NodeKind` {0} for {1}", node_kind, inst);
+      break;
+    }
+
+    case LocId::Kind::None:
+      break;
+  }
+  return LocIdAndInst(loc_id, inst, /*is_unchecked=*/true);
+}
+
 }  // namespace Carbon::SemIR

toolchain/sem_ir/inst.h

@@ -28,6 +28,8 @@ namespace Carbon::SemIR {
 template <typename... TypedInsts>
 struct CategoryOf;
 
+class File;
+
 // InstLikeTypeInfo is an implementation detail, and not public API.
 namespace Internal {
 
@@ -417,15 +419,12 @@ struct LocIdAndInst {
     return LocIdAndInst(LocId::None, inst, /*is_unchecked=*/true);
   }
 
-  // Unsafely form a pair of a location and an instruction. Used in the cases
-  // where we can't statically enforce the type matches. For `ImportIRInstId`,
-  // use `MakeImportedLocIdAndInst` in `import.h`.
-  template <typename LocT>
-    requires(std::convertible_to<LocT, LocId> &&
-             !std::same_as<LocT, ImportIRInstId>)
-  static auto UncheckedLoc(LocT loc_id, Inst inst) -> LocIdAndInst {
-    return LocIdAndInst(loc_id, inst, /*is_unchecked=*/true);
-  }
+  // Constructs a `LocIdAndInst` with a runtime verification of the location.
+  //
+  // Prefer `LocIdAndInst` constructors with compile-time verification,
+  // or `AddInst` overloads which make use of those constructors.
+  static auto RuntimeVerified(const File& file, LocId loc_id, Inst inst)
+      -> LocIdAndInst;
 
   // Construction for the common case with a typed node.
   template <typename InstT>
@@ -439,7 +438,7 @@ struct LocIdAndInst {
     requires(Internal::HasUntypedNodeId<InstT>)
   LocIdAndInst(LocId loc_id, InstT inst) : loc_id(loc_id), inst(inst) {}
 
-  // For `ImportIRInstId`, use `MakeImportedLocIdAndInst` in `import.h`.
+  // For `ImportIRInstId`, use `RuntimeVerified`.
   template <typename InstT>
   LocIdAndInst(ImportIRInstId loc_id, InstT inst) = delete;
 
@@ -447,6 +446,9 @@ struct LocIdAndInst {
   Inst inst;
 
  private:
+  // For `InstStore::GetWithLocId` to construct unchecked values.
+  friend class InstStore;
+
   // Note `is_unchecked` serves to disambiguate from public constructors.
   explicit LocIdAndInst(LocId loc_id, Inst inst, bool /*is_unchecked*/)
       : loc_id(loc_id), inst(inst) {}
@@ -503,7 +505,7 @@ class InstStore {
 
   // Returns the requested instruction and its location ID.
   auto GetWithLocId(InstId inst_id) const -> LocIdAndInst {
-    return LocIdAndInst::UncheckedLoc(LocId(inst_id), Get(inst_id));
+    return LocIdAndInst(LocId(inst_id), Get(inst_id), /*is_unchecked=*/true);
   }
 
   // Returns whether the requested instruction is the specified type.

toolchain/sem_ir/inst_kind.cpp

@@ -29,4 +29,12 @@ auto InstKind::has_type() const -> bool {
   return Table[AsInt()];
 }
 
+auto InstKind::IsAllowedNodeKind(Parse::NodeKind node_kind) const -> bool {
+  const auto& def = definition_info(*this);
+  if (def.internal_allow_all_node_kinds) {
+    return true;
+  }
+  return llvm::is_contained(def.internal_allowed_node_kinds, node_kind);
+}
+
 }  // namespace Carbon::SemIR

toolchain/sem_ir/inst_kind.h

@@ -5,6 +5,7 @@
 #ifndef CARBON_TOOLCHAIN_SEM_IR_INST_KIND_H_
 #define CARBON_TOOLCHAIN_SEM_IR_INST_KIND_H_
 
+#include <concepts>
 #include <cstdint>
 #include <optional>
 
@@ -290,6 +291,13 @@ class InstKind : public CARBON_ENUM_BASE(InstKind) {
     bool is_lowered = true;
     bool deduce_through = false;
     bool has_cleanup = false;
+
+    // The inst's allowed node kinds, for `IsAllowedNodeKind`.
+    //
+    // Do not set these directly. They are set by the `TypedNodeId` template
+    // parameter of `Define`.
+    bool internal_allow_all_node_kinds = false;
+    llvm::ArrayRef<Parse::NodeKind::RawEnumType> internal_allowed_node_kinds;
   };
 
   // Provides a definition for this instruction kind. Should only be called
@@ -365,6 +373,21 @@ class InstKind : public CARBON_ENUM_BASE(InstKind) {
     return definition_info(*this).has_cleanup;
   }
 
+  // Returns true if the passed `NodeKind` is allowed.
+  auto IsAllowedNodeKind(Parse::NodeKind node_kind) const -> bool;
+
+  // Returns true if all `NodeKind`s are allowed.
+  auto allow_all_node_kinds() const -> bool {
+    return definition_info(*this).internal_allow_all_node_kinds;
+  }
+
+  // Returns true if no `NodeKind`s are allowed.
+  auto disallow_all_node_kinds() const -> bool {
+    const auto& def = definition_info(*this);
+    return !def.internal_allow_all_node_kinds &&
+           def.internal_allowed_node_kinds.empty();
+  }
+
  private:
   // Returns the DefinitionInfo for the kind.
   static auto definition_info(InstKind kind) -> const DefinitionInfo&;
@@ -453,9 +476,44 @@ class InstKind::Definition : public InstKind {
   InstKind::DefinitionInfo info_;
 };
 
+namespace Internal {
+
+// Storage for `internal_allowed_node_kinds` where there's a list of kinds.
+template <Parse::NodeKind::RawEnumType... T>
+constexpr std::array<Parse::NodeKind::RawEnumType, sizeof...(T)> Kinds = {T...};
+
+// `NoneNodeId` uses should never have a node associated; it's mainly for
+// builtins.
+constexpr auto GetAllowedNodeKinds(Parse::NoneNodeId* /*unused*/)
+    -> llvm::ArrayRef<Parse::NodeKind::RawEnumType> {
+  return {};
+}
+
+// For a regular `NodeId`, returns an array of just its kind.
+template <const Parse::NodeKind& Kind>
+constexpr auto GetAllowedNodeKinds(Parse::NodeIdForKind<Kind>* /*unused*/)
+    -> llvm::ArrayRef<Parse::NodeKind::RawEnumType> {
+  return Kinds<static_cast<Parse::NodeKind::RawEnumType>(Kind)>;
+}
+
+// For `NodeIdOneOf`, returns an array of each kind.
+template <typename... T>
+constexpr auto GetAllowedNodeKinds(Parse::NodeIdOneOf<T...>* /*unused*/)
+    -> llvm::ArrayRef<Parse::NodeKind::RawEnumType> {
+  return Kinds<T::Kind...>;
+}
+
+}  // namespace Internal
+
 template <typename TypedNodeId>
 constexpr auto InstKind::Define(DefinitionInfo info) const
     -> Definition<TypedNodeId> {
+  if constexpr (std::same_as<Parse::NodeId, TypedNodeId>) {
+    info.internal_allow_all_node_kinds = true;
+  } else {
+    info.internal_allowed_node_kinds =
+        Internal::GetAllowedNodeKinds(static_cast<TypedNodeId*>(nullptr));
+  }
   return Definition<TypedNodeId>(*this, info);
 }
 

toolchain/sem_ir/typed_insts.h

@@ -749,8 +749,10 @@ struct FloatValue {
 // A form binding, such as the `x` declared by `x:? F`. See `AnyBinding` for
 // member documentation.
 struct FormBinding {
+  // TODO: Should this have been associated with the `FormBindingPatternId`
+  // instead?
   static constexpr auto Kind =
-      InstKind::FormBinding.Define<Parse::FormBindingPatternId>(
+      InstKind::FormBinding.Define<Parse::IdentifierNameNotBeforeSignatureId>(
           {.ir_name = "form_binding",
            .expr_category = ComputedExprCategory::DependsOnOperands,
            .constant_kind = InstConstantKind::Never});
@@ -763,12 +765,15 @@ struct FormBinding {
 // A form binding pattern, such as `x:? F`, that is not a parameter. See
 // `AnyBindingPattern` for member documentation.
 struct FormBindingPattern {
+  // TODO: Should this have been associated with the `FormBindingPatternId`
+  // instead?
   static constexpr auto Kind =
-      InstKind::FormBindingPattern.Define<Parse::FormBindingPatternId>(
-          {.ir_name = "form_binding_pattern",
-           .expr_category = ExprCategory::Pattern,
-           .constant_kind = InstConstantKind::AlwaysUnique,
-           .is_lowered = false});
+      InstKind::FormBindingPattern
+          .Define<Parse::IdentifierNameNotBeforeSignatureId>(
+              {.ir_name = "form_binding_pattern",
+               .expr_category = ExprCategory::Pattern,
+               .constant_kind = InstConstantKind::AlwaysUnique,
+               .is_lowered = false});
 
   TypeId type_id;
   // Note that the EntityName's `form_id` represents the scrutinee form, so it