From f7269482fe6738a511079977d2baa4bf3ad0511e Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Wed, 15 Jan 2025 14:12:22 -0800 Subject: [PATCH] Remove node_stack Peek templating where possible (#4801) The "templated for consistency" variants felt a little confusing when I was working on #4795, so suggesting to remove templating where it's not helpful to instantiate (particularly when there were both templated and non-templated variants). Note this leaves a `PeekIs` because there's indirection there, but that's more the exception than the rule. --- toolchain/check/handle_binding_pattern.cpp | 2 +- toolchain/check/handle_let_and_var.cpp | 4 +-- toolchain/check/handle_pattern_list.cpp | 2 +- toolchain/check/node_stack.h | 33 ++++++---------------- 4 files changed, 12 insertions(+), 29 deletions(-) diff --git a/toolchain/check/handle_binding_pattern.cpp b/toolchain/check/handle_binding_pattern.cpp index 569406b12aea..0bf456df68e2 100644 --- a/toolchain/check/handle_binding_pattern.cpp +++ b/toolchain/check/handle_binding_pattern.cpp @@ -77,7 +77,7 @@ static auto HandleAnyBindingPattern(Context& context, Parse::NodeId node_id, // A `self` binding can only appear in an implicit parameter list. if (name_id == SemIR::NameId::SelfValue && - !context.node_stack().PeekIs()) { + !context.node_stack().PeekIs(Parse::NodeKind::ImplicitParamListStart)) { CARBON_DIAGNOSTIC( SelfOutsideImplicitParamList, Error, "`self` can only be declared in an implicit parameter list"); diff --git a/toolchain/check/handle_let_and_var.cpp b/toolchain/check/handle_let_and_var.cpp index 623a482e9fb0..f07c4fcd373b 100644 --- a/toolchain/check/handle_let_and_var.cpp +++ b/toolchain/check/handle_let_and_var.cpp @@ -153,12 +153,12 @@ static auto HandleDecl(Context& context, NodeT node_id) context.pattern_block_stack().PopAndDiscard(); // Handle the optional initializer. - if (context.node_stack().PeekNextIs()) { + if (context.node_stack().PeekNextIs(InitializerNodeKind)) { decl_info->init_id = context.node_stack().PopExpr(); context.node_stack().PopAndDiscardSoloNodeId(); } - if (context.node_stack().PeekIs()) { + if (context.node_stack().PeekIs(Parse::NodeKind::TuplePattern)) { if (decl_info->init_id && context.scope_stack().PeekIndex() == ScopeIndex::Package) { context.global_init().Suspend(); diff --git a/toolchain/check/handle_pattern_list.cpp b/toolchain/check/handle_pattern_list.cpp index 19049dbe8192..b65324ff69d5 100644 --- a/toolchain/check/handle_pattern_list.cpp +++ b/toolchain/check/handle_pattern_list.cpp @@ -17,7 +17,7 @@ auto HandleParseNode(Context& context, Parse::ImplicitParamListStartId node_id) auto HandleParseNode(Context& context, Parse::ImplicitParamListId node_id) -> bool { - if (context.node_stack().PeekIs()) { + if (context.node_stack().PeekIs(Parse::NodeKind::ImplicitParamListStart)) { // End the subpattern started by a trailing comma, or the opening delimiter // of an empty list. context.EndSubpatternAsEmpty(); diff --git a/toolchain/check/node_stack.h b/toolchain/check/node_stack.h index 27df2a163c34..edcbe9018349 100644 --- a/toolchain/check/node_stack.h +++ b/toolchain/check/node_stack.h @@ -105,27 +105,12 @@ class NodeStack { return !stack_.empty() && PeekNodeKind() == kind; } - // Returns whether there is a node of the specified kind on top of the stack. - // Templated for consistency with other functions taking a parse node kind. - template - auto PeekIs() const -> bool { - return PeekIs(RequiredParseKind); - } - // Returns whether the node on the top of the stack has an overlapping // category. auto PeekIs(Parse::NodeCategory category) const -> bool { return !stack_.empty() && PeekNodeKind().category().HasAnyOf(category); } - // Returns whether the node on the top of the stack has an overlapping - // category. Templated for consistency with other functions taking a parse - // node category. - template - auto PeekIs() const -> bool { - return PeekIs(RequiredParseCategory); - } - // Returns whether there is a node with the corresponding ID on top of the // stack. template @@ -138,11 +123,9 @@ class NodeStack { // have the breadth of support versus other Peek functions because it's // expected to be used in narrow circumstances when determining how to treat // the *current* top of the stack. - template - auto PeekNextIs() const -> bool { + auto PeekNextIs(Parse::NodeKind kind) const -> bool { CARBON_CHECK(stack_.size() >= 2); - return parse_tree_->node_kind(stack_[stack_.size() - 2].node_id) == - RequiredParseKind; + return parse_tree_->node_kind(stack_[stack_.size() - 2].node_id) == kind; } // Pops the top of the stack without any verification. @@ -166,7 +149,7 @@ class NodeStack { template auto PopForSoloNodeIdIf() -> std::optional> { - if (PeekIs()) { + if (PeekIs(RequiredParseKind)) { return PopForSoloNodeId(); } return std::nullopt; @@ -182,7 +165,7 @@ class NodeStack { // was popped. template auto PopAndDiscardSoloNodeIdIf() -> bool { - if (!PeekIs()) { + if (!PeekIs(RequiredParseKind)) { return false; } PopForSoloNodeId(); @@ -256,7 +239,7 @@ class NodeStack { // Otherwise returns std::nullopt. template auto PopIf() -> std::optional())> { - if (PeekIs()) { + if (PeekIs(RequiredParseKind)) { return Pop(); } return std::nullopt; @@ -266,7 +249,7 @@ class NodeStack { // Otherwise returns std::nullopt. template auto PopIf() -> std::optional())> { - if (PeekIs()) { + if (PeekIs(RequiredParseCategory)) { return Pop(); } return std::nullopt; @@ -287,7 +270,7 @@ class NodeStack { template auto PopWithNodeIdIf() -> std::pair, decltype(PopIf())> { - if (!PeekIs()) { + if (!PeekIs(RequiredParseKind)) { return {Parse::NodeId::Invalid, std::nullopt}; } return PopWithNodeId(); @@ -299,7 +282,7 @@ class NodeStack { auto PopWithNodeIdIf() -> std::pair, decltype(PopIf())> { - if (!PeekIs()) { + if (!PeekIs(RequiredParseCategory)) { return {Parse::NodeId::Invalid, std::nullopt}; } return PopWithNodeId();