Restructure FormatInstRhs to allow for better logic sharing (#5494)

Taking a stab at restructuring towards allowing better reuse. Some of
that is with `AnyAggregateInit` and `AnyImportRef`. Some with
`FormatDeclRhs`.

This adds a `FormatArg` dispatch table so that `FormatInstRhs` doesn't
rely as heavily on templating. I'm mixed on the intermediate result -- a
step further might be to change `FormatArg` to dispatch to a
`FormatArgAndKind` that could use a switch. But, figured I'd check in on
the general direction.

Note this kind of direction opens up changing `FormatInst` to not be
templated, too, removing the `#define CARBON_SEM_IR_INST_KIND(InstT)`
variant of `FormatInst`.

toolchain/sem_ir/BUILD

@@ -182,6 +182,7 @@ cc_library(
         ":file",
         ":inst_namer",
         ":typed_insts",
+        "//common:concepts",
         "//common:ostream",
         "//toolchain/base:kind_switch",
         "//toolchain/base:shared_value_stores",

toolchain/sem_ir/formatter.cpp

@@ -756,6 +756,40 @@ auto Formatter::FormatInst(InstId inst_id, Inst inst) -> void {
   }
 }
 
+auto Formatter::FormatInst(InstId /*inst_id*/, Branch inst) -> void {
+  if (!in_terminator_sequence_) {
+    Indent();
+  }
+  out_ << Branch::Kind.ir_name() << " ";
+  FormatLabel(inst.target_id);
+  out_ << "\n";
+  in_terminator_sequence_ = false;
+}
+
+auto Formatter::FormatInst(InstId /*inst_id*/, BranchIf inst) -> void {
+  if (!in_terminator_sequence_) {
+    Indent();
+  }
+  out_ << "if ";
+  FormatName(inst.cond_id);
+  out_ << " " << Branch::Kind.ir_name() << " ";
+  FormatLabel(inst.target_id);
+  out_ << " else ";
+  in_terminator_sequence_ = true;
+}
+
+auto Formatter::FormatInst(InstId /*inst_id*/, BranchWithArg inst) -> void {
+  if (!in_terminator_sequence_) {
+    Indent();
+  }
+  out_ << BranchWithArg::Kind.ir_name() << " ";
+  FormatLabel(inst.target_id);
+  out_ << "(";
+  FormatName(inst.arg_id);
+  out_ << ")\n";
+  in_terminator_sequence_ = false;
+}
+
 auto Formatter::FormatInst(InstId inst_id, ImportRefUnloaded inst) -> void {
   Indent();
   FormatInstLhs(inst_id, inst);
@@ -862,64 +896,223 @@ auto Formatter::FormatInstLhs(InstId inst_id, Inst inst) -> void {
   out_ << " = ";
 }
 
-auto Formatter::FormatInstRhs(BindSymbolicName inst) -> void {
-  // A BindSymbolicName with no value is a purely symbolic binding, such as
-  // the `Self` in an interface. Don't print out `none` for the value.
-  if (inst.value_id.has_value()) {
-    FormatArgs(inst.entity_name_id, inst.value_id);
-  } else {
-    FormatArgs(inst.entity_name_id);
-  }
+auto Formatter::FormatInstArgAndKind(Inst::ArgAndKind arg_and_kind) -> void {
+  static constexpr auto Table =
+      MakeFormatArgFnTable(static_cast<SemIR::IdKind*>(nullptr));
+  Table[arg_and_kind.kind().ToIndex()](*this, arg_and_kind.value());
 }
 
-auto Formatter::FormatInstRhs(BlockArg inst) -> void {
-  out_ << " ";
-  FormatLabel(inst.block_id);
-}
+auto Formatter::FormatInstRhs(Inst inst) -> void {
+  CARBON_KIND_SWITCH(inst) {
+    case SemIR::InstKind::ArrayInit:
+    case SemIR::InstKind::StructInit:
+    case SemIR::InstKind::TupleInit: {
+      auto init = inst.As<AnyAggregateInit>();
+      FormatArgs(init.elements_id);
+      FormatReturnSlotArg(init.dest_id);
+      return;
+    }
 
-auto Formatter::FormatInstRhs(Namespace inst) -> void {
-  if (inst.import_id.has_value()) {
-    FormatArgs(inst.import_id, inst.name_scope_id);
-  } else {
-    FormatArgs(inst.name_scope_id);
+    case SemIR::InstKind::ImportRefLoaded:
+    case SemIR::InstKind::ImportRefUnloaded:
+      FormatImportRefRhs(inst.As<AnyImportRef>());
+      return;
+
+    case SemIR::InstKind::OutParam:
+    case SemIR::InstKind::RefParam:
+    case SemIR::InstKind::ValueParam: {
+      auto param = inst.As<AnyParam>();
+      FormatArgs(param.index);
+      // Omit pretty_name because it's an implementation detail of
+      // pretty-printing.
+      return;
+    }
+
+    case CARBON_KIND(AssociatedConstantDecl decl): {
+      FormatArgs(decl.assoc_const_id);
+      llvm::SaveAndRestore scope(scope_,
+                                 inst_namer_.GetScopeFor(decl.assoc_const_id));
+      FormatTrailingBlock(decl.decl_block_id);
+      return;
+    }
+
+    case CARBON_KIND(BindSymbolicName bind): {
+      // A BindSymbolicName with no value is a purely symbolic binding, such as
+      // the `Self` in an interface. Don't print out `none` for the value.
+      if (bind.value_id.has_value()) {
+        FormatArgs(bind.entity_name_id, bind.value_id);
+      } else {
+        FormatArgs(bind.entity_name_id);
+      }
+      return;
+    }
+
+    case CARBON_KIND(BlockArg block): {
+      out_ << " ";
+      FormatLabel(block.block_id);
+      return;
+    }
+
+    case CARBON_KIND(Call call): {
+      FormatCallRhs(call);
+      return;
+    }
+
+    case CARBON_KIND(ClassDecl decl): {
+      FormatDeclRhs(decl.class_id,
+                    sem_ir_->classes().Get(decl.class_id).pattern_block_id,
+                    decl.decl_block_id);
+      return;
+    }
+
+    case CARBON_KIND(FloatLiteral value): {
+      llvm::SmallVector<char, 16> buffer;
+      sem_ir_->floats().Get(value.float_id).toString(buffer);
+      out_ << " " << buffer;
+      return;
+    }
+
+    case CARBON_KIND(FunctionDecl decl): {
+      FormatDeclRhs(decl.function_id,
+                    sem_ir_->functions().Get(decl.function_id).pattern_block_id,
+                    decl.decl_block_id);
+      return;
+    }
+
+    case InstKind::ImportCppDecl: {
+      FormatImportCppDeclRhs();
+      return;
+    }
+
+    case CARBON_KIND(ImplDecl decl): {
+      FormatDeclRhs(decl.impl_id,
+                    sem_ir_->impls().Get(decl.impl_id).pattern_block_id,
+                    decl.decl_block_id);
+      return;
+    }
+
+    case CARBON_KIND(InitializeFrom init): {
+      FormatArgs(init.src_id);
+      FormatReturnSlotArg(init.dest_id);
+      return;
+    }
+
+    case CARBON_KIND(InstValue inst): {
+      out_ << ' ';
+      OpenBrace();
+      // TODO: Should we use a more compact representation in the case where the
+      // inst is a SpliceBlock?
+      FormatInst(inst.inst_id);
+      CloseBrace();
+      return;
+    }
+
+    case CARBON_KIND(InterfaceDecl decl): {
+      FormatDeclRhs(
+          decl.interface_id,
+          sem_ir_->interfaces().Get(decl.interface_id).pattern_block_id,
+          decl.decl_block_id);
+      return;
+    }
+
+    case CARBON_KIND(IntValue value): {
+      out_ << " ";
+      sem_ir_->ints()
+          .Get(value.int_id)
+          .print(out_, sem_ir_->types().IsSignedInt(value.type_id));
+      return;
+    }
+
+    case CARBON_KIND(NameBindingDecl name): {
+      FormatTrailingBlock(name.pattern_block_id);
+      return;
+    }
+
+    case CARBON_KIND(Namespace ns): {
+      if (ns.import_id.has_value()) {
+        FormatArgs(ns.import_id, ns.name_scope_id);
+      } else {
+        FormatArgs(ns.name_scope_id);
+      }
+      return;
+    }
+
+    case CARBON_KIND(ReturnExpr ret): {
+      FormatArgs(ret.expr_id);
+      if (ret.dest_id.has_value()) {
+        FormatReturnSlotArg(ret.dest_id);
+      }
+      return;
+    }
+
+    case CARBON_KIND(ReturnSlot ret): {
+      // Omit inst.type_inst_id because it's not semantically significant.
+      FormatArgs(ret.storage_id);
+      return;
+    }
+
+    case InstKind::ReturnSlotPattern:
+      // No-op because type_id is the only semantically significant field,
+      // and it's handled separately.
+      return;
+
+    case CARBON_KIND(SpliceBlock splice): {
+      FormatArgs(splice.result_id);
+      FormatTrailingBlock(splice.block_id);
+      return;
+    }
+
+    case CARBON_KIND(StructType struct_type): {
+      out_ << " {";
+      llvm::ListSeparator sep;
+      for (auto field :
+           sem_ir_->struct_type_fields().Get(struct_type.fields_id)) {
+        out_ << sep << ".";
+        FormatName(field.name_id);
+        out_ << ": ";
+        FormatInstAsType(field.type_inst_id);
+      }
+      out_ << "}";
+      return;
+    }
+
+    case CARBON_KIND(WhereExpr where): {
+      FormatArgs(where.period_self_id);
+      FormatTrailingBlock(where.requirements_id);
+      return;
+    }
+
+    default:
+      FormatInstRhsDefault(inst);
+      return;
   }
 }
 
-auto Formatter::FormatInst(InstId /*inst_id*/, BranchIf inst) -> void {
-  if (!in_terminator_sequence_) {
-    Indent();
+auto Formatter::FormatInstRhsDefault(Inst inst) -> void {
+  auto arg0 = inst.arg0_and_kind();
+  if (arg0.kind() == IdKind::None) {
+    return;
   }
-  out_ << "if ";
-  FormatName(inst.cond_id);
-  out_ << " " << Branch::Kind.ir_name() << " ";
-  FormatLabel(inst.target_id);
-  out_ << " else ";
-  in_terminator_sequence_ = true;
-}
+  out_ << " ";
+  FormatInstArgAndKind(arg0);
 
-auto Formatter::FormatInst(InstId /*inst_id*/, BranchWithArg inst) -> void {
-  if (!in_terminator_sequence_) {
-    Indent();
+  auto arg1 = inst.arg1_and_kind();
+  if (arg1.kind() == IdKind::None) {
+    return;
   }
-  out_ << BranchWithArg::Kind.ir_name() << " ";
-  FormatLabel(inst.target_id);
-  out_ << "(";
-  FormatName(inst.arg_id);
-  out_ << ")\n";
-  in_terminator_sequence_ = false;
-}
 
-auto Formatter::FormatInst(InstId /*inst_id*/, Branch inst) -> void {
-  if (!in_terminator_sequence_) {
-    Indent();
+  // Several instructions have a second operand that's a specific ID. We
+  // don't include it in the argument list if there is no corresponding
+  // specific, that is, when we're not in a generic context.
+  if (auto arg1_specific_id = arg1.TryAs<SpecificId>();
+      arg1_specific_id && !arg1_specific_id->has_value()) {
+    return;
   }
-  out_ << Branch::Kind.ir_name() << " ";
-  FormatLabel(inst.target_id);
-  out_ << "\n";
-  in_terminator_sequence_ = false;
+  out_ << ", ";
+  FormatInstArgAndKind(arg1);
 }
 
-auto Formatter::FormatInstRhs(Call inst) -> void {
+auto Formatter::FormatCallRhs(Call inst) -> void {
   out_ << " ";
   FormatArg(inst.callee_id);
 
@@ -955,116 +1148,7 @@ auto Formatter::FormatInstRhs(Call inst) -> void {
   }
 }
 
-auto Formatter::FormatInstRhs(ArrayInit inst) -> void {
-  FormatArgs(inst.inits_id);
-  FormatReturnSlotArg(inst.dest_id);
-}
-
-auto Formatter::Formatter::FormatInstRhs(InitializeFrom inst) -> void {
-  FormatArgs(inst.src_id);
-  FormatReturnSlotArg(inst.dest_id);
-}
-
-auto Formatter::FormatInstRhs(ValueParam inst) -> void {
-  FormatArgs(inst.index);
-  // Omit pretty_name because it's an implementation detail of
-  // pretty-printing.
-}
-
-auto Formatter::FormatInstRhs(RefParam inst) -> void {
-  FormatArgs(inst.index);
-  // Omit pretty_name because it's an implementation detail of
-  // pretty-printing.
-}
-
-auto Formatter::FormatInstRhs(OutParam inst) -> void {
-  FormatArgs(inst.index);
-  // Omit pretty_name because it's an implementation detail of
-  // pretty-printing.
-}
-
-auto Formatter::FormatInstRhs(ReturnExpr ret) -> void {
-  FormatArgs(ret.expr_id);
-  if (ret.dest_id.has_value()) {
-    FormatReturnSlotArg(ret.dest_id);
-  }
-}
-
-auto Formatter::FormatInstRhs(ReturnSlot inst) -> void {
-  // Omit inst.type_inst_id because it's not semantically significant.
-  FormatArgs(inst.storage_id);
-}
-
-auto Formatter::FormatInstRhs(ReturnSlotPattern /*inst*/) -> void {
-  // No-op because type_id is the only semantically significant field,
-  // and it's handled separately.
-}
-
-auto Formatter::FormatInstRhs(StructInit init) -> void {
-  FormatArgs(init.elements_id);
-  FormatReturnSlotArg(init.dest_id);
-}
-
-auto Formatter::FormatInstRhs(TupleInit init) -> void {
-  FormatArgs(init.elements_id);
-  FormatReturnSlotArg(init.dest_id);
-}
-
-auto Formatter::FormatInstRhs(FunctionDecl inst) -> void {
-  FormatArgs(inst.function_id);
-  llvm::SaveAndRestore class_scope(scope_,
-                                   inst_namer_.GetScopeFor(inst.function_id));
-  FormatTrailingBlock(
-      sem_ir_->functions().Get(inst.function_id).pattern_block_id);
-  FormatTrailingBlock(inst.decl_block_id);
-}
-
-auto Formatter::FormatInstRhs(ClassDecl inst) -> void {
-  FormatArgs(inst.class_id);
-  llvm::SaveAndRestore class_scope(scope_,
-                                   inst_namer_.GetScopeFor(inst.class_id));
-  FormatTrailingBlock(sem_ir_->classes().Get(inst.class_id).pattern_block_id);
-  FormatTrailingBlock(inst.decl_block_id);
-}
-
-auto Formatter::FormatInstRhs(ImplDecl inst) -> void {
-  FormatArgs(inst.impl_id);
-  llvm::SaveAndRestore class_scope(scope_,
-                                   inst_namer_.GetScopeFor(inst.impl_id));
-  FormatTrailingBlock(sem_ir_->impls().Get(inst.impl_id).pattern_block_id);
-  FormatTrailingBlock(inst.decl_block_id);
-}
-
-auto Formatter::FormatInstRhs(InterfaceDecl inst) -> void {
-  FormatArgs(inst.interface_id);
-  llvm::SaveAndRestore class_scope(scope_,
-                                   inst_namer_.GetScopeFor(inst.interface_id));
-  FormatTrailingBlock(
-      sem_ir_->interfaces().Get(inst.interface_id).pattern_block_id);
-  FormatTrailingBlock(inst.decl_block_id);
-}
-
-auto Formatter::FormatInstRhs(AssociatedConstantDecl inst) -> void {
-  FormatArgs(inst.assoc_const_id);
-  llvm::SaveAndRestore assoc_const_scope(
-      scope_, inst_namer_.GetScopeFor(inst.assoc_const_id));
-  FormatTrailingBlock(inst.decl_block_id);
-}
-
-auto Formatter::FormatInstRhs(IntValue inst) -> void {
-  out_ << " ";
-  sem_ir_->ints()
-      .Get(inst.int_id)
-      .print(out_, sem_ir_->types().IsSignedInt(inst.type_id));
-}
-
-auto Formatter::FormatInstRhs(FloatLiteral inst) -> void {
-  llvm::SmallVector<char, 16> buffer;
-  sem_ir_->floats().Get(inst.float_id).toString(buffer);
-  out_ << " " << buffer;
-}
-
-auto Formatter::FormatInstRhs(ImportCppDecl /*inst*/) -> void {
+auto Formatter::FormatImportCppDeclRhs() -> void {
   out_ << " ";
   OpenBrace();
   for (ImportCpp import_cpp : sem_ir_->import_cpps().array_ref()) {
@@ -1077,16 +1161,14 @@ auto Formatter::FormatInstRhs(ImportCppDecl /*inst*/) -> void {
   CloseBrace();
 }
 
-auto Formatter::FormatImportRefRhs(ImportIRInstId import_ir_inst_id,
-                                   EntityNameId entity_name_id,
-                                   llvm::StringLiteral loaded_label) -> void {
+auto Formatter::FormatImportRefRhs(AnyImportRef inst) -> void {
   out_ << " ";
-  auto import_ir_inst = sem_ir_->import_ir_insts().Get(import_ir_inst_id);
+  auto import_ir_inst = sem_ir_->import_ir_insts().Get(inst.import_ir_inst_id);
   FormatArg(import_ir_inst.ir_id());
   out_ << ", ";
-  if (entity_name_id.has_value()) {
+  if (inst.entity_name_id.has_value()) {
     // Prefer to show the entity name when possible.
-    FormatArg(entity_name_id);
+    FormatArg(inst.entity_name_id);
   } else {
     // Show a name based on the location when possible, or the numeric
     // instruction as a last resort.
@@ -1116,50 +1198,8 @@ auto Formatter::FormatImportRefRhs(ImportIRInstId import_ir_inst_id,
         CARBON_FATAL("Unexpected LocId: {0}", loc_id);
     }
   }
-  out_ << ", " << loaded_label;
-}
-
-auto Formatter::FormatInstRhs(ImportRefLoaded inst) -> void {
-  FormatImportRefRhs(inst.import_ir_inst_id, inst.entity_name_id, "loaded");
-}
-
-auto Formatter::FormatInstRhs(ImportRefUnloaded inst) -> void {
-  FormatImportRefRhs(inst.import_ir_inst_id, inst.entity_name_id, "unloaded");
-}
-
-auto Formatter::FormatInstRhs(InstValue inst) -> void {
-  out_ << ' ';
-  OpenBrace();
-  // TODO: Should we use a more compact representation in the case where the
-  // inst is a SpliceBlock?
-  FormatInst(inst.inst_id);
-  CloseBrace();
-}
-
-auto Formatter::FormatInstRhs(NameBindingDecl inst) -> void {
-  FormatTrailingBlock(inst.pattern_block_id);
-}
-
-auto Formatter::FormatInstRhs(SpliceBlock inst) -> void {
-  FormatArgs(inst.result_id);
-  FormatTrailingBlock(inst.block_id);
-}
-
-auto Formatter::FormatInstRhs(WhereExpr inst) -> void {
-  FormatArgs(inst.period_self_id);
-  FormatTrailingBlock(inst.requirements_id);
-}
-
-auto Formatter::FormatInstRhs(StructType inst) -> void {
-  out_ << " {";
-  llvm::ListSeparator sep;
-  for (auto field : sem_ir_->struct_type_fields().Get(inst.fields_id)) {
-    out_ << sep << ".";
-    FormatName(field.name_id);
-    out_ << ": ";
-    FormatInstAsType(field.type_inst_id);
-  }
-  out_ << "}";
+  out_ << ", "
+       << (inst.kind == InstKind::ImportRefLoaded ? "loaded" : "unloaded");
 }
 
 auto Formatter::FormatArg(EntityNameId id) -> void {

toolchain/sem_ir/formatter.h

@@ -7,6 +7,7 @@
 
 #include <concepts>
 
+#include "common/concepts.h"
 #include "llvm/Support/raw_ostream.h"
 #include "toolchain/parse/tree_and_subtrees.h"
 #include "toolchain/sem_ir/file.h"
@@ -207,6 +208,15 @@ class Formatter {
   // Don't print a constant for ImportRefUnloaded.
   auto FormatInst(InstId inst_id, ImportRefUnloaded inst) -> void;
 
+  // Formats as "branch <target>".
+  auto FormatInst(InstId inst_id, Branch inst) -> void;
+
+  // Formats as "if <cond> branch_if <target> else ".
+  auto FormatInst(InstId inst_id, BranchIf inst) -> void;
+
+  // Formats as "branch_with_arg <target>(arg)".
+  auto FormatInst(InstId inst_id, BranchWithArg inst) -> void;
+
   // Prints a single instruction. This typically dispatches to one of the
   // `FormatInst` overloads, based on a specific instruction type.
   //
@@ -234,53 +244,33 @@ class Formatter {
   // `inst_namer_`). Typed instructions must be named.
   auto FormatInstLhs(InstId inst_id, Inst inst) -> void;
 
-  template <typename InstT>
-  auto FormatInstRhs(InstT inst) -> void;
-
-  auto FormatInstRhs(BindSymbolicName inst) -> void;
+  // Formats arguments to an instruction. This will typically look like "
+  // <arg0>, <arg1>".
+  auto FormatInstRhs(Inst inst) -> void;
 
-  auto FormatInstRhs(BlockArg inst) -> void;
-  auto FormatInstRhs(Namespace inst) -> void;
+  // Formats the default case for `FormatInstRhs`.
+  auto FormatInstRhsDefault(Inst inst) -> void;
 
-  auto FormatInst(InstId inst_id, BranchIf inst) -> void;
-  auto FormatInst(InstId inst_id, BranchWithArg inst) -> void;
-  auto FormatInst(InstId inst_id, Branch inst) -> void;
+  // Formats arguments as " <callee>(<args>) -> <return>".
+  auto FormatCallRhs(Call inst) -> void;
 
-  auto FormatInstRhs(Call inst) -> void;
-  auto FormatInstRhs(ArrayInit inst) -> void;
-  auto FormatInstRhs(InitializeFrom inst) -> void;
-  auto FormatInstRhs(ValueParam inst) -> void;
-  auto FormatInstRhs(RefParam inst) -> void;
-  auto FormatInstRhs(OutParam inst) -> void;
-  auto FormatInstRhs(ReturnExpr ret) -> void;
-  auto FormatInstRhs(ReturnSlot inst) -> void;
-  auto FormatInstRhs(ReturnSlotPattern inst) -> void;
-  auto FormatInstRhs(StructInit init) -> void;
-  auto FormatInstRhs(TupleInit init) -> void;
-  auto FormatInstRhs(FunctionDecl inst) -> void;
-  auto FormatInstRhs(ClassDecl inst) -> void;
-  auto FormatInstRhs(ImplDecl inst) -> void;
-  auto FormatInstRhs(InterfaceDecl inst) -> void;
-  auto FormatInstRhs(AssociatedConstantDecl inst) -> void;
-  auto FormatInstRhs(IntValue inst) -> void;
-  auto FormatInstRhs(FloatLiteral inst) -> void;
+  // Standard formatting for a declaration instruction's arguments.
+  template <typename IdT>
+  auto FormatDeclRhs(IdT decl_id, InstBlockId pattern_block_id,
+                     InstBlockId decl_block_id) {
+    FormatArgs(decl_id);
+    llvm::SaveAndRestore scope(scope_, inst_namer_.GetScopeFor(decl_id));
+    FormatTrailingBlock(pattern_block_id);
+    FormatTrailingBlock(decl_block_id);
+  }
 
   // Format the metadata in File for `import Cpp`.
-  auto FormatInstRhs(ImportCppDecl inst) -> void;
-
-  auto FormatImportRefRhs(ImportIRInstId import_ir_inst_id,
-                          EntityNameId entity_name_id,
-                          llvm::StringLiteral loaded_label) -> void;
-
-  auto FormatInstRhs(ImportRefLoaded inst) -> void;
-  auto FormatInstRhs(ImportRefUnloaded inst) -> void;
-  auto FormatInstRhs(InstValue inst) -> void;
-  auto FormatInstRhs(NameBindingDecl inst) -> void;
-  auto FormatInstRhs(SpliceBlock inst) -> void;
-  auto FormatInstRhs(WhereExpr inst) -> void;
-  auto FormatInstRhs(StructType inst) -> void;
+  auto FormatImportCppDeclRhs() -> void;
 
-  auto FormatArgs() -> void {}
+  // Formats an import ref. In an ideal case, this looks like " <ir>, <entity
+  // name>, <loaded|unloaded>". However, if the entity name isn't present, this
+  // may fall back to printing location information from the import source.
+  auto FormatImportRefRhs(AnyImportRef inst) -> void;
 
   template <typename... Args>
   auto FormatArgs(Args... args) -> void {
@@ -293,6 +283,10 @@ class Formatter {
   // provide equivalent behavior with `FormatName`, so we provide that as the
   // default.
   template <typename IdT>
+    requires(
+        InstNamer::ScopeIdTypeEnum::Contains<IdT> ||
+        SameAsOneOf<IdT, GenericId, NameId, SpecificId, SpecificInterfaceId> ||
+        std::derived_from<IdT, InstId>)
   auto FormatArg(IdT id) -> void {
     FormatName(id);
   }
@@ -312,6 +306,19 @@ class Formatter {
   auto FormatArg(RealId id) -> void;
   auto FormatArg(StringLiteralValueId id) -> void;
 
+  // For MakeFormatArgFnTable.
+  using FormatArgFnT = auto(Formatter& formatter, int32_t arg) -> void;
+
+  // Returns a lookup table to format arguments by their `IdKind`, for
+  // `FormatInstArgAndKind`. Requires a null IdKind as a parameter in order to
+  // get the type pack.
+  template <typename... Types>
+  static constexpr auto MakeFormatArgFnTable(TypeEnum<Types...>* /*id_kind*/)
+      -> std::array<FormatArgFnT*, SemIR::IdKind::NumValues>;
+
+  // Calls `FormatArg` from an `ArgAndKind`.
+  auto FormatInstArgAndKind(Inst::ArgAndKind arg_and_kind) -> void;
+
   auto FormatReturnSlotArg(InstId dest_id) -> void;
 
   // `FormatName` is used when we need the name from an id. Most id types use
@@ -455,27 +462,27 @@ auto Formatter::FormatInst(InstId inst_id, InstT inst) -> void {
   out_ << "\n";
 }
 
-template <typename InstT>
-auto Formatter::FormatInstRhs(InstT inst) -> void {
-  // By default, an instruction has a comma-separated argument list.
-  using Info = Internal::InstLikeTypeInfo<InstT>;
-  if constexpr (Info::NumArgs == 2) {
-    // Several instructions have a second operand that's a specific ID. We
-    // don't include it in the argument list if there is no corresponding
-    // specific, that is, when we're not in a generic context.
-    if constexpr (std::is_same_v<typename Info::template ArgType<1>,
-                                 SpecificId>) {
-      if (!Info::template Get<1>(inst).has_value()) {
-        FormatArgs(Info::template Get<0>(inst));
-        return;
-      }
-    }
-    FormatArgs(Info::template Get<0>(inst), Info::template Get<1>(inst));
-  } else if constexpr (Info::NumArgs == 1) {
-    FormatArgs(Info::template Get<0>(inst));
-  } else {
-    FormatArgs();
-  }
+template <typename... Types>
+constexpr auto Formatter::MakeFormatArgFnTable(TypeEnum<Types...>* /*id_kind*/)
+    -> std::array<FormatArgFnT*, SemIR::IdKind::NumValues> {
+  std::array<FormatArgFnT*, SemIR::IdKind::NumValues> table = {};
+  ((table[SemIR::IdKind::template For<Types>.ToIndex()] =
+        [](Formatter& formatter, int32_t arg) -> void {
+     auto typed_arg = SemIR::Inst::FromRaw<Types>(arg);
+     if constexpr (requires { formatter.FormatArg(typed_arg); }) {
+       formatter.FormatArg(typed_arg);
+     } else {
+       CARBON_FATAL("Missing FormatArg for {0}", typeid(Types).name());
+     }
+   }),
+   ...);
+  table[SemIR::IdKind::Invalid.ToIndex()] = [](Formatter& /*formatter*/,
+                                               int32_t /*arg*/) -> void {
+    CARBON_FATAL("Instruction has argument with invalid IdKind");
+  };
+  table[SemIR::IdKind::None.ToIndex()] = [](Formatter& /*formatter*/,
+                                            int32_t /*arg*/) -> void {};
+  return table;
 }
 
 }  // namespace Carbon::SemIR