Tidy up and refactor the node stack. (#3685)

Add a type representing a non-discriminated union of IDs. Refactor the
node stack to use it. Plus a few other refactorings aiming to clean up
and simplify the code. The overall goal here is that adding a new kind
of ID, node category, or instruction should only require changing one
place in the node stack rather than a bunch of different changes.

One minor functionality change: crash backtraces now use the correct
type for IDs when dumping the node stack rather than using `InstID`
printing for all but one case.

toolchain/check/node_stack.cpp

@@ -9,16 +9,27 @@
 namespace Carbon::Check {
 
 auto NodeStack::PrintForStackDump(llvm::raw_ostream& output) const -> void {
+  auto print_id = [&]<Id::Kind Kind>(Id id) {
+    if constexpr (Kind == Id::Kind::None) {
+      output << " -> no value";
+    } else if constexpr (Kind == Id::Kind::Invalid) {
+      CARBON_FATAL() << "Should not be in node stack";
+    } else {
+      output << " -> " << id.As<Kind>();
+    }
+  };
+
   output << "NodeStack:\n";
   for (auto [i, entry] : llvm::enumerate(stack_)) {
     auto parse_node_kind = parse_tree_->node_kind(entry.parse_node);
     output << "\t" << i << ".\t" << parse_node_kind;
-    if (parse_node_kind == Parse::NodeKind::BindingPattern) {
-      output << " -> " << entry.name_id;
-    } else {
-      if (entry.inst_id.is_valid()) {
-        output << " -> " << entry.inst_id;
-      }
+    switch (parse_node_kind) {
+#define CARBON_PARSE_NODE_KIND(Kind)                                   \
+  case Parse::NodeKind::Kind:                                          \
+    print_id.operator()<ParseNodeKindToIdKind(Parse::NodeKind::Kind)>( \
+        entry.id);                                                     \
+    break;
+#include "toolchain/parse/node_kind.def"
     }
     output << "\n";
   }

toolchain/check/node_stack.h

@@ -16,9 +16,67 @@
 
 namespace Carbon::Check {
 
-// Wraps the stack of parse nodes for Context. Each parse node can have an
-// associated id of some kind (instruction, instruction block, function, class,
-// ...).
+// A non-discriminated union of ID types.
+template <typename... IdTypes>
+class IdUnion {
+ public:
+  // The default constructor forms an invalid ID.
+  explicit constexpr IdUnion() : index(IdBase::InvalidIndex) {}
+
+  template <typename IdT>
+    requires(std::same_as<IdT, IdTypes> || ...)
+  explicit constexpr IdUnion(IdT id) : index(id.index) {}
+
+  static constexpr std::size_t NumValidKinds = sizeof...(IdTypes);
+
+  // A numbering for the associated ID types.
+  enum class Kind : int8_t {
+    // The first `sizeof...(IdTypes)` indexes correspond to the types in
+    // `IdTypes`.
+
+    // An explicit invalid state.
+    Invalid = NumValidKinds,
+
+    // No active union element.
+    None,
+  };
+
+  // Returns the ID given its type.
+  template <typename IdT>
+    requires(std::same_as<IdT, IdTypes> || ...)
+  constexpr auto As() const -> IdT {
+    return IdT(index);
+  }
+
+  // Returns the ID given its kind.
+  template <Kind K>
+    requires(static_cast<size_t>(K) < sizeof...(IdTypes))
+  constexpr auto As() const {
+    using IdT = __type_pack_element<static_cast<size_t>(K), IdTypes...>;
+    return As<IdT>();
+  }
+
+  // Translates an ID type to the enum ID kind. Returns Invalid if `IdT` isn't
+  // a type that can be stored in this union.
+  template <typename IdT>
+  static constexpr auto KindFor() -> Kind {
+    // A bool for each type saying whether it matches. The result is the index
+    // of the first `true` in this list. If none matches, then the result is the
+    // length of the list, which is mapped to `Invalid`.
+    constexpr bool TypeMatches[] = {std::same_as<IdT, IdTypes>...};
+    constexpr int Index =
+        std::find(TypeMatches, TypeMatches + sizeof...(IdTypes), true) -
+        TypeMatches;
+    return static_cast<Kind>(Index);
+  }
+
+ private:
+  decltype(IdBase::index) index;
+};
+
+// The stack of parse nodes representing the current state of a Check::Context.
+// Each parse node can have an associated id of some kind (instruction,
+// instruction block, function, class, ...).
 //
 // All pushes and pops will be vlogged.
 //
@@ -42,20 +100,20 @@ class NodeStack {
   // IR generated by the node.
   auto Push(Parse::NodeId parse_node) -> void {
     auto kind = parse_tree_->node_kind(parse_node);
-    CARBON_CHECK(ParseNodeKindToIdKind(kind) == IdKind::SoloParseNode)
+    CARBON_CHECK(ParseNodeKindToIdKind(kind) == Id::Kind::None)
         << "Parse kind expects an Id: " << kind;
     CARBON_VLOG() << "Node Push " << stack_.size() << ": " << kind
                   << " -> <none>\n";
     CARBON_CHECK(stack_.size() < (1 << 20))
         << "Excessive stack size: likely infinite loop";
-    stack_.push_back(Entry(parse_node, SemIR::InstId::Invalid));
+    stack_.push_back(Entry{parse_node, Id()});
   }
 
   // Pushes a parse tree node onto the stack with an ID.
   template <typename IdT>
   auto Push(Parse::NodeId parse_node, IdT id) -> void {
     auto kind = parse_tree_->node_kind(parse_node);
-    CARBON_CHECK(ParseNodeKindToIdKind(kind) == IdTypeToIdKind<IdT>())
+    CARBON_CHECK(ParseNodeKindToIdKind(kind) == Id::KindFor<IdT>())
         << "Parse kind expected a different IdT: " << kind << " -> " << id
         << "\n";
     CARBON_CHECK(id.is_valid()) << "Push called with invalid id: "
@@ -64,7 +122,7 @@ class NodeStack {
                   << id << "\n";
     CARBON_CHECK(stack_.size() < (1 << 20))
         << "Excessive stack size: likely infinite loop";
-    stack_.push_back(Entry(parse_node, id));
+    stack_.push_back(Entry{parse_node, Id(id)});
   }
 
   // Returns whether there is a node of the specified kind on top of the stack.
@@ -95,8 +153,8 @@ class NodeStack {
 
   // Returns whether there is a name on top of the stack.
   auto PeekIsName() const -> bool {
-    return !stack_.empty() &&
-           ParseNodeKindToIdKind(PeekParseNodeKind()) == IdKind::NameId;
+    return !stack_.empty() && ParseNodeKindToIdKind(PeekParseNodeKind()) ==
+                                  Id::KindFor<SemIR::NameId>();
   }
 
   // Pops the top of the stack without any verification.
@@ -111,7 +169,7 @@ class NodeStack {
   template <const Parse::NodeKind& RequiredParseKind>
   auto PopForSoloParseNode() -> Parse::NodeIdForKind<RequiredParseKind> {
     Entry back = PopEntry<SemIR::InstId>();
-    RequireIdKind(RequiredParseKind, IdKind::SoloParseNode);
+    RequireIdKind(RequiredParseKind, Id::Kind::None);
     RequireParseKind<RequiredParseKind>(back.parse_node);
     return Parse::NodeIdForKind<RequiredParseKind>(back.parse_node);
   }
@@ -163,49 +221,10 @@ class NodeStack {
   // Pops the top of the stack and returns the parse_node and the ID.
   template <const Parse::NodeKind& RequiredParseKind>
   auto PopWithParseNode() -> auto {
-    constexpr IdKind RequiredIdKind = ParseNodeKindToIdKind(RequiredParseKind);
-    auto node_id_cast = [&](auto back) {
-      using NodeIdT = Parse::NodeIdForKind<RequiredParseKind>;
-      return std::pair<NodeIdT, decltype(back.second)>(back);
-    };
-
-    if constexpr (RequiredIdKind == IdKind::InstId) {
-      auto back = PopWithParseNode<SemIR::InstId>();
-      RequireParseKind<RequiredParseKind>(back.first);
-      return node_id_cast(back);
-    }
-    if constexpr (RequiredIdKind == IdKind::InstBlockId) {
-      auto back = PopWithParseNode<SemIR::InstBlockId>();
-      RequireParseKind<RequiredParseKind>(back.first);
-      return node_id_cast(back);
-    }
-    if constexpr (RequiredIdKind == IdKind::FunctionId) {
-      auto back = PopWithParseNode<SemIR::FunctionId>();
-      RequireParseKind<RequiredParseKind>(back.first);
-      return node_id_cast(back);
-    }
-    if constexpr (RequiredIdKind == IdKind::ClassId) {
-      auto back = PopWithParseNode<SemIR::ClassId>();
-      RequireParseKind<RequiredParseKind>(back.first);
-      return node_id_cast(back);
-    }
-    if constexpr (RequiredIdKind == IdKind::InterfaceId) {
-      auto back = PopWithParseNode<SemIR::InterfaceId>();
-      RequireParseKind<RequiredParseKind>(back.first);
-      return node_id_cast(back);
-    }
-    if constexpr (RequiredIdKind == IdKind::NameId) {
-      auto back = PopWithParseNode<SemIR::NameId>();
-      RequireParseKind<RequiredParseKind>(back.first);
-      return node_id_cast(back);
-    }
-    if constexpr (RequiredIdKind == IdKind::TypeId) {
-      auto back = PopWithParseNode<SemIR::TypeId>();
-      RequireParseKind<RequiredParseKind>(back.first);
-      return node_id_cast(back);
-    }
-    CARBON_FATAL() << "Unpoppable IdKind for parse kind: " << RequiredParseKind
-                   << "; see value in ParseNodeKindToIdKind";
+    auto id = Peek<RequiredParseKind>();
+    Parse::NodeIdForKind<RequiredParseKind> parse_node(
+        stack_.pop_back_val().parse_node);
+    return std::make_pair(parse_node, id);
   }
 
   // Pops the top of the stack and returns the parse_node and the ID.
@@ -305,30 +324,9 @@ class NodeStack {
   auto Peek() const -> auto {
     Entry back = stack_.back();
     RequireParseKind<RequiredParseKind>(back.parse_node);
-    constexpr IdKind RequiredIdKind = ParseNodeKindToIdKind(RequiredParseKind);
-    if constexpr (RequiredIdKind == IdKind::InstId) {
-      return back.id<SemIR::InstId>();
-    }
-    if constexpr (RequiredIdKind == IdKind::InstBlockId) {
-      return back.id<SemIR::InstBlockId>();
-    }
-    if constexpr (RequiredIdKind == IdKind::FunctionId) {
-      return back.id<SemIR::FunctionId>();
-    }
-    if constexpr (RequiredIdKind == IdKind::ClassId) {
-      return back.id<SemIR::ClassId>();
-    }
-    if constexpr (RequiredIdKind == IdKind::InterfaceId) {
-      return back.id<SemIR::InterfaceId>();
-    }
-    if constexpr (RequiredIdKind == IdKind::NameId) {
-      return back.id<SemIR::NameId>();
-    }
-    if constexpr (RequiredIdKind == IdKind::TypeId) {
-      return back.id<SemIR::TypeId>();
-    }
-    CARBON_FATAL() << "Unpeekable IdKind for parse kind: " << RequiredParseKind
-                   << "; see value in ParseNodeKindToIdKind";
+    constexpr Id::Kind RequiredIdKind =
+        ParseNodeKindToIdKind(RequiredParseKind);
+    return Peek<RequiredIdKind>();
   }
 
   // Peeks at the ID associated with the top of the name stack.
@@ -336,16 +334,10 @@ class NodeStack {
   auto Peek() const -> auto {
     Entry back = stack_.back();
     RequireParseCategory<RequiredParseCategory>(back.parse_node);
-    constexpr std::optional<IdKind> RequiredIdKind =
-        ParseNodeCategoryToIdKind(RequiredParseCategory);
+    constexpr std::optional<Id::Kind> RequiredIdKind =
+        ParseNodeCategoryToIdKind(RequiredParseCategory, false);
     static_assert(RequiredIdKind.has_value());
-    if constexpr (*RequiredIdKind == IdKind::InstId) {
-      return back.id<SemIR::InstId>();
-    } else {
-      static_assert(*RequiredIdKind == IdKind::NameId,
-                    "Unpeekable IdKind for parse category");
-      return back.id<SemIR::NameId>();
-    }
+    return Peek<*RequiredIdKind>();
   }
 
   // Prints the stack for a stack dump.
@@ -354,127 +346,77 @@ class NodeStack {
   auto empty() const -> bool { return stack_.empty(); }
   auto size() const -> size_t { return stack_.size(); }
 
- private:
-  // Possible associated ID types.
-  enum class IdKind : int8_t {
-    InstId,
-    InstBlockId,
-    FunctionId,
-    ClassId,
-    InterfaceId,
-    NameId,
-    // NOTE: Currently unused.
-    TypeId,
-    // No associated ID type.
-    SoloParseNode,
-    // Not expected in the node stack.
-    Unused,
-  };
+ protected:
+  // An ID that can be associated with a parse node.
+  //
+  // Each parse node kind has a corresponding Id::Kind indicating which kind of
+  // ID is stored, computed by ParseNodeKindToIdKind. Id::Kind::None indicates
+  // that the parse node has no associated ID, in which case the *SoloParseNode
+  // functions should be used to push and pop it. Id::Kind::Invalid indicates
+  // that the parse node should not appear in the node stack at all.
+  using Id =
+      IdUnion<SemIR::InstId, SemIR::InstBlockId, SemIR::FunctionId,
+              SemIR::ClassId, SemIR::InterfaceId, SemIR::NameId, SemIR::TypeId>;
 
   // An entry in stack_.
   struct Entry {
-    explicit Entry(Parse::NodeId parse_node, SemIR::InstId inst_id)
-        : parse_node(parse_node), inst_id(inst_id) {}
-    explicit Entry(Parse::NodeId parse_node, SemIR::InstBlockId inst_block_id)
-        : parse_node(parse_node), inst_block_id(inst_block_id) {}
-    explicit Entry(Parse::NodeId parse_node, SemIR::FunctionId function_id)
-        : parse_node(parse_node), function_id(function_id) {}
-    explicit Entry(Parse::NodeId parse_node, SemIR::ClassId class_id)
-        : parse_node(parse_node), class_id(class_id) {}
-    explicit Entry(Parse::NodeId parse_node, SemIR::InterfaceId interface_id)
-        : parse_node(parse_node), interface_id(interface_id) {}
-    explicit Entry(Parse::NodeId parse_node, SemIR::NameId name_id)
-        : parse_node(parse_node), name_id(name_id) {}
-    explicit Entry(Parse::NodeId parse_node, SemIR::TypeId type_id)
-        : parse_node(parse_node), type_id(type_id) {}
-
-    // Returns the appropriate ID basaed on type.
-    template <typename T>
-    auto id() -> T& {
-      if constexpr (std::is_same<T, SemIR::InstId>()) {
-        return inst_id;
-      }
-      if constexpr (std::is_same<T, SemIR::InstBlockId>()) {
-        return inst_block_id;
-      }
-      if constexpr (std::is_same<T, SemIR::FunctionId>()) {
-        return function_id;
-      }
-      if constexpr (std::is_same<T, SemIR::ClassId>()) {
-        return class_id;
-      }
-      if constexpr (std::is_same<T, SemIR::InterfaceId>()) {
-        return interface_id;
-      }
-      if constexpr (std::is_same<T, SemIR::NameId>()) {
-        return name_id;
-      }
-      if constexpr (std::is_same<T, SemIR::TypeId>()) {
-        return type_id;
-      }
-    }
-
     // The parse node associated with the stack entry.
     Parse::NodeId parse_node;
 
-    // The entries will evaluate as invalid if and only if they're a solo
-    // parse_node. Invalid is used instead of optional to save space.
-    //
-    // A discriminator isn't needed because the caller can determine which field
-    // is used based on the Parse::NodeKind.
-    union {
-      SemIR::InstId inst_id;
-      SemIR::InstBlockId inst_block_id;
-      SemIR::FunctionId function_id;
-      SemIR::ClassId class_id;
-      SemIR::InterfaceId interface_id;
-      SemIR::NameId name_id;
-      SemIR::TypeId type_id;
-    };
+    // The ID associated with this parse node. The kind of ID is determined by
+    // the kind of the parse node, so a separate discriminiator is not needed.
+    Id id;
   };
   static_assert(sizeof(Entry) == 8, "Unexpected Entry size");
 
   // Translate a parse node category to the enum ID kind it should always
   // provide, if it is consistent.
-  static constexpr auto ParseNodeCategoryToIdKind(Parse::NodeCategory category)
-      -> std::optional<IdKind> {
+  static constexpr auto ParseNodeCategoryToIdKind(Parse::NodeCategory category,
+                                                  bool for_node_kind)
+      -> std::optional<Id::Kind> {
+    std::optional<Id::Kind> result;
+    auto set_id_if_category_is = [&](Parse::NodeCategory cat, Id::Kind kind) {
+      if (!!(category & cat)) {
+        // Check for no consistent Id::Kind due to category with multiple bits
+        // set. When computing the Id::Kind for a node kind, a partial category
+        // match is OK, so long as we don't match two inconsistent categories.
+        // When computing the Id::Kind for a category query, the query can't
+        // have any extra bits set or we could be popping a node that is not in
+        // this category.
+        if (for_node_kind ? result.has_value() : !!(category & ~cat)) {
+          result = Id::Kind::Invalid;
+        } else {
+          result = kind;
+        }
+      }
+    };
+
     // TODO: Patterns should also produce an `InstId`, but currently
     // `TuplePattern` produces an `InstBlockId`.
-    if (!!(category & Parse::NodeCategory::Expr)) {
-      // Check for no consistent IdKind due to category with multiple bits set.
-      if (!!(category & ~Parse::NodeCategory::Expr)) {
-        return std::nullopt;
-      }
-      return IdKind::InstId;
-    }
-    if (!!(category & Parse::NodeCategory::MemberName)) {
-      // Check for no consistent IdKind due to category with multiple bits set.
-      if (!!(category & ~Parse::NodeCategory::MemberName)) {
-        return std::nullopt;
-      }
-      return IdKind::NameId;
-    }
-    constexpr Parse::NodeCategory UnusedCategories =
-        Parse::NodeCategory::Decl | Parse::NodeCategory::Statement |
-        Parse::NodeCategory::Modifier;
-    if (!!(category & UnusedCategories)) {
-      // Check for no consistent IdKind due to category with multiple bits set.
-      if (!!(category & ~UnusedCategories)) {
-        return std::nullopt;
-      }
-      return IdKind::Unused;
-    }
-    return std::nullopt;
+    set_id_if_category_is(Parse::NodeCategory::Expr,
+                          Id::KindFor<SemIR::InstId>());
+    set_id_if_category_is(Parse::NodeCategory::MemberName,
+                          Id::KindFor<SemIR::NameId>());
+    set_id_if_category_is(Parse::NodeCategory::Decl |
+                              Parse::NodeCategory::Statement |
+                              Parse::NodeCategory::Modifier,
+                          Id::Kind::None);
+    return result;
   }
 
-  static constexpr auto ComputeIdKindTable()
-      -> std::array<IdKind, Parse::NodeKind::ValidCount> {
-    std::array<IdKind, Parse::NodeKind::ValidCount> table = {};
+  using IdKindTableType = std::array<Id::Kind, Parse::NodeKind::ValidCount>;
+
+  // Lookup table to implement `ParseNodeKindToIdKind`. Initialized to the
+  // return value of `ComputeIdKindTable()`.
+  static const IdKindTableType IdKindTable;
+
+  static constexpr auto ComputeIdKindTable() -> IdKindTableType {
+    IdKindTableType table = {};
 
     auto to_id_kind =
-        [](const Parse::NodeKind::Definition& node_kind) -> IdKind {
+        [](const Parse::NodeKind::Definition& node_kind) -> Id::Kind {
       if (auto from_category =
-              NodeStack::ParseNodeCategoryToIdKind(node_kind.category())) {
+              ParseNodeCategoryToIdKind(node_kind.category(), true)) {
         return *from_category;
       }
       switch (node_kind) {
@@ -490,7 +432,7 @@ class NodeStack {
         case Parse::NodeKind::StructFieldType:
         case Parse::NodeKind::TypeImplAs:
         case Parse::NodeKind::VariableInitializer:
-          return IdKind::InstId;
+          return Id::KindFor<SemIR::InstId>();
         case Parse::NodeKind::IfCondition:
         case Parse::NodeKind::IfExprIf:
         case Parse::NodeKind::ImplForall:
@@ -498,16 +440,15 @@ class NodeStack {
         case Parse::NodeKind::TuplePattern:
         case Parse::NodeKind::WhileCondition:
         case Parse::NodeKind::WhileConditionStart:
-          return IdKind::InstBlockId;
+          return Id::KindFor<SemIR::InstBlockId>();
         case Parse::NodeKind::FunctionDefinitionStart:
-          return IdKind::FunctionId;
+          return Id::KindFor<SemIR::FunctionId>();
         case Parse::NodeKind::ClassDefinitionStart:
-          return IdKind::ClassId;
+          return Id::KindFor<SemIR::ClassId>();
         case Parse::NodeKind::InterfaceDefinitionStart:
-          return IdKind::InterfaceId;
-        case Parse::NodeKind::IdentifierName:
+          return Id::KindFor<SemIR::InterfaceId>();
         case Parse::NodeKind::SelfValueName:
-          return IdKind::NameId;
+          return Id::KindFor<SemIR::NameId>();
         case Parse::NodeKind::ArrayExprSemi:
         case Parse::NodeKind::ClassIntroducer:
         case Parse::NodeKind::CodeBlockStart:
@@ -525,9 +466,9 @@ class NodeStack {
         case Parse::NodeKind::StructLiteralOrStructTypeLiteralStart:
         case Parse::NodeKind::TuplePatternStart:
         case Parse::NodeKind::VariableIntroducer:
-          return IdKind::SoloParseNode;
+          return Id::Kind::None;
         default:
-          return IdKind::Unused;
+          return Id::Kind::Invalid;
       }
     };
 
@@ -538,40 +479,17 @@ class NodeStack {
     return table;
   }
 
-  // Lookup table to implement `ParseNodeKindToIdKind`. Initialized to the
-  // return value of `ComputeIdKindTable()`.
-  static const std::array<IdKind, Parse::NodeKind::ValidCount> IdKindTable;
-
   // Translate a parse node kind to the enum ID kind it should always provide.
-  static constexpr auto ParseNodeKindToIdKind(Parse::NodeKind kind) -> IdKind {
+  static constexpr auto ParseNodeKindToIdKind(Parse::NodeKind kind)
+      -> Id::Kind {
     return IdKindTable[kind.AsInt()];
   }
 
-  // Translates an ID type to the enum ID kind for comparison with
-  // ParseNodeKindToIdKind.
-  template <typename IdT>
-  static constexpr auto IdTypeToIdKind() -> IdKind {
-    if constexpr (std::is_same_v<IdT, SemIR::InstId>) {
-      return IdKind::InstId;
-    }
-    if constexpr (std::is_same_v<IdT, SemIR::InstBlockId>) {
-      return IdKind::InstBlockId;
-    }
-    if constexpr (std::is_same_v<IdT, SemIR::FunctionId>) {
-      return IdKind::FunctionId;
-    }
-    if constexpr (std::is_same_v<IdT, SemIR::ClassId>) {
-      return IdKind::ClassId;
-    }
-    if constexpr (std::is_same_v<IdT, SemIR::InterfaceId>) {
-      return IdKind::InterfaceId;
-    }
-    if constexpr (std::is_same_v<IdT, SemIR::NameId>) {
-      return IdKind::NameId;
-    }
-    if constexpr (std::is_same_v<IdT, SemIR::TypeId>) {
-      return IdKind::TypeId;
-    }
+  // Peeks at the ID associated with the top of the name stack.
+  template <Id::Kind RequiredIdKind>
+  auto Peek() const -> auto {
+    Id id = stack_.back().id;
+    return id.As<RequiredIdKind>();
   }
 
   // Pops an entry.
@@ -580,7 +498,7 @@ class NodeStack {
     Entry back = stack_.pop_back_val();
     CARBON_VLOG() << "Node Pop " << stack_.size() << ": "
                   << parse_tree_->node_kind(back.parse_node) << " -> "
-                  << back.id<IdT>() << "\n";
+                  << back.id.template As<IdT>() << "\n";
     return back;
   }
 
@@ -588,15 +506,15 @@ class NodeStack {
   template <typename IdT>
   auto PopWithParseNode() -> std::pair<Parse::NodeId, IdT> {
     Entry back = PopEntry<IdT>();
-    RequireIdKind(parse_tree_->node_kind(back.parse_node),
-                  IdTypeToIdKind<IdT>());
-    return {back.parse_node, back.id<IdT>()};
+    RequireIdKind(parse_tree_->node_kind(back.parse_node), Id::KindFor<IdT>());
+    return {back.parse_node, back.id.template As<IdT>()};
   }
 
-  // Require a Parse::NodeKind be mapped to a particular IdKind.
-  auto RequireIdKind(Parse::NodeKind parse_kind, IdKind id_kind) const -> void {
+  // Require a Parse::NodeKind be mapped to a particular Id::Kind.
+  auto RequireIdKind(Parse::NodeKind parse_kind, Id::Kind id_kind) const
+      -> void {
     CARBON_CHECK(ParseNodeKindToIdKind(parse_kind) == id_kind)
-        << "Unexpected IdKind mapping for " << parse_kind;
+        << "Unexpected Id::Kind mapping for " << parse_kind;
   }
 
   // Require an entry to have the given Parse::NodeKind.
@@ -628,8 +546,8 @@ class NodeStack {
   llvm::SmallVector<Entry> stack_;
 };
 
-constexpr std::array<NodeStack::IdKind, Parse::NodeKind::ValidCount>
-    NodeStack::IdKindTable = NodeStack::ComputeIdKindTable();
+constexpr NodeStack::IdKindTableType NodeStack::IdKindTable =
+    ComputeIdKindTable();
 
 inline auto NodeStack::PopExprWithParseNode()
     -> std::pair<Parse::AnyExprId, SemIR::InstId> {