Bläddra i källkod

Refactor NodeStack APIs to better handle the increase of distinct ID types. (#2864)

I'm looking at adding CallableId, so figured I'd do this API refactoring which should reduce code duplication. This increases the amount of cross-calls between APIs because it may not be an issue for performance after optimizations, and should simplify reading of the API.

Also note the prior static_asserts on layout were missing a couple types, which is why I'm moving them into Entry where it's going to be more obvious when something's added.
Jon Ross-Perkins 2 år sedan
förälder
incheckning
a1a7251716

+ 2 - 2
toolchain/semantics/semantics_context.cpp

@@ -116,7 +116,7 @@ auto SemanticsContext::BindName(ParseTree::Node name_node,
 auto SemanticsContext::TempRemoveLatestNameFromLookup() -> SemanticsNodeId {
   // Save the storage ID.
   auto it = name_lookup_.find(
-      node_stack_.PeekForNameId(ParseNodeKind::PatternBinding));
+      node_stack_.Peek<SemanticsStringId>(ParseNodeKind::PatternBinding));
   CARBON_CHECK(it != name_lookup_.end());
   CARBON_CHECK(!it->second.empty());
   auto storage_id = it->second.back();
@@ -312,7 +312,7 @@ auto SemanticsContext::ParamOrArgSave(bool for_args) -> void {
     // For an argument, we add a stub reference to the expression on the top of
     // the stack. There may not be anything on the IR prior to this.
     auto [entry_parse_node, entry_node_id] =
-        node_stack_.PopForParseNodeAndNodeId();
+        node_stack_.PopWithParseNode<SemanticsNodeId>();
     param_or_arg_id = AddNode(SemanticsNode::StubReference::Make(
         entry_parse_node, semantics_ir_->GetNode(entry_node_id).type_id(),
         entry_node_id));

+ 12 - 11
toolchain/semantics/semantics_handle.cpp

@@ -3,6 +3,7 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
 #include "toolchain/semantics/semantics_context.h"
+#include "toolchain/semantics/semantics_node.h"
 
 namespace Carbon {
 
@@ -70,10 +71,10 @@ auto SemanticsHandleDesignatedName(SemanticsContext& context,
 
 auto SemanticsHandleDesignatorExpression(SemanticsContext& context,
                                          ParseTree::Node parse_node) -> bool {
-  auto [_, name_id] = context.node_stack().PopForParseNodeAndNameId(
+  auto name_id = context.node_stack().Pop<SemanticsStringId>(
       ParseNodeKind::DesignatedName);
 
-  auto base_id = context.node_stack().PopForNodeId();
+  auto base_id = context.node_stack().Pop<SemanticsNodeId>();
   auto base = context.semantics_ir().GetNode(base_id);
   auto base_type = context.semantics_ir().GetNode(
       context.semantics_ir().GetType(base.type_id()));
@@ -189,8 +190,8 @@ auto SemanticsHandleIfStatementElse(SemanticsContext& context,
 
 auto SemanticsHandleInfixOperator(SemanticsContext& context,
                                   ParseTree::Node parse_node) -> bool {
-  auto rhs_id = context.node_stack().PopForNodeId();
-  auto lhs_id = context.node_stack().PopForNodeId();
+  auto rhs_id = context.node_stack().Pop<SemanticsNodeId>();
+  auto lhs_id = context.node_stack().Pop<SemanticsNodeId>();
 
   // TODO: This should search for a compatible interface. For now, it's a very
   // trivial check of validity on the operation.
@@ -367,7 +368,7 @@ auto SemanticsHandleParenExpressionOrTupleLiteralStart(
 auto SemanticsHandlePatternBinding(SemanticsContext& context,
                                    ParseTree::Node parse_node) -> bool {
   auto [type_node, parsed_type_id] =
-      context.node_stack().PopForParseNodeAndNodeId();
+      context.node_stack().PopWithParseNode<SemanticsNodeId>();
   auto cast_type_id = context.ExpressionAsType(type_node, parsed_type_id);
 
   // Get the name.
@@ -423,7 +424,7 @@ auto SemanticsHandleReturnStatement(SemanticsContext& context,
 
     context.AddNodeAndPush(parse_node, SemanticsNode::Return::Make(parse_node));
   } else {
-    auto arg = context.node_stack().PopForNodeId();
+    auto arg = context.node_stack().Pop<SemanticsNodeId>();
     context.node_stack().PopAndDiscardSoloParseNode(
         ParseNodeKind::ReturnStatementStart);
 
@@ -461,7 +462,7 @@ auto SemanticsHandleReturnType(SemanticsContext& context,
                                ParseTree::Node parse_node) -> bool {
   // Propagate the type expression.
   auto [type_parse_node, type_node_id] =
-      context.node_stack().PopForParseNodeAndNodeId();
+      context.node_stack().PopWithParseNode<SemanticsNodeId>();
   auto cast_node_id = context.ExpressionAsType(type_parse_node, type_node_id);
   context.node_stack().Push(parse_node, cast_node_id);
   return true;
@@ -495,14 +496,14 @@ auto SemanticsHandleTupleLiteralComma(SemanticsContext& context,
 auto SemanticsHandleVariableDeclaration(SemanticsContext& context,
                                         ParseTree::Node parse_node) -> bool {
   auto [last_parse_node, last_node_id] =
-      context.node_stack().PopForParseNodeAndNodeId();
+      context.node_stack().PopWithParseNode<SemanticsNodeId>();
 
   if (context.parse_tree().node_kind(last_parse_node) !=
       ParseNodeKind::PatternBinding) {
-    auto storage_id =
-        context.node_stack().PopForNodeId(ParseNodeKind::VariableInitializer);
+    auto storage_id = context.node_stack().Pop<SemanticsNodeId>(
+        ParseNodeKind::VariableInitializer);
 
-    auto binding = context.node_stack().PopForParseNodeAndNameId(
+    auto binding = context.node_stack().PopWithParseNode<SemanticsStringId>(
         ParseNodeKind::PatternBinding);
 
     // Restore the name now that the initializer is complete.

+ 2 - 2
toolchain/semantics/semantics_handle_call_expression.cpp

@@ -13,7 +13,7 @@ auto SemanticsHandleCallExpression(SemanticsContext& context,
 
   // TODO: Convert to call expression.
   auto [call_expr_parse_node, name_id] =
-      context.node_stack().PopForParseNodeAndNodeId(
+      context.node_stack().PopWithParseNode<SemanticsNodeId>(
           ParseNodeKind::CallExpressionStart);
   auto name_node = context.semantics_ir().GetNode(name_id);
   if (name_node.kind() != SemanticsNodeKind::FunctionDeclaration) {
@@ -58,7 +58,7 @@ auto SemanticsHandleCallExpressionComma(SemanticsContext& context,
 auto SemanticsHandleCallExpressionStart(SemanticsContext& context,
                                         ParseTree::Node parse_node) -> bool {
   auto name_id =
-      context.node_stack().PopForNodeId(ParseNodeKind::NameReference);
+      context.node_stack().Pop<SemanticsNodeId>(ParseNodeKind::NameReference);
   context.node_stack().Push(parse_node, name_id);
   context.ParamOrArgStart();
   return true;

+ 5 - 5
toolchain/semantics/semantics_handle_function.cpp

@@ -18,8 +18,8 @@ auto SemanticsHandleFunctionDefinition(SemanticsContext& context,
          ParseNodeKind::FunctionDefinitionStart) {
     context.node_stack().PopAndIgnore();
   }
-  auto decl_id =
-      context.node_stack().PopForNodeId(ParseNodeKind::FunctionDefinitionStart);
+  auto decl_id = context.node_stack().Pop<SemanticsNodeId>(
+      ParseNodeKind::FunctionDefinitionStart);
 
   context.return_scope_stack().pop_back();
   context.PopScope();
@@ -38,13 +38,13 @@ auto SemanticsHandleFunctionDefinitionStart(SemanticsContext& context,
   if (context.parse_tree().node_kind(context.node_stack().PeekParseNode()) ==
       ParseNodeKind::ReturnType) {
     return_type_id =
-        context.node_stack().PopForTypeId(ParseNodeKind::ReturnType);
+        context.node_stack().Pop<SemanticsTypeId>(ParseNodeKind::ReturnType);
   } else {
     // Canonicalize the empty tuple for the implicit return.
     context.CanonicalizeType(SemanticsNodeId::BuiltinEmptyTupleType);
   }
-  auto param_refs_id =
-      context.node_stack().PopForNodeBlockId(ParseNodeKind::ParameterList);
+  auto param_refs_id = context.node_stack().Pop<SemanticsNodeBlockId>(
+      ParseNodeKind::ParameterList);
   auto name_node =
       context.node_stack().PopForSoloParseNode(ParseNodeKind::DeclaredName);
   auto fn_node = context.node_stack().PopForSoloParseNode(

+ 7 - 5
toolchain/semantics/semantics_handle_struct.cpp

@@ -27,11 +27,13 @@ auto SemanticsHandleStructFieldDesignator(SemanticsContext& context,
 
 auto SemanticsHandleStructFieldType(SemanticsContext& context,
                                     ParseTree::Node parse_node) -> bool {
-  auto [type_node, type_id] = context.node_stack().PopForParseNodeAndNodeId();
+  auto [type_node, type_id] =
+      context.node_stack().PopWithParseNode<SemanticsNodeId>();
   SemanticsTypeId cast_type_id = context.ExpressionAsType(type_node, type_id);
 
-  auto [name_node, name_id] = context.node_stack().PopForParseNodeAndNameId(
-      ParseNodeKind::DesignatedName);
+  auto [name_node, name_id] =
+      context.node_stack().PopWithParseNode<SemanticsStringId>(
+          ParseNodeKind::DesignatedName);
 
   context.AddNode(
       SemanticsNode::StructTypeField::Make(name_node, cast_type_id, name_id));
@@ -47,8 +49,8 @@ auto SemanticsHandleStructFieldUnknown(SemanticsContext& context,
 auto SemanticsHandleStructFieldValue(SemanticsContext& context,
                                      ParseTree::Node parse_node) -> bool {
   auto [value_parse_node, value_node_id] =
-      context.node_stack().PopForParseNodeAndNodeId();
-  auto [_, name_id] = context.node_stack().PopForParseNodeAndNameId(
+      context.node_stack().PopWithParseNode<SemanticsNodeId>();
+  auto name_id = context.node_stack().Pop<SemanticsStringId>(
       ParseNodeKind::DesignatedName);
 
   // Store the name for the type.

+ 0 - 157
toolchain/semantics/semantics_node_stack.cpp

@@ -4,167 +4,10 @@
 
 #include "toolchain/semantics/semantics_node_stack.h"
 
-#include "common/vlog.h"
 #include "toolchain/semantics/semantics_node.h"
 
 namespace Carbon {
 
-auto SemanticsNodeStack::PushEntry(Entry entry, DebugLog debug_log) -> void {
-  CARBON_VLOG() << "Node Push " << stack_.size() << ": "
-                << parse_tree_->node_kind(entry.parse_node) << " -> ";
-  switch (debug_log) {
-    case DebugLog::None:
-      CARBON_VLOG() << "<none>";
-      break;
-    case DebugLog::NodeId:
-      CARBON_VLOG() << entry.node_id;
-      break;
-    case DebugLog::NodeBlockId:
-      CARBON_VLOG() << entry.node_block_id;
-      break;
-    case DebugLog::NameId:
-      CARBON_VLOG() << entry.name_id;
-      break;
-    case DebugLog::TypeId:
-      CARBON_VLOG() << entry.type_id;
-      break;
-  }
-  CARBON_VLOG() << "\n";
-  CARBON_CHECK(stack_.size() < (1 << 20))
-      << "Excessive stack size: likely infinite loop";
-  stack_.push_back(entry);
-}
-
-auto SemanticsNodeStack::PopEntry() -> Entry {
-  auto back = stack_.pop_back_val();
-  CARBON_VLOG() << "Node Pop " << stack_.size() << ": any ("
-                << parse_tree_->node_kind(back.parse_node) << ") -> "
-                << back.node_id << "\n";
-  return back;
-}
-
-auto SemanticsNodeStack::PopEntry(ParseNodeKind pop_parse_kind) -> Entry {
-  auto back = stack_.pop_back_val();
-  CARBON_VLOG() << "Node Pop " << stack_.size() << ": " << pop_parse_kind
-                << " -> " << back.node_id << "\n";
-  RequireParseKind(back, pop_parse_kind);
-  return back;
-}
-
-auto SemanticsNodeStack::RequireParseKind(Entry entry,
-                                          ParseNodeKind require_kind) -> void {
-  auto actual_kind = parse_tree_->node_kind(entry.parse_node);
-  CARBON_CHECK(require_kind == actual_kind)
-      << "Expected " << require_kind << ", found " << actual_kind;
-}
-
-// RequireSoloParseNode and RequireValidId rely on type punning. They read
-// node_id.is_valid, even though that may not be the active union member.
-// These asserts enforce standard layout in order to help ensure that works.
-// TODO: Use is_layout_compatible in C++20.
-static_assert(std::is_standard_layout_v<SemanticsNodeId>,
-              "Need standard layout for type punning");
-static_assert(std::is_standard_layout_v<SemanticsStringId>,
-              "Need standard layout for type punning");
-
-auto SemanticsNodeStack::RequireSoloParseNode(Entry entry) -> void {
-  // See above comment on type punning.
-  CARBON_CHECK(!entry.node_id.is_valid())
-      << "Expected invalid id on " << parse_tree_->node_kind(entry.parse_node)
-      << ", was " << entry.node_id << " (may not be node)";
-}
-
-auto SemanticsNodeStack::RequireValidId(Entry entry) -> void {
-  // See above comment on type punning.
-  CARBON_CHECK(entry.node_id.is_valid())
-      << "Expected valid id on " << parse_tree_->node_kind(entry.parse_node);
-}
-
-auto SemanticsNodeStack::PopAndDiscardId() -> void {
-  auto back = PopEntry();
-  RequireValidId(back);
-}
-
-auto SemanticsNodeStack::PopAndDiscardId(ParseNodeKind pop_parse_kind) -> void {
-  auto back = PopEntry(pop_parse_kind);
-  RequireValidId(back);
-}
-
-auto SemanticsNodeStack::PopAndDiscardSoloParseNode(
-    ParseNodeKind pop_parse_kind) -> void {
-  auto back = PopEntry(pop_parse_kind);
-  RequireSoloParseNode(back);
-}
-
-auto SemanticsNodeStack::PopForSoloParseNode() -> ParseTree::Node {
-  auto back = PopEntry();
-  RequireSoloParseNode(back);
-  return back.parse_node;
-}
-
-auto SemanticsNodeStack::PopForSoloParseNode(ParseNodeKind pop_parse_kind)
-    -> ParseTree::Node {
-  auto back = PopEntry(pop_parse_kind);
-  RequireSoloParseNode(back);
-  return back.parse_node;
-}
-
-auto SemanticsNodeStack::PopForNodeId() -> SemanticsNodeId {
-  auto back = PopEntry();
-  RequireValidId(back);
-  return back.node_id;
-}
-
-auto SemanticsNodeStack::PopForNodeId(ParseNodeKind pop_parse_kind)
-    -> SemanticsNodeId {
-  auto back = PopEntry(pop_parse_kind);
-  RequireValidId(back);
-  return back.node_id;
-}
-
-auto SemanticsNodeStack::PopForParseNodeAndNodeId()
-    -> std::pair<ParseTree::Node, SemanticsNodeId> {
-  auto back = PopEntry();
-  RequireValidId(back);
-  return {back.parse_node, back.node_id};
-}
-
-auto SemanticsNodeStack::PopForParseNodeAndNodeId(ParseNodeKind pop_parse_kind)
-    -> std::pair<ParseTree::Node, SemanticsNodeId> {
-  auto back = PopEntry(pop_parse_kind);
-  RequireValidId(back);
-  return {back.parse_node, back.node_id};
-}
-
-auto SemanticsNodeStack::PopForNodeBlockId(ParseNodeKind pop_parse_kind)
-    -> SemanticsNodeBlockId {
-  auto back = PopEntry(pop_parse_kind);
-  RequireValidId(back);
-  return back.node_block_id;
-}
-
-auto SemanticsNodeStack::PopForTypeId(ParseNodeKind pop_parse_kind)
-    -> SemanticsTypeId {
-  auto back = PopEntry(pop_parse_kind);
-  RequireValidId(back);
-  return back.type_id;
-}
-
-auto SemanticsNodeStack::PopForParseNodeAndNameId(ParseNodeKind pop_parse_kind)
-    -> std::pair<ParseTree::Node, SemanticsStringId> {
-  auto back = PopEntry(pop_parse_kind);
-  RequireValidId(back);
-  return {back.parse_node, back.name_id};
-}
-
-auto SemanticsNodeStack::PeekForNameId(ParseNodeKind parse_kind)
-    -> SemanticsStringId {
-  auto back = stack_.back();
-  RequireParseKind(back, parse_kind);
-  RequireValidId(back);
-  return back.name_id;
-}
-
 auto SemanticsNodeStack::PrintForStackDump(llvm::raw_ostream& output) const
     -> void {
   output << "SemanticsNodeStack:\n";

+ 133 - 72
toolchain/semantics/semantics_node_stack.h

@@ -7,6 +7,7 @@
 
 #include <type_traits>
 
+#include "common/vlog.h"
 #include "llvm/ADT/SmallVector.h"
 #include "toolchain/parser/parse_node_kind.h"
 #include "toolchain/parser/parse_tree.h"
@@ -36,79 +37,93 @@ class SemanticsNodeStack {
   // Pushes a solo parse tree node onto the stack. Used when there is no
   // IR generated by the node.
   auto Push(ParseTree::Node parse_node) -> void {
-    PushEntry({.parse_node = parse_node, .node_id = SemanticsNodeId::Invalid},
-              DebugLog::None);
+    CARBON_VLOG() << "Node Push " << stack_.size() << ": "
+                  << parse_tree_->node_kind(parse_node) << " -> <none>\n";
+    CARBON_CHECK(stack_.size() < (1 << 20))
+        << "Excessive stack size: likely infinite loop";
+    stack_.push_back(Entry(parse_node, SemanticsNodeId::Invalid));
   }
 
-  // Pushes a parse tree node onto the stack with a semantics node.
-  auto Push(ParseTree::Node parse_node, SemanticsNodeId node_id) -> void {
-    PushEntry({.parse_node = parse_node, .node_id = node_id}, DebugLog::NodeId);
-  }
-
-  // Pushes a parse tree node onto the stack with a semantics node block.
-  auto Push(ParseTree::Node parse_node, SemanticsNodeBlockId node_block_id)
-      -> void {
-    PushEntry({.parse_node = parse_node, .node_block_id = node_block_id},
-              DebugLog::NodeBlockId);
-  }
-
-  // Pushes a parse tree node onto the stack with its name.
-  auto Push(ParseTree::Node parse_node, SemanticsStringId name_id) -> void {
-    PushEntry({.parse_node = parse_node, .name_id = name_id}, DebugLog::NameId);
-  }
-
-  // Pushes a parse tree node onto the stack with a semantics node block.
-  auto Push(ParseTree::Node parse_node, SemanticsTypeId type_id) -> void {
-    PushEntry({.parse_node = parse_node, .type_id = type_id}, DebugLog::TypeId);
+  // Pushes a parse tree node onto the stack with an ID.
+  template <typename IdT>
+  auto Push(ParseTree::Node parse_node, IdT id) -> void {
+    CARBON_VLOG() << "Node Push " << stack_.size() << ": "
+                  << parse_tree_->node_kind(parse_node) << " -> " << id << "\n";
+    CARBON_CHECK(stack_.size() < (1 << 20))
+        << "Excessive stack size: likely infinite loop";
+    stack_.push_back(Entry(parse_node, id));
   }
 
   // Pops the top of the stack without any verification.
-  auto PopAndIgnore() -> void { PopEntry(); }
-
-  // Pops the top of the stack.
-  auto PopAndDiscardSoloParseNode(ParseNodeKind pop_parse_kind) -> void;
-
-  // Pops the top of the stack, and discards the ID.
-  auto PopAndDiscardId() -> void;
-
-  // Pops the top of the stack, and discards the ID.
-  auto PopAndDiscardId(ParseNodeKind pop_parse_kind) -> void;
+  auto PopAndIgnore() -> void { PopEntry<SemanticsNodeId>(); }
 
   // Pops the top of the stack and returns the parse_node.
-  auto PopForSoloParseNode() -> ParseTree::Node;
+  auto PopForSoloParseNode() -> ParseTree::Node {
+    Entry back = PopEntry<SemanticsNodeId>();
+    RequireSoloParseNode(back);
+    return back.parse_node;
+  }
 
   // Pops the top of the stack and returns the parse_node.
-  auto PopForSoloParseNode(ParseNodeKind pop_parse_kind) -> ParseTree::Node;
+  auto PopForSoloParseNode(ParseNodeKind pop_parse_kind) -> ParseTree::Node {
+    auto parse_node = PopForSoloParseNode();
+    RequireParseKind(parse_node, pop_parse_kind);
+    return parse_node;
+  }
 
-  // Pops the top of the stack and returns the node_id.
-  auto PopForNodeId(ParseNodeKind pop_parse_kind) -> SemanticsNodeId;
+  // Pops the top of the stack.
+  auto PopAndDiscardSoloParseNode(ParseNodeKind pop_parse_kind) -> void {
+    PopForSoloParseNode(pop_parse_kind);
+  }
 
-  // Pops the top of the stack and returns the parse_node and node_id.
-  auto PopForParseNodeAndNodeId()
-      -> std::pair<ParseTree::Node, SemanticsNodeId>;
+  // Pops the top of the stack and returns the parse_node and the ID.
+  template <typename IdT>
+  auto PopWithParseNode() -> std::pair<ParseTree::Node, IdT> {
+    Entry back = PopEntry<IdT>();
+    RequireValidId(back);
+    return {back.parse_node, back.id<IdT>()};
+  }
 
-  // Pops the top of the stack and returns the parse_node and node_id.
-  auto PopForParseNodeAndNodeId(ParseNodeKind pop_parse_kind)
-      -> std::pair<ParseTree::Node, SemanticsNodeId>;
+  // Pops the top of the stack and returns the parse_node and the ID.
+  template <typename IdT>
+  auto PopWithParseNode(ParseNodeKind pop_parse_kind)
+      -> std::pair<ParseTree::Node, IdT> {
+    auto back = PopWithParseNode<IdT>();
+    RequireParseKind(back.first, pop_parse_kind);
+    return back;
+  }
 
-  // Pops the top of the stack and returns the node_id.
-  auto PopForNodeId() -> SemanticsNodeId;
+  // Pops the top of the stack and returns the ID.
+  template <typename IdT>
+  auto Pop() -> IdT {
+    return PopWithParseNode<IdT>().second;
+  }
 
-  // Pops the top of the stack and returns the node_block_id.
-  auto PopForNodeBlockId(ParseNodeKind pop_parse_kind) -> SemanticsNodeBlockId;
+  // Pops the top of the stack and returns the ID.
+  template <typename IdT>
+  auto Pop(ParseNodeKind pop_parse_kind) -> IdT {
+    return PopWithParseNode<IdT>(pop_parse_kind).second;
+  }
 
-  // Pops the top of the stack and returns the type_id.
-  auto PopForTypeId(ParseNodeKind pop_parse_kind) -> SemanticsTypeId;
+  // Pops the top of the stack, and discards the ID.
+  auto PopAndDiscardId() -> void { PopWithParseNode<SemanticsNodeId>(); }
 
-  // Pops the top of the stack and returns the parse_node and name_id.
-  auto PopForParseNodeAndNameId(ParseNodeKind pop_parse_kind)
-      -> std::pair<ParseTree::Node, SemanticsStringId>;
+  // Pops the top of the stack, and discards the ID.
+  auto PopAndDiscardId(ParseNodeKind pop_parse_kind) -> void {
+    PopWithParseNode<SemanticsNodeId>(pop_parse_kind);
+  }
 
   // Peeks at the parse_node of the top of the stack.
   auto PeekParseNode() -> ParseTree::Node { return stack_.back().parse_node; }
 
-  // Peeks at the name_id of the top of the stack.
-  auto PeekForNameId(ParseNodeKind parse_kind) -> SemanticsStringId;
+  // Peeks at the ID of the top of the stack.
+  template <typename IdT>
+  auto Peek(ParseNodeKind parse_kind) -> IdT {
+    Entry back = stack_.back();
+    RequireParseKind(back.parse_node, parse_kind);
+    RequireValidId(back);
+    return back.id<IdT>();
+  }
 
   // Prints the stack for a stack dump.
   auto PrintForStackDump(llvm::raw_ostream& output) const -> void;
@@ -119,6 +134,33 @@ class SemanticsNodeStack {
  private:
   // An entry in stack_.
   struct Entry {
+    explicit Entry(ParseTree::Node parse_node, SemanticsNodeId node_id)
+        : parse_node(parse_node), node_id(node_id) {}
+    explicit Entry(ParseTree::Node parse_node,
+                   SemanticsNodeBlockId node_block_id)
+        : parse_node(parse_node), node_block_id(node_block_id) {}
+    explicit Entry(ParseTree::Node parse_node, SemanticsStringId name_id)
+        : parse_node(parse_node), name_id(name_id) {}
+    explicit Entry(ParseTree::Node parse_node, SemanticsTypeId 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, SemanticsNodeId>()) {
+        return node_id;
+      }
+      if constexpr (std::is_same<T, SemanticsNodeBlockId>()) {
+        return node_block_id;
+      }
+      if constexpr (std::is_same<T, SemanticsStringId>()) {
+        return name_id;
+      }
+      if constexpr (std::is_same<T, SemanticsTypeId>()) {
+        return type_id;
+      }
+    }
+
     // The node associated with the stack entry.
     ParseTree::Node parse_node;
 
@@ -133,35 +175,54 @@ class SemanticsNodeStack {
       SemanticsStringId name_id;
       SemanticsTypeId type_id;
     };
-  };
-  static_assert(sizeof(Entry) == 8, "Unexpected Entry size");
 
-  // Which Entry union member to log.
-  enum DebugLog {
-    None,
-    NodeId,
-    NodeBlockId,
-    NameId,
-    TypeId,
+    // APIs rely on type punning. They read node_id.is_valid, even though that
+    // may not be the active union member. These asserts enforce standard layout
+    // in order to help ensure that works.
+    // TODO: Use is_layout_compatible in C++20.
+    static_assert(std::is_standard_layout_v<SemanticsNodeId>,
+                  "Need standard layout for type punning");
+    static_assert(std::is_standard_layout_v<SemanticsNodeBlockId>,
+                  "Need standard layout for type punning");
+    static_assert(std::is_standard_layout_v<SemanticsStringId>,
+                  "Need standard layout for type punning");
+    static_assert(std::is_standard_layout_v<SemanticsTypeId>,
+                  "Need standard layout for type punning");
   };
-
-  // Pushes an entry onto the stack.
-  auto PushEntry(Entry entry, DebugLog debug_log) -> void;
+  static_assert(sizeof(Entry) == 8, "Unexpected Entry size");
 
   // Pops an entry.
-  auto PopEntry() -> Entry;
-
-  // Pops an entry, requiring the specific kind.
-  auto PopEntry(ParseNodeKind pop_parse_kind) -> Entry;
+  template <typename IdT>
+  auto PopEntry() -> Entry {
+    Entry back = stack_.pop_back_val();
+    CARBON_VLOG() << "Node Pop " << stack_.size() << ": "
+                  << parse_tree_->node_kind(back.parse_node) << " -> "
+                  << back.id<IdT>() << "\n";
+    return back;
+  }
 
   // Require an entry to have the given ParseNodeKind.
-  auto RequireParseKind(Entry entry, ParseNodeKind require_kind) -> void;
+  auto RequireParseKind(ParseTree::Node parse_node, ParseNodeKind require_kind)
+      -> void {
+    auto actual_kind = parse_tree_->node_kind(parse_node);
+    CARBON_CHECK(require_kind == actual_kind)
+        << "Expected " << require_kind << ", found " << actual_kind;
+  }
 
   // Requires an entry to have a invalid node_id.
-  auto RequireSoloParseNode(Entry entry) -> void;
+  auto RequireSoloParseNode(Entry entry) -> void {
+    // See above comment on type punning.
+    CARBON_CHECK(!entry.node_id.is_valid())
+        << "Expected invalid id on " << parse_tree_->node_kind(entry.parse_node)
+        << ", was " << entry.node_id << " (may not be node)";
+  }
 
   // Requires an entry to have a valid id.
-  auto RequireValidId(Entry entry) -> void;
+  auto RequireValidId(Entry entry) -> void {
+    // See above comment on type punning.
+    CARBON_CHECK(entry.node_id.is_valid())
+        << "Expected valid id on " << parse_tree_->node_kind(entry.parse_node);
+  }
 
   // The file's parse tree.
   const ParseTree* parse_tree_;