From 32a9976d52714c7ece1e4ed98afc1bf2e7068967 Mon Sep 17 00:00:00 2001
From: danakj <danakj@orodu.net>
Date: Mon, 20 Jan 2025 11:15:20 -0500
Subject: [PATCH] Avoid walking the node stack

---
 toolchain/check/handle_binding_pattern.cpp |  7 ++++---
 toolchain/check/handle_choice.cpp          |  7 ++-----
 toolchain/check/node_stack.h               | 14 --------------
 3 files changed, 6 insertions(+), 22 deletions(-)

diff --git a/toolchain/check/handle_binding_pattern.cpp b/toolchain/check/handle_binding_pattern.cpp
index 3b72a64850843..803ea1437a441 100644
--- a/toolchain/check/handle_binding_pattern.cpp
+++ b/toolchain/check/handle_binding_pattern.cpp
@@ -181,10 +181,11 @@ static auto HandleAnyBindingPattern(Context& context, Parse::NodeId node_id,
           break;
         }
         case Lex::TokenKind::Choice:
-          if (context.node_stack()
-                  .PeekContains<Parse::NodeKind::ChoiceDefinitionStart>()) {
+          if (context.scope_stack().PeekInstId().is_valid()) {
             // We're building a pattern for a choice alternative, not the choice
-            // type itself.
+            // type itself. The Choice itself has an invalid instruction id on
+            // the scope stack, but the Choice's declaration instruction is
+            // present and valid for checking the alternatives.
             if (context_node_kind == Parse::NodeKind::ImplicitParamListStart) {
               CARBON_DIAGNOSTIC(ChoiceAlternativeImplicitParams, Error,
                                 "choice alternative with implicit parameters");
diff --git a/toolchain/check/handle_choice.cpp b/toolchain/check/handle_choice.cpp
index 75575d8342ea7..1852bb2ff9301 100644
--- a/toolchain/check/handle_choice.cpp
+++ b/toolchain/check/handle_choice.cpp
@@ -113,11 +113,8 @@ auto HandleParseNode(Context& context, Parse::ChoiceDefinitionStartId node_id)
   context.scope_stack().Push(class_decl_id, class_info.scope_id,
                              self_specific_id);
   // Checking the binding pattern for an alternative requires a non-empty stack.
-  // FIXME: `Lex::TokenKind::Choice` is incorrect as we're not parsing the
-  // pattern for a choice name, but there's no Lex token that's a decl
-  // introducer that we could safely use here. Is there a better way to
-  // communicate to `HandleAnyBindingPattern` that we're checking a choice
-  // alternative?
+  // We reuse the `Choice` token kind, along with the scope_stac() entry to let
+  // the pattern checking determine we are building a choice alternative.
   context.decl_introducer_state_stack().Push<Lex::TokenKind::Choice>();
   StartGenericDefinition(context);
 
diff --git a/toolchain/check/node_stack.h b/toolchain/check/node_stack.h
index 188466c68386f..92fd55f5eba7d 100644
--- a/toolchain/check/node_stack.h
+++ b/toolchain/check/node_stack.h
@@ -145,20 +145,6 @@ class NodeStack {
            RequiredParseKind;
   }
 
-  // Returns whether any node on the stack above the current node is a given
-  // kind.
-  template <const Parse::NodeKind& RequiredParseKind>
-  auto PeekContains() const -> bool {
-    CARBON_CHECK(stack_.size() >= 2);
-    for (int i = 0; i < static_cast<int>(stack_.size()) - 1; ++i) {
-      if (parse_tree_->node_kind(stack_[stack_.size() - 2 - i].node_id) ==
-          RequiredParseKind) {
-        return true;
-      }
-    }
-    return false;
-  }
-
   // Pops the top of the stack without any verification.
   auto PopAndIgnore() -> void {
     Entry back = stack_.pop_back_val();