Try out a different IdKind table approach (#5528)

I was thinking about these after #5526, was wondering how others will
feel about this kind of approach:

- Adding a helper to `TypeEnum` to get the table construction.
- In what were previously `Make` functions, return the element instead
of returning the table.
- By returning the element, no more need to pass in a nullptr (now have
a concrete instance).

I think this is a mild simplification, but maybe worth it.

Note, would appreciate it if there are thoughts on how to provide a
boilerplate `Invalid` implementation (maybe it'd be fine to just return
`nullptr` and cause a crash that way, but I was hesitant to do that).

toolchain/check/eval.cpp

@@ -669,34 +669,28 @@ static constexpr bool HasGetConstantValueOverload = requires {
 using ArgHandlerFnT = auto(EvalContext& context, int32_t arg, Phase* phase)
     -> int32_t;
 
-// Returns a lookup table to get constants by Id::Kind. Requires a null IdKind
-// as a parameter in order to get the type pack.
+// Returns the arg handler for an `IdKind`.
 template <typename... Types>
-static constexpr auto MakeArgHandlerTable(TypeEnum<Types...>* /*id_kind*/)
-    -> std::array<ArgHandlerFnT*, SemIR::IdKind::NumValues> {
-  std::array<ArgHandlerFnT*, SemIR::IdKind::NumValues> table = {};
-  ((table[SemIR::IdKind::template For<Types>.ToIndex()] =
-        [](EvalContext& eval_context, int32_t arg, Phase* phase) -> int32_t {
-     auto id = SemIR::Inst::FromRaw<Types>(arg);
-     if constexpr (HasGetConstantValueOverload<Types>) {
-       // If we have a custom `GetConstantValue` overload, call it.
-       return SemIR::Inst::ToRaw(GetConstantValue(eval_context, id, phase));
-     } else {
-       // Otherwise, we assume the value is already constant.
-       return arg;
-     }
-   }),
-   ...);
-  table[SemIR::IdKind::Invalid.ToIndex()] = [](EvalContext& /*context*/,
-                                               int32_t /*arg*/,
-                                               Phase* /*phase*/) -> int32_t {
-    CARBON_FATAL("Instruction has argument with invalid IdKind");
-  };
-  table[SemIR::IdKind::None.ToIndex()] =
-      [](EvalContext& /*context*/, int32_t arg, Phase* /*phase*/) -> int32_t {
-    return arg;
-  };
-  return table;
+static auto GetArgHandlerFn(TypeEnum<Types...> id_kind) -> ArgHandlerFnT* {
+  static constexpr std::array<ArgHandlerFnT*, SemIR::IdKind::NumValues> Table =
+      {
+          [](EvalContext& eval_context, int32_t arg, Phase* phase) -> int32_t {
+            auto id = SemIR::Inst::FromRaw<Types>(arg);
+            if constexpr (HasGetConstantValueOverload<Types>) {
+              // If we have a custom `GetConstantValue` overload, call it.
+              return SemIR::Inst::ToRaw(
+                  GetConstantValue(eval_context, id, phase));
+            } else {
+              // Otherwise, we assume the value is already constant.
+              return arg;
+            }
+          }...,
+          // Invalid and None handling (ordering-sensitive).
+          [](auto...) -> int32_t { CARBON_FATAL("Unexpected invalid IdKind"); },
+          [](EvalContext& /*context*/, int32_t arg,
+             Phase* /*phase*/) -> int32_t { return arg; },
+      };
+  return Table[id_kind.ToIndex()];
 }
 
 // Given the stored value `arg` of an instruction field and its corresponding
@@ -707,9 +701,7 @@ static constexpr auto MakeArgHandlerTable(TypeEnum<Types...>* /*id_kind*/)
 static auto GetConstantValueForArg(EvalContext& eval_context,
                                    SemIR::Inst::ArgAndKind arg_and_kind,
                                    Phase* phase) -> int32_t {
-  static constexpr auto Table =
-      MakeArgHandlerTable(static_cast<SemIR::IdKind*>(nullptr));
-  return Table[arg_and_kind.kind().ToIndex()](eval_context,
+  return GetArgHandlerFn(arg_and_kind.kind())(eval_context,
                                               arg_and_kind.value(), phase);
 }
 

toolchain/sem_ir/formatter.cpp

@@ -906,9 +906,7 @@ auto Formatter::FormatInstLhs(InstId inst_id, Inst inst) -> void {
 }
 
 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());
+  GetFormatArgFn(arg_and_kind.kind())(*this, arg_and_kind.value());
 }
 
 auto Formatter::FormatInstRhs(Inst inst) -> void {

toolchain/sem_ir/formatter.h

@@ -286,15 +286,12 @@ class Formatter {
   auto FormatArg(RealId id) -> void;
   auto FormatArg(StringLiteralValueId id) -> void;
 
-  // For MakeFormatArgFnTable.
+  // A `FormatArg` wrapper for `FormatInstArgAndKind`.
   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.
+  // Returns the `FormatArgFnT` for the given `IdKind`.
   template <typename... Types>
-  static constexpr auto MakeFormatArgFnTable(TypeEnum<Types...>* /*id_kind*/)
-      -> std::array<FormatArgFnT*, SemIR::IdKind::NumValues>;
+  static auto GetFormatArgFn(TypeEnum<Types...> id_kind) -> FormatArgFnT*;
 
   // Calls `FormatArg` from an `ArgAndKind`.
   auto FormatInstArgAndKind(Inst::ArgAndKind arg_and_kind) -> void;
@@ -430,26 +427,21 @@ auto Formatter::FormatEntityStart(llvm::StringRef entity_kind,
 }
 
 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");
+auto Formatter::GetFormatArgFn(TypeEnum<Types...> id_kind) -> FormatArgFnT* {
+  static constexpr std::array<FormatArgFnT*, IdKind::NumValues> Table = {
+      [](Formatter& formatter, int32_t arg) -> void {
+        auto typed_arg = 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());
+        }
+      }...,
+      // Invalid and None handling (ordering-sensitive).
+      [](auto...) -> void { CARBON_FATAL("Unexpected invalid IdKind"); },
+      [](auto...) -> void {},
   };
-  table[SemIR::IdKind::None.ToIndex()] = [](Formatter& /*formatter*/,
-                                            int32_t /*arg*/) -> void {};
-  return table;
+  return Table[id_kind.ToIndex()];
 }
 
 }  // namespace Carbon::SemIR

toolchain/sem_ir/inst_fingerprinter.cpp

@@ -314,31 +314,23 @@ struct Worklist {
 
   using AddFnT = auto(Worklist& worklist, int32_t arg) -> void;
 
-  // Returns a lookup table to add an argument of the given kind. Requires a
-  // null IdKind as a parameter in order to get the type pack.
+  // Returns the arg handler for an `IdKind`.
   template <typename... Types>
-  static constexpr auto MakeAddTable(TypeEnum<Types...>* /*id_kind*/)
-      -> std::array<AddFnT*, IdKind::NumValues> {
-    std::array<AddFnT*, IdKind::NumValues> table = {};
-    ((table[IdKind::template For<Types>.ToIndex()] =
-          [](Worklist& worklist, int32_t arg) {
-            worklist.Add(Inst::FromRaw<Types>(arg));
-          }),
-     ...);
-    table[IdKind::Invalid.ToIndex()] = [](Worklist& /*worklist*/,
-                                          int32_t /*arg*/) {
-      CARBON_FATAL("Unexpected invalid argument kind");
+  static auto GetAddFn(TypeEnum<Types...> id_kind) -> AddFnT* {
+    static constexpr std::array<AddFnT*, IdKind::NumValues> Table = {
+        [](Worklist& worklist, int32_t arg) {
+          worklist.Add(Inst::FromRaw<Types>(arg));
+        }...,
+        // Invalid and None handling (ordering-sensitive).
+        [](auto...) { CARBON_FATAL("Unexpected invalid IdKind"); },
+        [](auto...) {},
     };
-    table[IdKind::None.ToIndex()] = [](Worklist& /*worklist*/,
-                                       int32_t /*arg*/) {};
-    return table;
+    return Table[id_kind.ToIndex()];
   }
 
   // Add an instruction argument to the contents of the current instruction.
   auto AddWithKind(Inst::ArgAndKind arg) -> void {
-    static constexpr auto Table = MakeAddTable(static_cast<IdKind*>(nullptr));
-
-    Table[arg.kind().ToIndex()](*this, arg.value());
+    GetAddFn(arg.kind())(*this, arg.value());
   }
 
   // Ensure all the instructions on the todo list have fingerprints. To avoid a