Factor param/arg ref logic to a class. (#3728)

Just segmenting out a chunk of logic while I'm thinking about function
parameters.

toolchain/check/BUILD

@@ -68,6 +68,7 @@ cc_library(
         "eval.h",
         "inst_block_stack.h",
         "modifiers.h",
+        "param_and_arg_refs_stack.h",
         "pending_block.h",
         "return.h",
     ],

toolchain/check/context.cpp

@@ -36,7 +36,7 @@ Context::Context(const Lex::TokenizedBuffer& tokens, DiagnosticEmitter& emitter,
       vlog_stream_(vlog_stream),
       node_stack_(parse_tree, vlog_stream),
       inst_block_stack_("inst_block_stack_", sem_ir, vlog_stream),
-      params_or_args_stack_("params_or_args_stack_", sem_ir, vlog_stream),
+      param_and_arg_refs_stack_(sem_ir, vlog_stream, node_stack_),
       args_type_info_stack_("args_type_info_stack_", sem_ir, vlog_stream),
       decl_name_stack_(this),
       scope_stack_(sem_ir_->identifiers()) {
@@ -63,8 +63,8 @@ auto Context::VerifyOnFinish() -> void {
   // remain.
   // node_stack_ will still contain top-level entities.
   scope_stack_.VerifyOnFinish();
-  CARBON_CHECK(inst_block_stack_.empty()) << inst_block_stack_.size();
-  CARBON_CHECK(params_or_args_stack_.empty()) << params_or_args_stack_.size();
+  inst_block_stack_.VerifyOnFinish();
+  param_and_arg_refs_stack_.VerifyOnFinish();
 }
 
 auto Context::AddInstInNoBlock(SemIR::ParseNodeAndInst parse_node_and_inst)
@@ -476,31 +476,6 @@ auto Context::is_current_position_reachable() -> bool {
          SemIR::TerminatorKind::Terminator;
 }
 
-auto Context::ParamOrArgStart() -> void { params_or_args_stack_.Push(); }
-
-auto Context::ParamOrArgComma() -> void {
-  // Support expressions, parameters, and other nodes like `StructFieldValue`
-  // that produce InstIds.
-  ParamOrArgSave(node_stack_.Pop<SemIR::InstId>());
-}
-
-auto Context::ParamOrArgEndNoPop(Parse::NodeKind start_kind) -> void {
-  if (!node_stack_.PeekIs(start_kind)) {
-    // Support expressions, parameters, and other nodes like `StructFieldValue`
-    // that produce InstIds.
-    ParamOrArgSave(node_stack_.Pop<SemIR::InstId>());
-  }
-}
-
-auto Context::ParamOrArgPop() -> SemIR::InstBlockId {
-  return params_or_args_stack_.Pop();
-}
-
-auto Context::ParamOrArgEnd(Parse::NodeKind start_kind) -> SemIR::InstBlockId {
-  ParamOrArgEndNoPop(start_kind);
-  return ParamOrArgPop();
-}
-
 auto Context::FinalizeGlobalInit() -> void {
   inst_block_stack().PushGlobalInit();
   if (!inst_block_stack().PeekCurrentBlockContents().empty()) {
@@ -1036,7 +1011,7 @@ auto Context::GetUnqualifiedType(SemIR::TypeId type_id) -> SemIR::TypeId {
 auto Context::PrintForStackDump(llvm::raw_ostream& output) const -> void {
   node_stack_.PrintForStackDump(output);
   inst_block_stack_.PrintForStackDump(output);
-  params_or_args_stack_.PrintForStackDump(output);
+  param_and_arg_refs_stack_.PrintForStackDump(output);
   args_type_info_stack_.PrintForStackDump(output);
 }
 

toolchain/check/context.h

@@ -12,6 +12,7 @@
 #include "toolchain/check/decl_state.h"
 #include "toolchain/check/inst_block_stack.h"
 #include "toolchain/check/node_stack.h"
+#include "toolchain/check/param_and_arg_refs_stack.h"
 #include "toolchain/check/scope_stack.h"
 #include "toolchain/parse/node_ids.h"
 #include "toolchain/parse/tree.h"
@@ -253,34 +254,6 @@ class Context {
   // Removes any top-level `const` qualifiers from a type.
   auto GetUnqualifiedType(SemIR::TypeId type_id) -> SemIR::TypeId;
 
-  // Starts handling parameters or arguments.
-  auto ParamOrArgStart() -> void;
-
-  // On a comma, pushes the entry. On return, the top of node_stack_ will be
-  // start_kind.
-  auto ParamOrArgComma() -> void;
-
-  // Detects whether there's an entry to push from the end of a parameter or
-  // argument list, and if so, moves it to the current parameter or argument
-  // list. Does not pop the list. `start_kind` is the node kind at the start
-  // of the parameter or argument list, and will be at the top of the parse node
-  // stack when this function returns.
-  auto ParamOrArgEndNoPop(Parse::NodeKind start_kind) -> void;
-
-  // Pops the current parameter or argument list. Should only be called after
-  // `ParamOrArgEndNoPop`.
-  auto ParamOrArgPop() -> SemIR::InstBlockId;
-
-  // Detects whether there's an entry to push. Pops and returns the argument
-  // list. This is the same as `ParamOrArgEndNoPop` followed by `ParamOrArgPop`.
-  auto ParamOrArgEnd(Parse::NodeKind start_kind) -> SemIR::InstBlockId;
-
-  // Saves a parameter from the top block in node_stack_ to the top block in
-  // params_or_args_stack_.
-  auto ParamOrArgSave(SemIR::InstId inst_id) -> void {
-    params_or_args_stack_.AddInstId(inst_id);
-  }
-
   // Adds an exported name.
   auto AddExport(SemIR::InstId inst_id) -> void { exports_.push_back(inst_id); }
 
@@ -312,8 +285,8 @@ class Context {
 
   auto inst_block_stack() -> InstBlockStack& { return inst_block_stack_; }
 
-  auto params_or_args_stack() -> InstBlockStack& {
-    return params_or_args_stack_;
+  auto param_and_arg_refs_stack() -> ParamAndArgRefsStack& {
+    return param_and_arg_refs_stack_;
   }
 
   auto args_type_info_stack() -> InstBlockStack& {
@@ -418,16 +391,14 @@ class Context {
   // The stack of instruction blocks being used for general IR generation.
   InstBlockStack inst_block_stack_;
 
-  // The stack of instruction blocks being used for per-element tracking of
-  // instructions in parameter and argument instruction blocks. Versus
-  // inst_block_stack_, an element will have 1 or more instructions in blocks in
-  // inst_block_stack_, but only ever 1 instruction in blocks here.
-  InstBlockStack params_or_args_stack_;
+  // The stack of instruction blocks being used for param and arg ref blocks.
+  ParamAndArgRefsStack param_and_arg_refs_stack_;
 
   // The stack of instruction blocks being used for type information while
-  // processing arguments. This is used in parallel with params_or_args_stack_.
-  // It's currently only used for struct literals, where we need to track names
-  // for a type separate from the literal arguments.
+  // processing arguments. This is used in parallel with
+  // param_and_arg_refs_stack_. It's currently only used for struct literals,
+  // where we need to track names for a type separate from the literal
+  // arguments.
   InstBlockStack args_type_info_stack_;
 
   // The stack used for qualified declaration name construction.

toolchain/check/handle_call_expr.cpp

@@ -13,22 +13,22 @@ auto HandleCallExprStart(Context& context, Parse::CallExprStartId parse_node)
     -> bool {
   auto name_id = context.node_stack().PopExpr();
   context.node_stack().Push(parse_node, name_id);
-  context.ParamOrArgStart();
+  context.param_and_arg_refs_stack().Push();
   return true;
 }
 
 auto HandleCallExprComma(Context& context,
                          Parse::CallExprCommaId /*parse_node*/) -> bool {
-  context.ParamOrArgComma();
+  context.param_and_arg_refs_stack().ApplyComma();
   return true;
 }
 
 auto HandleCallExpr(Context& context, Parse::CallExprId parse_node) -> bool {
   // Process the final explicit call argument now, but leave the arguments
   // block on the stack until the end of this function.
-  context.ParamOrArgEndNoPop(Parse::NodeKind::CallExprStart);
+  context.param_and_arg_refs_stack().EndNoPop(Parse::NodeKind::CallExprStart);
   auto discard_args_block = llvm::make_scope_exit(
-      [&] { context.params_or_args_stack().PopAndDiscard(); });
+      [&] { context.param_and_arg_refs_stack().PopAndDiscard(); });
 
   auto [call_expr_parse_node, callee_id] =
       context.node_stack().PopWithParseNode<Parse::NodeKind::CallExprStart>();
@@ -87,11 +87,11 @@ auto HandleCallExpr(Context& context, Parse::CallExprId parse_node) -> bool {
   }
 
   // Convert the arguments to match the parameters.
-  auto converted_args_id =
-      ConvertCallArgs(context, call_expr_parse_node, self_id,
-                      context.params_or_args_stack().PeekCurrentBlockContents(),
-                      return_storage_id, function_decl_id.inst_id(),
-                      callable.implicit_param_refs_id, callable.param_refs_id);
+  auto converted_args_id = ConvertCallArgs(
+      context, call_expr_parse_node, self_id,
+      context.param_and_arg_refs_stack().PeekCurrentBlockContents(),
+      return_storage_id, function_decl_id.inst_id(),
+      callable.implicit_param_refs_id, callable.param_refs_id);
   auto call_inst_id =
       context.AddInst({call_expr_parse_node,
                        SemIR::Call{type_id, callee_id, converted_args_id}});

toolchain/check/handle_paren.cpp

@@ -9,15 +9,17 @@ namespace Carbon::Check {
 auto HandleExprOpenParen(Context& context, Parse::ExprOpenParenId parse_node)
     -> bool {
   context.node_stack().Push(parse_node);
-  context.ParamOrArgStart();
+  context.param_and_arg_refs_stack().Push();
   return true;
 }
 
 auto HandleParenExpr(Context& context, Parse::ParenExprId parse_node) -> bool {
   auto value_id = context.node_stack().PopExpr();
-  // ParamOrArgStart was called for tuple handling; clean up the ParamOrArg
-  // support for non-tuple cases.
-  context.ParamOrArgEnd(Parse::NodeKind::ExprOpenParen);
+
+  // This always is always pushed at the open paren. It's only used for tuples,
+  // not paren exprs, but we still need to clean up.
+  context.param_and_arg_refs_stack().PopAndDiscard();
+
   context.node_stack()
       .PopAndDiscardSoloParseNode<Parse::NodeKind::ExprOpenParen>();
   context.node_stack().Push(parse_node, value_id);
@@ -27,13 +29,14 @@ auto HandleParenExpr(Context& context, Parse::ParenExprId parse_node) -> bool {
 auto HandleTupleLiteralComma(Context& context,
                              Parse::TupleLiteralCommaId /*parse_node*/)
     -> bool {
-  context.ParamOrArgComma();
+  context.param_and_arg_refs_stack().ApplyComma();
   return true;
 }
 
 auto HandleTupleLiteral(Context& context, Parse::TupleLiteralId parse_node)
     -> bool {
-  auto refs_id = context.ParamOrArgEnd(Parse::NodeKind::ExprOpenParen);
+  auto refs_id = context.param_and_arg_refs_stack().EndAndPop(
+      Parse::NodeKind::ExprOpenParen);
 
   context.node_stack()
       .PopAndDiscardSoloParseNode<Parse::NodeKind::ExprOpenParen>();

toolchain/check/handle_pattern_list.cpp

@@ -10,13 +10,14 @@ auto HandleImplicitParamListStart(Context& context,
                                   Parse::ImplicitParamListStartId parse_node)
     -> bool {
   context.node_stack().Push(parse_node);
-  context.ParamOrArgStart();
+  context.param_and_arg_refs_stack().Push();
   return true;
 }
 
 auto HandleImplicitParamList(Context& context,
                              Parse::ImplicitParamListId parse_node) -> bool {
-  auto refs_id = context.ParamOrArgEnd(Parse::NodeKind::ImplicitParamListStart);
+  auto refs_id = context.param_and_arg_refs_stack().EndAndPop(
+      Parse::NodeKind::ImplicitParamListStart);
   context.node_stack()
       .PopAndDiscardSoloParseNode<Parse::NodeKind::ImplicitParamListStart>();
   context.node_stack().Push(parse_node, refs_id);
@@ -28,19 +29,20 @@ auto HandleImplicitParamList(Context& context,
 auto HandleTuplePatternStart(Context& context,
                              Parse::TuplePatternStartId parse_node) -> bool {
   context.node_stack().Push(parse_node);
-  context.ParamOrArgStart();
+  context.param_and_arg_refs_stack().Push();
   return true;
 }
 
 auto HandlePatternListComma(Context& context,
                             Parse::PatternListCommaId /*parse_node*/) -> bool {
-  context.ParamOrArgComma();
+  context.param_and_arg_refs_stack().ApplyComma();
   return true;
 }
 
 auto HandleTuplePattern(Context& context, Parse::TuplePatternId parse_node)
     -> bool {
-  auto refs_id = context.ParamOrArgEnd(Parse::NodeKind::TuplePatternStart);
+  auto refs_id = context.param_and_arg_refs_stack().EndAndPop(
+      Parse::NodeKind::TuplePatternStart);
   context.node_stack()
       .PopAndDiscardSoloParseNode<Parse::NodeKind::TuplePatternStart>();
   context.node_stack().Push(parse_node, refs_id);

toolchain/check/handle_struct.cpp

@@ -16,7 +16,7 @@ auto HandleStructLiteralOrStructTypeLiteralStart(
   // so we push onto args irrespective. It just won't be used for a type
   // literal.
   context.args_type_info_stack().Push();
-  context.ParamOrArgStart();
+  context.param_and_arg_refs_stack().Push();
   return true;
 }
 
@@ -30,7 +30,7 @@ auto HandleStructFieldDesignator(Context& context,
 
 auto HandleStructComma(Context& context, Parse::StructCommaId /*parse_node*/)
     -> bool {
-  context.ParamOrArgComma();
+  context.param_and_arg_refs_stack().ApplyComma();
   return true;
 }
 
@@ -91,7 +91,7 @@ static auto DiagnoseDuplicateNames(Context& context,
 
 auto HandleStructLiteral(Context& context, Parse::StructLiteralId parse_node)
     -> bool {
-  auto refs_id = context.ParamOrArgEnd(
+  auto refs_id = context.param_and_arg_refs_stack().EndAndPop(
       Parse::NodeKind::StructLiteralOrStructTypeLiteralStart);
 
   context.scope_stack().Pop();
@@ -114,7 +114,7 @@ auto HandleStructLiteral(Context& context, Parse::StructLiteralId parse_node)
 
 auto HandleStructTypeLiteral(Context& context,
                              Parse::StructTypeLiteralId parse_node) -> bool {
-  auto refs_id = context.ParamOrArgEnd(
+  auto refs_id = context.param_and_arg_refs_stack().EndAndPop(
       Parse::NodeKind::StructLiteralOrStructTypeLiteralStart);
 
   context.scope_stack().Pop();

toolchain/check/inst_block_stack.cpp

@@ -27,8 +27,8 @@ auto InstBlockStack::PushGlobalInit() -> void {
 }
 
 auto InstBlockStack::PeekOrAdd(int depth) -> SemIR::InstBlockId {
-  CARBON_CHECK(size() > depth) << "no such block";
-  int index = size() - depth - 1;
+  CARBON_CHECK(size_ > depth) << "no such block";
+  int index = size_ - depth - 1;
   auto& slot = stack_[index];
   if (!slot.id.is_valid()) {
     slot.id = sem_ir_->inst_blocks().AddDefaultValue();

toolchain/check/inst_block_stack.h

@@ -79,8 +79,10 @@ class InstBlockStack {
   // Prints the stack for a stack dump.
   auto PrintForStackDump(llvm::raw_ostream& output) const -> void;
 
-  auto empty() const -> bool { return size() == 0; }
-  auto size() const -> int { return size_; }
+  // Runs verification that the processing cleanly finished.
+  auto VerifyOnFinish() const -> void { CARBON_CHECK(empty()) << size_; }
+
+  auto empty() const -> bool { return size_ == 0; }
 
  private:
   struct StackEntry {

toolchain/check/param_and_arg_refs_stack.h

@@ -0,0 +1,89 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#ifndef CARBON_TOOLCHAIN_CHECK_PARAM_AND_ARG_REFS_STACK_H_
+#define CARBON_TOOLCHAIN_CHECK_PARAM_AND_ARG_REFS_STACK_H_
+
+#include "common/check.h"
+#include "toolchain/check/inst_block_stack.h"
+#include "toolchain/check/node_stack.h"
+
+namespace Carbon::Check {
+
+// The stack of instruction blocks being used for per-element tracking of
+// instructions in parameter and argument instruction blocks. Versus
+// InstBlockStack, an element will have 1 or more instructions in blocks in
+// InstBlockStack, but only ever 1 instruction in blocks here. The result is
+// typically referred to as "param_refs" or "arg_refs".
+class ParamAndArgRefsStack {
+ public:
+  explicit ParamAndArgRefsStack(SemIR::File& sem_ir,
+                                llvm::raw_ostream* vlog_stream,
+                                NodeStack& node_stack)
+      : node_stack_(&node_stack),
+        stack_("param_and_arg_refs_stack", sem_ir, vlog_stream) {}
+
+  // Starts handling parameters or arguments.
+  auto Push() -> void { stack_.Push(); }
+
+  // On a comma, pushes the most recent instruction, becoming param or arg ref.
+  // This also pops the NodeStack, meaning its top will remain start_kind.
+  auto ApplyComma() -> void {
+    // Support expressions, parameters, and other nodes like `StructFieldValue`
+    // that produce InstIds.
+    stack_.AddInstId(node_stack_->Pop<SemIR::InstId>());
+  }
+
+  // Detects whether there's an entry to push from the end of a parameter or
+  // argument list, and if so, moves it to the current parameter or argument
+  // list. Does not pop the list. `start_kind` is the node kind at the start
+  // of the parameter or argument list, and will be at the top of the parse node
+  // stack when this function returns.
+  auto EndNoPop(Parse::NodeKind start_kind) -> void {
+    if (!node_stack_->PeekIs(start_kind)) {
+      // Support expressions, parameters, and other nodes like
+      // `StructFieldValue` that produce InstIds.
+      stack_.AddInstId(node_stack_->Pop<SemIR::InstId>());
+    }
+  }
+
+  // Pops the current parameter or argument list. Should only be called after
+  // `EndNoPop`.
+  auto Pop() -> SemIR::InstBlockId { return stack_.Pop(); }
+
+  // Detects whether there's an entry to push. Pops and returns the argument
+  // list. This is the same as `EndNoPop` followed by `Pop`.
+  auto EndAndPop(Parse::NodeKind start_kind) -> SemIR::InstBlockId {
+    EndNoPop(start_kind);
+    return Pop();
+  }
+
+  // Pops the top instruction block, and discards it if it hasn't had an ID
+  // allocated.
+  auto PopAndDiscard() -> void { stack_.PopAndDiscard(); }
+
+  // Returns a view of the contents of the top instruction block on the stack.
+  auto PeekCurrentBlockContents() -> llvm::ArrayRef<SemIR::InstId> {
+    return stack_.PeekCurrentBlockContents();
+  }
+
+  // Runs verification that the processing cleanly finished.
+  auto VerifyOnFinish() -> void { stack_.VerifyOnFinish(); }
+
+  // Prints the stack for a stack dump.
+  auto PrintForStackDump(llvm::raw_ostream& output) const -> void {
+    return stack_.PrintForStackDump(output);
+  }
+
+ private:
+  // The node stack is manipulated when adding refs.
+  NodeStack* node_stack_;
+
+  // The refs stack.
+  InstBlockStack stack_;
+};
+
+}  // namespace Carbon::Check
+
+#endif  // CARBON_TOOLCHAIN_CHECK_PARAM_AND_ARG_REFS_STACK_H_