Get rid of `AddPatternInst` (#7075)

Instead, use the inst category to select the right block stack. This
simplifies the API for adding insts, and in subsequent changes it will
enable certain inst kinds like `SpliceInst` to seamlessly function as
either procedural insts or pattern insts.

toolchain/check/cpp/import.cpp

@@ -1218,7 +1218,7 @@ static auto MakeParamPatternsBlockId(Context& context, SemIR::LocId loc_id,
       auto param_block_id = context.inst_blocks().Add(param_ids);
       auto tuple_pattern_type_id =
           GetPatternType(context, GetTupleType(context, param_type_ids));
-      SemIR::InstId pattern_id = AddPatternInst(
+      SemIR::InstId pattern_id = AddInst(
           context, SemIR::LocIdAndInst::RuntimeVerified(
                        context.sem_ir(), loc_id,
                        SemIR::TuplePattern{.type_id = tuple_pattern_type_id,
@@ -1331,19 +1331,19 @@ static auto GetReturnInfo(Context& context, SemIR::LocId loc_id,
   auto return_patterns_id = SemIR::InstBlockId::Empty;
   if (auto init_form =
           context.insts().TryGetAs<SemIR::InitForm>(form_inst_id)) {
-    auto param_pattern_id = AddPatternInst(
+    auto param_pattern_id = AddInst(
         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,
-        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})));
+    SemIR::InstId return_slot_pattern_id =
+        AddInst(context,
+                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})));
     return_patterns_id = context.inst_blocks().Add({return_slot_pattern_id});
   }
   return {.return_type_inst_id = type_inst_id,

toolchain/check/function.cpp

@@ -45,11 +45,11 @@ auto AddReturnPatterns(Context& context, SemIR::LocId loc_id,
     case CARBON_KIND(SemIR::InitForm _): {
       auto pattern_type_id =
           GetPatternType(context, form_expr.type_component_id);
-      auto out_param_id = AddPatternInst<SemIR::OutParamPattern>(
+      auto out_param_id = AddInst<SemIR::OutParamPattern>(
           context, SemIR::LocId(form_expr.form_inst_id),
           {.type_id = pattern_type_id,
            .pretty_name_id = SemIR::NameId::ReturnSlot});
-      return_patterns.push_back(AddPatternInst<SemIR::ReturnSlotPattern>(
+      return_patterns.push_back(AddInst<SemIR::ReturnSlotPattern>(
           context, SemIR::LocId(form_expr.form_inst_id),
           {.type_id = pattern_type_id,
            .subpattern_id = out_param_id,

toolchain/check/handle_binding_pattern.cpp

@@ -297,17 +297,17 @@ static auto HandleAnyBindingPattern(Context& context, Parse::NodeId node_id,
           auto pattern_type_id =
               GetPatternType(context, type_expr.type_component_id);
           if (is_ref) {
-            param_pattern_id = AddPatternInst<SemIR::RefParamPattern>(
+            param_pattern_id = AddInst<SemIR::RefParamPattern>(
                 context, node_id,
                 {.type_id = pattern_type_id, .pretty_name_id = name_id});
           } else if (node_kind == Parse::NodeKind::FormBindingPattern) {
-            param_pattern_id = AddPatternInst<SemIR::FormParamPattern>(
-                context, node_id,
-                {.type_id = pattern_type_id,
-                 .pretty_name_id = name_id,
-                 .form_id = form_id});
+            param_pattern_id =
+                AddInst<SemIR::FormParamPattern>(context, node_id,
+                                                 {.type_id = pattern_type_id,
+                                                  .pretty_name_id = name_id,
+                                                  .form_id = form_id});
           } else {
-            param_pattern_id = AddPatternInst<SemIR::ValueParamPattern>(
+            param_pattern_id = AddInst<SemIR::ValueParamPattern>(
                 context, node_id,
                 {.type_id = pattern_type_id, .pretty_name_id = name_id});
           }

toolchain/check/handle_let_and_var.cpp

@@ -111,12 +111,12 @@ auto HandleParseNode(Context& context, Parse::VariablePatternId node_id)
   switch (context.full_pattern_stack().CurrentKind()) {
     case FullPatternStack::Kind::ExplicitParamList:
     case FullPatternStack::Kind::ImplicitParamList:
-      pattern_id = AddPatternInst<SemIR::VarParamPattern>(
+      pattern_id = AddInst<SemIR::VarParamPattern>(
           context, node_id,
           {.type_id = type_id, .subpattern_id = subpattern_id});
       break;
     case FullPatternStack::Kind::NameBindingDecl:
-      pattern_id = AddPatternInst<SemIR::VarPattern>(
+      pattern_id = AddInst<SemIR::VarPattern>(
           context, node_id,
           {.type_id = type_id, .subpattern_id = subpattern_id});
       break;

toolchain/check/handle_pattern_list.cpp

@@ -102,7 +102,7 @@ auto HandleParseNode(Context& context, Parse::TuplePatternId node_id) -> bool {
   auto type_id = GetPatternType(context, GetTupleType(context, type_inst_ids));
   context.node_stack().Push(
       node_id,
-      AddPatternInst<SemIR::TuplePattern>(
+      AddInst<SemIR::TuplePattern>(
           context, node_id, {.type_id = type_id, .elements_id = refs_id}));
   return true;
 }

toolchain/check/inst.cpp

@@ -10,6 +10,7 @@
 #include "toolchain/check/generic.h"
 #include "toolchain/check/import.h"
 #include "toolchain/sem_ir/constant.h"
+#include "toolchain/sem_ir/expr_info.h"
 #include "toolchain/sem_ir/ids.h"
 #include "toolchain/sem_ir/inst_kind.h"
 
@@ -58,7 +59,15 @@ static auto FinishInst(Context& context, SemIR::InstId inst_id,
 auto AddInst(Context& context, SemIR::LocIdAndInst loc_id_and_inst)
     -> SemIR::InstId {
   auto inst_id = AddInstInNoBlock(context, loc_id_and_inst);
-  context.inst_block_stack().AddInstId(inst_id);
+  if (SemIR::GetExprCategory(context.sem_ir(), inst_id) ==
+      SemIR::ExprCategory::Pattern) {
+    auto type_id = loc_id_and_inst.inst.type_id();
+    CARBON_CHECK(type_id == SemIR::ErrorInst::TypeId ||
+                 context.types().Is<SemIR::PatternType>(type_id));
+    context.pattern_block_stack().AddInstId(inst_id);
+  } else {
+    context.inst_block_stack().AddInstId(inst_id);
+  }
   return inst_id;
 }
 
@@ -91,16 +100,6 @@ auto AddDependentActionInst(Context& context,
   return inst_id;
 }
 
-auto AddPatternInst(Context& context, SemIR::LocIdAndInst loc_id_and_inst)
-    -> SemIR::InstId {
-  auto type_id = loc_id_and_inst.inst.type_id();
-  CARBON_CHECK(type_id == SemIR::ErrorInst::TypeId ||
-               context.types().Is<SemIR::PatternType>(type_id));
-  auto inst_id = AddInstInNoBlock(context, loc_id_and_inst);
-  context.pattern_block_stack().AddInstId(inst_id);
-  return inst_id;
-}
-
 auto GetOrAddInst(Context& context, SemIR::LocIdAndInst loc_id_and_inst)
     -> SemIR::InstId {
   CARBON_CHECK(!loc_id_and_inst.inst.kind().has_cleanup());

toolchain/check/inst.h

@@ -132,23 +132,6 @@ auto AddDependentActionTypeInst(Context& context, LocT loc, InstT inst)
       AddDependentActionInst(context, loc, inst));
 }
 
-// Adds an instruction to the current pattern block, returning the produced
-// ID.
-// TODO: Is it possible to remove this and pattern_block_stack, now that
-// we have BeginSubpattern etc. instead?
-auto AddPatternInst(Context& context, SemIR::LocIdAndInst loc_id_and_inst)
-    -> SemIR::InstId;
-
-// Convenience for AddPatternInst with typed nodes.
-//
-// As a safety check, prevent use with storage insts (see `AddInstWithCleanup`).
-template <typename InstT, typename LocT>
-  requires(!InstT::Kind.has_cleanup() &&
-           std::convertible_to<LocT, SemIR::LocId>)
-auto AddPatternInst(Context& context, LocT loc, InstT inst) -> SemIR::InstId {
-  return AddPatternInst(context, SemIR::LocIdAndInst(loc, inst));
-}
-
 // Adds an instruction to the current block, returning the produced ID. The
 // instruction is a placeholder that is expected to be replaced by
 // `ReplaceInstBeforeConstantUse`.

toolchain/check/pattern.cpp

@@ -69,7 +69,7 @@ auto EndSubpattern(Context& context, NodeStack& node_stack) -> void {
     auto expr_region_id = PopSubpatternExpr(context, *maybe_expr_id);
     auto pattern_type_id =
         GetPatternType(context, context.insts().Get(*maybe_expr_id).type_id());
-    node_stack.Push(node_id, AddPatternInst<SemIR::ExprPattern>(
+    node_stack.Push(node_id, AddInst<SemIR::ExprPattern>(
                                  context, node_id,
                                  {.type_id = pattern_type_id,
                                   .expr_region_id = expr_region_id}));
@@ -147,8 +147,8 @@ auto AddBindingPattern(Context& context, SemIR::LocId name_loc,
                                      .value_id = SemIR::InstId::None}));
 
   auto binding_pattern_id =
-      AddPatternInst(context, SemIR::LocIdAndInst::RuntimeVerified(
-                                  context.sem_ir(), name_loc, pattern));
+      AddInst(context, SemIR::LocIdAndInst::RuntimeVerified(context.sem_ir(),
+                                                            name_loc, pattern));
 
   if (pattern.kind == SemIR::SymbolicBindingPattern::Kind) {
     context.scope_stack().PushCompileTimeBinding(bind_id);
@@ -206,7 +206,7 @@ auto AddParamPattern(Context& context, SemIR::LocId loc_id,
   auto pattern_type_id = GetPatternType(context, type_id);
   const auto& param_pattern_kind =
       is_ref ? SemIR::RefParamPattern::Kind : SemIR::ValueParamPattern::Kind;
-  auto pattern_id = AddPatternInst(
+  auto pattern_id = AddInst(
       context, SemIR::LocIdAndInst::RuntimeVerified(
                    context.sem_ir(), loc_id,
                    SemIR::AnyLeafParamPattern{.kind = param_pattern_kind,

toolchain/check/thunk.cpp

@@ -41,9 +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::RuntimeVerified(
-                   context.sem_ir(), SemIR::LocId(orig_inst_id), new_inst));
+  return AddInst(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/docs/check/pattern_matching.md

@@ -77,8 +77,8 @@ any other expression.
 All the pattern instructions for a given full-pattern are grouped together in a
 distinct block that contains only pattern instructions. Consequently,
 `Check::Context` maintains `pattern_block_stack` as a separate `InstBlockStack`
-for pattern blocks, and provides separate methods like `AddPatternInst` for
-adding instructions to it.
+for pattern blocks, and operations like `AddInst` automatically put
+newly-created pattern insts on that stack.
 
 ## Instruction ordering