From 2fee4d072feb001684cd7df6eec01aa4ec4ffe64 Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Tue, 27 Feb 2024 11:58:19 -0800 Subject: [PATCH] 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 | 1 + toolchain/check/context.cpp | 33 +------- toolchain/check/context.h | 47 +++--------- toolchain/check/handle_call_expr.cpp | 18 ++--- toolchain/check/handle_paren.cpp | 15 ++-- toolchain/check/handle_pattern_list.cpp | 12 +-- toolchain/check/handle_struct.cpp | 8 +- toolchain/check/inst_block_stack.cpp | 4 +- toolchain/check/inst_block_stack.h | 6 +- toolchain/check/param_and_arg_refs_stack.h | 89 ++++++++++++++++++++++ 10 files changed, 138 insertions(+), 95 deletions(-) create mode 100644 toolchain/check/param_and_arg_refs_stack.h diff --git a/toolchain/check/BUILD b/toolchain/check/BUILD index e7824d0ea772..1eb437c1ff77 100644 --- a/toolchain/check/BUILD +++ b/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", ], diff --git a/toolchain/check/context.cpp b/toolchain/check/context.cpp index 55bcb4298b0c..f55c1d8ff5df 100644 --- a/toolchain/check/context.cpp +++ b/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()); -} - -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()); - } -} - -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); } diff --git a/toolchain/check/context.h b/toolchain/check/context.h index 57b761ede1be..134b5ff46ca1 100644 --- a/toolchain/check/context.h +++ b/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. diff --git a/toolchain/check/handle_call_expr.cpp b/toolchain/check/handle_call_expr.cpp index b01d6c997dfa..2a95e7c25477 100644 --- a/toolchain/check/handle_call_expr.cpp +++ b/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(); @@ -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}}); diff --git a/toolchain/check/handle_paren.cpp b/toolchain/check/handle_paren.cpp index 10e8c8807f01..73b438b63008 100644 --- a/toolchain/check/handle_paren.cpp +++ b/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(); 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(); diff --git a/toolchain/check/handle_pattern_list.cpp b/toolchain/check/handle_pattern_list.cpp index 4e72b71f217c..df27a9a47c7b 100644 --- a/toolchain/check/handle_pattern_list.cpp +++ b/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(); 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(); context.node_stack().Push(parse_node, refs_id); diff --git a/toolchain/check/handle_struct.cpp b/toolchain/check/handle_struct.cpp index 59a75f8e50ea..4d8a9ee7ad5c 100644 --- a/toolchain/check/handle_struct.cpp +++ b/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(); diff --git a/toolchain/check/inst_block_stack.cpp b/toolchain/check/inst_block_stack.cpp index cf3e2dec5fda..84e5ae8b308f 100644 --- a/toolchain/check/inst_block_stack.cpp +++ b/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(); diff --git a/toolchain/check/inst_block_stack.h b/toolchain/check/inst_block_stack.h index da3d78040b7e..186340d6655e 100644 --- a/toolchain/check/inst_block_stack.h +++ b/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 { diff --git a/toolchain/check/param_and_arg_refs_stack.h b/toolchain/check/param_and_arg_refs_stack.h new file mode 100644 index 000000000000..c3d131f094d8 --- /dev/null +++ b/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()); + } + + // 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()); + } + } + + // 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 { + 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_