Change result_id to use is_valid instead of an optional. (#2449)

Incrementally shrinks size, but should also make the errors a bit clearer with better context.

toolchain/common/index_base.h

@@ -29,7 +29,7 @@ struct IndexBase {
 
   auto Print(llvm::raw_ostream& output) const -> void { output << index; }
 
-  auto is_valid() -> bool { return index != InvalidIndex; }
+  auto is_valid() const -> bool { return index != InvalidIndex; }
 
   int32_t index;
 };

toolchain/semantics/semantics_node.h

@@ -23,6 +23,7 @@ struct SemanticsNodeId : public IndexBase {
   static auto MakeCrossReference(int32_t index) -> SemanticsNodeId {
     return SemanticsNodeId(index | CrossReferenceBit);
   }
+
   // Constructs a cross-reference node ID for a builtin. This relies on
   // SemanticsIR guarantees for builtin cross-reference placement.
   static auto MakeBuiltinReference(SemanticsBuiltinKind kind)
@@ -30,6 +31,9 @@ struct SemanticsNodeId : public IndexBase {
     return MakeCrossReference(kind.AsInt());
   }
 
+  // Constructs an explicitly invalid instance.
+  static auto MakeInvalid() -> SemanticsNodeId { return SemanticsNodeId(); }
+
   using IndexBase::IndexBase;
 
   auto is_cross_reference() const -> bool { return index & CrossReferenceBit; }

toolchain/semantics/semantics_parse_tree_handler.cpp

@@ -27,8 +27,8 @@ class SemanticsParseTreeHandler::PrettyStackTraceNodeStack
       const auto& entry = handler_->node_stack_[i];
       output << "\t" << i << ".\t"
              << handler_->parse_tree_->node_kind(entry.parse_node);
-      if (entry.result_id) {
-        output << " -> " << *entry.result_id;
+      if (entry.result_id.is_valid()) {
+        output << " -> " << entry.result_id;
       }
       output << "\n";
     }
@@ -139,7 +139,7 @@ auto SemanticsParseTreeHandler::Push(ParseTree::Node parse_node) -> void {
                 << parse_tree_->node_kind(parse_node) << "\n";
   CARBON_CHECK(node_stack_.size() < (1 << 20))
       << "Excessive stack size: likely infinite loop";
-  node_stack_.push_back({parse_node, std::nullopt});
+  node_stack_.push_back({parse_node, SemanticsNodeId::MakeInvalid()});
 }
 
 auto SemanticsParseTreeHandler::Push(ParseTree::Node parse_node,
@@ -170,15 +170,19 @@ auto SemanticsParseTreeHandler::Pop(ParseNodeKind pop_parse_kind) -> void {
                 << "\n";
   CARBON_CHECK(parse_kind == pop_parse_kind)
       << "Expected " << pop_parse_kind << ", found " << parse_kind;
-  CARBON_CHECK(!back.result_id) << "Expected no result ID on " << parse_kind;
+  CARBON_CHECK(!back.result_id.is_valid())
+      << "Expected no result ID on " << parse_kind << ", was "
+      << back.result_id;
 }
 
 auto SemanticsParseTreeHandler::PopWithResult() -> SemanticsNodeId {
   auto back = node_stack_.pop_back_val();
-  auto node_id = *back.result_id;
+  auto node_id = back.result_id;
   CARBON_VLOG() << "Pop " << node_stack_.size() << ": any ("
                 << parse_tree_->node_kind(back.parse_node) << ") -> " << node_id
                 << "\n";
+  CARBON_CHECK(node_id.is_valid())
+      << "Invalid PopWithResult on " << parse_tree_->node_kind(back.parse_node);
   return node_id;
 }
 
@@ -186,11 +190,13 @@ auto SemanticsParseTreeHandler::PopWithResult(ParseNodeKind pop_parse_kind)
     -> SemanticsNodeId {
   auto back = node_stack_.pop_back_val();
   auto parse_kind = parse_tree_->node_kind(back.parse_node);
-  auto node_id = *back.result_id;
+  auto node_id = back.result_id;
   CARBON_VLOG() << "Pop " << node_stack_.size() << ": " << pop_parse_kind
                 << ") -> " << node_id << "\n";
   CARBON_CHECK(parse_kind == pop_parse_kind)
       << "Expected " << pop_parse_kind << ", found " << parse_kind;
+  CARBON_CHECK(node_id.is_valid())
+      << "Invalid PopWithResult with " << parse_kind;
   return node_id;
 }
 

toolchain/semantics/semantics_parse_tree_handler.h

@@ -37,8 +37,11 @@ class SemanticsParseTreeHandler {
 
   struct TraversalStackEntry {
     ParseTree::Node parse_node;
-    std::optional<SemanticsNodeId> result_id;
+    // The result_id may be invalid if there's no result.
+    SemanticsNodeId result_id;
   };
+  static_assert(sizeof(TraversalStackEntry) == 8,
+                "Unexpected TraversalStackEntry size");
 
   // Adds an identifier for a DeclaredName node, returning its reference.
   auto AddIdentifier(ParseTree::Node decl_node) -> SemanticsIdentifierId;