Refactor subpattern logic out of Context (#4929)

Note this is building on #4927 which factors out RegionStack, because
this heavily uses the region stack. However, it's a more complex level
of logic that appears specific to pattern handling.

I've moved InsertHere to pattern_match.cpp because it appears to be in
specific use there.

toolchain/check/BUILD

@@ -39,6 +39,7 @@ cc_library(
         "operator.cpp",
         "pattern_match.cpp",
         "return.cpp",
+        "subpattern.cpp",
         "subst.cpp",
         "type_completion.cpp",
     ],
@@ -73,6 +74,7 @@ cc_library(
         "pending_block.h",
         "region_stack.h",
         "return.h",
+        "subpattern.h",
         "subst.h",
         "type_completion.h",
     ],

toolchain/check/context.cpp

@@ -747,78 +747,6 @@ auto Context::LookupNameInCore(SemIR::LocId loc_id, llvm::StringRef name)
   return constant_values().GetConstantInstId(scope_result.target_inst_id());
 }
 
-auto Context::BeginSubpattern() -> void {
-  inst_block_stack().Push();
-  region_stack_.PushRegion(inst_block_stack().PeekOrAdd());
-}
-
-auto Context::EndSubpatternAsExpr(SemIR::InstId result_id)
-    -> SemIR::ExprRegionId {
-  if (region_stack_.PeekRegion().size() > 1) {
-    // End the exit block with a branch to a successor block, whose contents
-    // will be determined later.
-    AddInst(SemIR::LocIdAndInst::NoLoc<SemIR::Branch>(
-        {.target_id = inst_blocks().AddDefaultValue()}));
-  } else {
-    // This single-block region will be inserted as a SpliceBlock, so we don't
-    // need control flow out of it.
-  }
-  auto block_id = inst_block_stack().Pop();
-  CARBON_CHECK(block_id == region_stack_.PeekRegion().back());
-
-  // TODO: Is it possible to validate that this region is genuinely
-  // single-entry, single-exit?
-  return sem_ir().expr_regions().Add(
-      {.block_ids = region_stack_.PopRegion(), .result_id = result_id});
-}
-
-auto Context::EndSubpatternAsEmpty() -> void {
-  auto block_id = inst_block_stack().Pop();
-  CARBON_CHECK(block_id == region_stack_.PeekRegion().back());
-  CARBON_CHECK(region_stack_.PeekRegion().size() == 1);
-  CARBON_CHECK(inst_blocks().Get(block_id).empty());
-  region_stack_.PopAndDiscardRegion();
-}
-
-auto Context::InsertHere(SemIR::ExprRegionId region_id) -> SemIR::InstId {
-  auto region = sem_ir_->expr_regions().Get(region_id);
-  auto loc_id = insts().GetLocId(region.result_id);
-  auto exit_block = inst_blocks().Get(region.block_ids.back());
-  if (region.block_ids.size() == 1) {
-    // TODO: Is it possible to avoid leaving an "orphan" block in the IR in the
-    // first two cases?
-    if (exit_block.empty()) {
-      return region.result_id;
-    }
-    if (exit_block.size() == 1) {
-      inst_block_stack_.AddInstId(exit_block.front());
-      return region.result_id;
-    }
-    return AddInst<SemIR::SpliceBlock>(
-        loc_id, {.type_id = insts().Get(region.result_id).type_id(),
-                 .block_id = region.block_ids.front(),
-                 .result_id = region.result_id});
-  }
-  if (region_stack_.empty()) {
-    TODO(loc_id,
-         "Control flow expressions are currently only supported inside "
-         "functions.");
-    return SemIR::ErrorInst::SingletonInstId;
-  }
-  AddInst(SemIR::LocIdAndInst::NoLoc<SemIR::Branch>(
-      {.target_id = region.block_ids.front()}));
-  inst_block_stack_.Pop();
-  // TODO: this will cumulatively cost O(MN) running time for M blocks
-  // at the Nth level of the stack. Figure out how to do better.
-  region_stack_.AddToRegion(region.block_ids);
-  auto resume_with_block_id =
-      insts().GetAs<SemIR::Branch>(exit_block.back()).target_id;
-  CARBON_CHECK(inst_blocks().GetOrEmpty(resume_with_block_id).empty());
-  inst_block_stack_.Push(resume_with_block_id);
-  region_stack_.AddToRegion(resume_with_block_id, loc_id);
-  return region.result_id;
-}
-
 auto Context::Finalize() -> void {
   // Pop information for the file-level scope.
   sem_ir().set_top_inst_block_id(inst_block_stack().Pop());

toolchain/check/context.h

@@ -543,32 +543,6 @@ class Context {
 
   auto global_init() -> GlobalInit& { return global_init_; }
 
-  // Marks the start of a region of insts in a pattern context that might
-  // represent an expression or a pattern. Typically this is called when
-  // handling a parse node that can immediately precede a subpattern (such
-  // as `let` or a `,` in a pattern list), and the handler for the subpattern
-  // node makes the matching `EndSubpatternAs*` call.
-  auto BeginSubpattern() -> void;
-
-  // Ends a region started by BeginSubpattern (in stack order), treating it as
-  // an expression with the given result, and returns the ID of the region. The
-  // region will not yet have any control-flow edges into or out of it.
-  auto EndSubpatternAsExpr(SemIR::InstId result_id) -> SemIR::ExprRegionId;
-
-  // Ends a region started by BeginSubpattern (in stack order), asserting that
-  // it was empty.
-  auto EndSubpatternAsEmpty() -> void;
-
-  // TODO: Add EndSubpatternAsPattern, when needed.
-
-  // Inserts the given region into the current code block. If the region
-  // consists of a single block, this will be implemented as a `splice_block`
-  // inst. Otherwise, this will end the current block with a branch to the entry
-  // block of the region, and add future insts to a new block which is the
-  // immediate successor of the region's exit block. As a result, this cannot be
-  // called more than once for the same region.
-  auto InsertHere(SemIR::ExprRegionId region_id) -> SemIR::InstId;
-
   auto import_ref_ids() -> llvm::SmallVector<SemIR::InstId>& {
     return import_ref_ids_;
   }

toolchain/check/handle_binding_pattern.cpp

@@ -7,6 +7,7 @@
 #include "toolchain/check/handle.h"
 #include "toolchain/check/interface.h"
 #include "toolchain/check/return.h"
+#include "toolchain/check/subpattern.h"
 #include "toolchain/check/type_completion.h"
 #include "toolchain/diagnostics/format_providers.h"
 #include "toolchain/sem_ir/ids.h"
@@ -24,7 +25,7 @@ static auto HandleAnyBindingPattern(Context& context, Parse::NodeId node_id,
   // TODO: Handle `_` bindings.
 
   SemIR::ExprRegionId type_expr_region_id =
-      context.EndSubpatternAsExpr(cast_type_inst_id);
+      EndSubpatternAsExpr(context, cast_type_inst_id);
 
   // Every other kind of pattern binding has a name.
   auto [name_node, name_id] = context.node_stack().PopNameWithNodeId();

toolchain/check/handle_let_and_var.cpp

@@ -12,6 +12,7 @@
 #include "toolchain/check/modifiers.h"
 #include "toolchain/check/pattern_match.h"
 #include "toolchain/check/return.h"
+#include "toolchain/check/subpattern.h"
 #include "toolchain/diagnostics/diagnostic_emitter.h"
 #include "toolchain/lex/token_kind.h"
 #include "toolchain/parse/node_kind.h"
@@ -82,7 +83,7 @@ static auto HandleIntroducer(Context& context, Parse::NodeId node_id) -> bool {
   context.pattern_block_stack().Push();
   context.full_pattern_stack().PushFullPattern(
       FullPatternStack::Kind::NameBindingDecl);
-  context.BeginSubpattern();
+  BeginSubpattern(context);
   return true;
 }
 
@@ -256,7 +257,7 @@ static auto HandleDecl(Context& context) -> DeclInfo {
     // associated with an InstBlockId on the node stack rather than an InstId,
     // and leave behind an entry on the subpattern stack and one on the node
     // stack.
-    context.EndSubpatternAsExpr(SemIR::ErrorInst::SingletonInstId);
+    EndSubpatternAsExpr(context, SemIR::ErrorInst::SingletonInstId);
     context.node_stack().PopForSoloNodeId<Parse::NodeKind::TuplePatternStart>();
     decl_info.pattern_id = SemIR::ErrorInst::SingletonInstId;
   } else {

toolchain/check/handle_pattern_list.cpp

@@ -4,6 +4,7 @@
 
 #include "toolchain/check/context.h"
 #include "toolchain/check/handle.h"
+#include "toolchain/check/subpattern.h"
 
 namespace Carbon::Check {
 
@@ -11,7 +12,7 @@ auto HandleParseNode(Context& context, Parse::ImplicitParamListStartId node_id)
     -> bool {
   context.node_stack().Push(node_id);
   context.param_and_arg_refs_stack().Push();
-  context.BeginSubpattern();
+  BeginSubpattern(context);
   return true;
 }
 
@@ -20,7 +21,7 @@ auto HandleParseNode(Context& context, Parse::ImplicitParamListId node_id)
   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();
+    EndSubpatternAsEmpty(context);
   }
   // Note the Start node remains on the stack, where the param list handler can
   // make use of it.
@@ -36,7 +37,7 @@ auto HandleParseNode(Context& context, Parse::TuplePatternStartId node_id)
     -> bool {
   context.node_stack().Push(node_id);
   context.param_and_arg_refs_stack().Push();
-  context.BeginSubpattern();
+  BeginSubpattern(context);
   // TODO: Remove this branch once the parse tree differentiates between
   // tuple patterns and param patterns.
   if (context.full_pattern_stack().CurrentKind() ==
@@ -49,7 +50,7 @@ auto HandleParseNode(Context& context, Parse::TuplePatternStartId node_id)
 auto HandleParseNode(Context& context, Parse::PatternListCommaId /*node_id*/)
     -> bool {
   context.param_and_arg_refs_stack().ApplyComma();
-  context.BeginSubpattern();
+  BeginSubpattern(context);
   return true;
 }
 
@@ -57,7 +58,7 @@ auto HandleParseNode(Context& context, Parse::TuplePatternId node_id) -> bool {
   if (context.node_stack().PeekIs(Parse::NodeKind::TuplePatternStart)) {
     // End the subpattern started by a trailing comma, or the opening delimiter
     // of an empty list.
-    context.EndSubpatternAsEmpty();
+    EndSubpatternAsEmpty(context);
   }
   // Note the Start node remains on the stack, where the param list handler can
   // make use of it.

toolchain/check/pattern_match.cpp

@@ -12,6 +12,7 @@
 #include "toolchain/base/kind_switch.h"
 #include "toolchain/check/context.h"
 #include "toolchain/check/convert.h"
+#include "toolchain/check/subpattern.h"
 
 namespace Carbon::Check {
 
@@ -124,6 +125,52 @@ auto MatchContext::DoWork(Context& context) -> SemIR::InstBlockId {
   return block_id;
 }
 
+// Inserts the given region into the current code block. If the region
+// consists of a single block, this will be implemented as a `splice_block`
+// inst. Otherwise, this will end the current block with a branch to the entry
+// block of the region, and add future insts to a new block which is the
+// immediate successor of the region's exit block. As a result, this cannot be
+// called more than once for the same region.
+static auto InsertHere(Context& context, SemIR::ExprRegionId region_id)
+    -> SemIR::InstId {
+  auto region = context.sem_ir().expr_regions().Get(region_id);
+  auto loc_id = context.insts().GetLocId(region.result_id);
+  auto exit_block = context.inst_blocks().Get(region.block_ids.back());
+  if (region.block_ids.size() == 1) {
+    // TODO: Is it possible to avoid leaving an "orphan" block in the IR in the
+    // first two cases?
+    if (exit_block.empty()) {
+      return region.result_id;
+    }
+    if (exit_block.size() == 1) {
+      context.inst_block_stack().AddInstId(exit_block.front());
+      return region.result_id;
+    }
+    return context.AddInst<SemIR::SpliceBlock>(
+        loc_id, {.type_id = context.insts().Get(region.result_id).type_id(),
+                 .block_id = region.block_ids.front(),
+                 .result_id = region.result_id});
+  }
+  if (context.region_stack().empty()) {
+    context.TODO(loc_id,
+                 "Control flow expressions are currently only supported inside "
+                 "functions.");
+    return SemIR::ErrorInst::SingletonInstId;
+  }
+  context.AddInst(SemIR::LocIdAndInst::NoLoc<SemIR::Branch>(
+      {.target_id = region.block_ids.front()}));
+  context.inst_block_stack().Pop();
+  // TODO: this will cumulatively cost O(MN) running time for M blocks
+  // at the Nth level of the stack. Figure out how to do better.
+  context.region_stack().AddToRegion(region.block_ids);
+  auto resume_with_block_id =
+      context.insts().GetAs<SemIR::Branch>(exit_block.back()).target_id;
+  CARBON_CHECK(context.inst_blocks().GetOrEmpty(resume_with_block_id).empty());
+  context.inst_block_stack().Push(resume_with_block_id);
+  context.region_stack().AddToRegion(resume_with_block_id, loc_id);
+  return region.result_id;
+}
+
 auto MatchContext::EmitPatternMatch(Context& context,
                                     MatchContext::WorkItem entry) -> void {
   if (entry.pattern_id == SemIR::ErrorInst::SingletonInstId) {
@@ -149,7 +196,7 @@ auto MatchContext::EmitPatternMatch(Context& context,
           context.bind_name_map().Lookup(entry.pattern_id).value(),
           {.bind_name_id = SemIR::InstId::None,
            .type_expr_region_id = SemIR::ExprRegionId::None});
-      context.InsertHere(type_expr_region_id);
+      InsertHere(context, type_expr_region_id);
       auto value_id = entry.scrutinee_id;
       switch (kind_) {
         case MatchKind::Local: {

toolchain/check/subpattern.cpp

@@ -0,0 +1,43 @@
+// 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
+
+#include "toolchain/check/subpattern.h"
+
+namespace Carbon::Check {
+
+auto BeginSubpattern(Context& context) -> void {
+  context.inst_block_stack().Push();
+  context.region_stack().PushRegion(context.inst_block_stack().PeekOrAdd());
+}
+
+auto EndSubpatternAsExpr(Context& context, SemIR::InstId result_id)
+    -> SemIR::ExprRegionId {
+  if (context.region_stack().PeekRegion().size() > 1) {
+    // End the exit block with a branch to a successor block, whose contents
+    // will be determined later.
+    context.AddInst(SemIR::LocIdAndInst::NoLoc<SemIR::Branch>(
+        {.target_id = context.inst_blocks().AddDefaultValue()}));
+  } else {
+    // This single-block region will be inserted as a SpliceBlock, so we don't
+    // need control flow out of it.
+  }
+  auto block_id = context.inst_block_stack().Pop();
+  CARBON_CHECK(block_id == context.region_stack().PeekRegion().back());
+
+  // TODO: Is it possible to validate that this region is genuinely
+  // single-entry, single-exit?
+  return context.sem_ir().expr_regions().Add(
+      {.block_ids = context.region_stack().PopRegion(),
+       .result_id = result_id});
+}
+
+auto EndSubpatternAsEmpty(Context& context) -> void {
+  auto block_id = context.inst_block_stack().Pop();
+  CARBON_CHECK(block_id == context.region_stack().PeekRegion().back());
+  CARBON_CHECK(context.region_stack().PeekRegion().size() == 1);
+  CARBON_CHECK(context.inst_blocks().Get(block_id).empty());
+  context.region_stack().PopAndDiscardRegion();
+}
+
+}  // namespace Carbon::Check

toolchain/check/subpattern.h

@@ -0,0 +1,34 @@
+// 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_SUBPATTERN_H_
+#define CARBON_TOOLCHAIN_CHECK_SUBPATTERN_H_
+
+#include "toolchain/check/context.h"
+#include "toolchain/sem_ir/ids.h"
+
+namespace Carbon::Check {
+
+// Marks the start of a region of insts in a pattern context that might
+// represent an expression or a pattern. Typically this is called when
+// handling a parse node that can immediately precede a subpattern (such
+// as `let` or a `,` in a pattern list), and the handler for the subpattern
+// node makes the matching `EndSubpatternAs*` call.
+auto BeginSubpattern(Context& context) -> void;
+
+// Ends a region started by BeginSubpattern (in stack order), treating it as
+// an expression with the given result, and returns the ID of the region. The
+// region will not yet have any control-flow edges into or out of it.
+auto EndSubpatternAsExpr(Context& context, SemIR::InstId result_id)
+    -> SemIR::ExprRegionId;
+
+// Ends a region started by BeginSubpattern (in stack order), asserting that
+// it was empty.
+auto EndSubpatternAsEmpty(Context& context) -> void;
+
+// TODO: Add EndSubpatternAsPattern, when needed.
+
+}  // namespace Carbon::Check
+
+#endif  // CARBON_TOOLCHAIN_CHECK_SUBPATTERN_H_