diff --git a/toolchain/check/cpp/import.cpp b/toolchain/check/cpp/import.cpp index 14e25378865e..35e2163a2cb4 100644 --- a/toolchain/check/cpp/import.cpp +++ b/toolchain/check/cpp/import.cpp @@ -1473,7 +1473,7 @@ static auto MakeSelfParamPatternBlockId( const auto* method_decl = cast(&clang_decl); // Build a `self` parameter from the object parameter. - BeginSubpattern(context); + BeginExprRegionForPattern(context); clang::QualType param_type = method_decl->getFunctionObjectParameterReferenceType(); @@ -1483,9 +1483,9 @@ static auto MakeSelfParamPatternBlockId( auto param_info = MapParameterType(context, loc_id, param_type, passing_mode); auto [type_inst_id, type_id] = param_info.type; SemIR::ExprRegionId type_expr_region_id = - ConsumeSubpatternExpr(context, type_inst_id); + ConsumeExprRegionForPattern(context, type_inst_id); - EndEmptySubpattern(context); + EndEmptyExprRegionForPattern(context); if (!type_id.has_value()) { context.TODO(loc_id, @@ -1538,8 +1538,8 @@ static auto MakeParamPatternsBlockId(Context& context, SemIR::LocId loc_id, ClangGetUnqualifiedTypePreserveNonNull(context, orig_param_type); // Mark the start of a region of insts, needed for the type expression - // created later with the call of `ConsumeSubpatternExpr()`. - BeginSubpattern(context); + // created later with the call of `ConsumeExprRegionForPattern()`. + BeginExprRegionForPattern(context); auto param_info = MapParameterType(context, loc_id, param_type, signature.GetPassingMode(i)); auto [type_inst_id, type_id] = param_info.type; @@ -1547,8 +1547,8 @@ static auto MakeParamPatternsBlockId(Context& context, SemIR::LocId loc_id, // region that allows control flow in the type expression e.g. fn F(x: if C // then i32 else i64). SemIR::ExprRegionId type_expr_region_id = - ConsumeSubpatternExpr(context, type_inst_id); - EndEmptySubpattern(context); + ConsumeExprRegionForPattern(context, type_inst_id); + EndEmptyExprRegionForPattern(context); if (!type_id.has_value()) { context.TODO(loc_id, llvm::formatv("Unsupported: parameter type: {0}", diff --git a/toolchain/check/function.cpp b/toolchain/check/function.cpp index 414171732b64..a5fbe13905d3 100644 --- a/toolchain/check/function.cpp +++ b/toolchain/check/function.cpp @@ -117,29 +117,23 @@ static auto MakeFunctionSignature(Context& context, SemIR::LocId loc_id, if (!args.self_type_id.has_value() && args.param_type_ids.empty()) { insts.param_patterns_id = SemIR::InstBlockId::Empty; } else { - context.inst_block_stack().Push(); + llvm::SmallVector param_patterns; if (args.self_type_id.has_value()) { - BeginSubpattern(context); - auto self_type_region_id = ConsumeSubpatternExpr( + auto self_type_region_id = MakeEmptyRegion( context, context.types().GetTypeInstId(args.self_type_id)); - EndEmptySubpattern(context); - insts.self_param_id = AddParamPattern( context, loc_id, SemIR::NameId::SelfValue, self_type_region_id, args.self_type_id, args.self_kind); - context.inst_block_stack().AddInstId(insts.self_param_id); + param_patterns.push_back(insts.self_param_id); } for (auto param_type_id : args.param_type_ids) { - BeginSubpattern(context); - auto param_type_region_id = ConsumeSubpatternExpr( + auto param_type_region_id = MakeEmptyRegion( context, context.types().GetTypeInstId(param_type_id)); - EndEmptySubpattern(context); - - context.inst_block_stack().AddInstId(AddParamPattern( + param_patterns.push_back(AddParamPattern( context, loc_id, SemIR::NameId::Underscore, param_type_region_id, param_type_id, args.param_kind)); } - insts.param_patterns_id = context.inst_block_stack().Pop(); + insts.param_patterns_id = context.inst_blocks().Add(param_patterns); } context.full_pattern_stack().EndExplicitParamList(); diff --git a/toolchain/check/handle_binding_pattern.cpp b/toolchain/check/handle_binding_pattern.cpp index b8ce5e41664f..79bceb2488eb 100644 --- a/toolchain/check/handle_binding_pattern.cpp +++ b/toolchain/check/handle_binding_pattern.cpp @@ -167,7 +167,7 @@ static auto HandleAnyBindingPattern( self_type_inst_id); SemIR::ExprRegionId type_expr_region_id = - ConsumeSubpatternExpr(context, type_expr.inst_id); + ConsumeExprRegionForPattern(context, type_expr.inst_id); // The name in a generic binding may be wrapped in `template`. bool is_generic = node_kind == Parse::NodeKind::CompileTimeBindingPattern; @@ -502,7 +502,7 @@ auto HandleParseNode(Context& context, auto [cast_type_inst_id, cast_type_id] = ExprAsType(context, type_node, parsed_type_id); - auto region_id = ConsumeSubpatternExpr(context, cast_type_inst_id); + auto region_id = ConsumeExprRegionForPattern(context, cast_type_inst_id); // TODO: Should we be tracking this somewhere? (void)region_id; diff --git a/toolchain/check/handle_let_and_var.cpp b/toolchain/check/handle_let_and_var.cpp index b55ae3a849f9..91d84fbdf4eb 100644 --- a/toolchain/check/handle_let_and_var.cpp +++ b/toolchain/check/handle_let_and_var.cpp @@ -74,7 +74,7 @@ static auto HandleIntroducer(Context& context, Parse::NodeId node_id) -> bool { } else { context.full_pattern_stack().PushNameBindingDecl(); } - BeginSubpattern(context); + BeginExprRegionForPattern(context); return true; } @@ -97,11 +97,13 @@ auto HandleParseNode(Context& context, Parse::VariableIntroducerId node_id) auto HandleParseNode(Context& context, Parse::VariablePatternId node_id) -> bool { + EndExprRegionForPattern(context, context.node_stack()); auto subpattern_id = context.node_stack().PopPattern(); auto type_id = context.insts().Get(subpattern_id).type_id(); if (subpattern_id == SemIR::ErrorInst::InstId) { context.node_stack().Push(node_id, SemIR::ErrorInst::InstId); + BeginExprRegionForPattern(context); return true; } @@ -140,13 +142,14 @@ auto HandleParseNode(Context& context, Parse::VariablePatternId node_id) } context.node_stack().Push(node_id, pattern_id); + BeginExprRegionForPattern(context); return true; } // Handle the end of the full-pattern of a let/var declaration (before the // start of the initializer, if any). static auto EndFullPattern(Context& context) -> void { - EndSubpattern(context, context.node_stack()); + EndExprRegionForPattern(context, context.node_stack()); if (context.name_scopes().InstIs( context.scope_stack().PeekNameScopeId())) { // Don't emit NameBindingDecl for an associated constant, because it will diff --git a/toolchain/check/handle_loop_statement.cpp b/toolchain/check/handle_loop_statement.cpp index 735057b4edbb..d3b788901b3c 100644 --- a/toolchain/check/handle_loop_statement.cpp +++ b/toolchain/check/handle_loop_statement.cpp @@ -117,14 +117,14 @@ auto HandleParseNode(Context& context, Parse::ForHeaderStartId node_id) context.decl_introducer_state_stack().Push(); context.pattern_block_stack().Push(); context.full_pattern_stack().PushNameBindingDecl(); - BeginSubpattern(context); + BeginExprRegionForPattern(context); context.node_stack().Push(node_id); return true; } auto HandleParseNode(Context& context, Parse::ForInId node_id) -> bool { - EndSubpattern(context, context.node_stack()); + EndExprRegionForPattern(context, context.node_stack()); auto pattern_block_id = context.pattern_block_stack().Pop(); AddInst(context, node_id, {.pattern_block_id = pattern_block_id}); diff --git a/toolchain/check/handle_pattern_list.cpp b/toolchain/check/handle_pattern_list.cpp index a88b427f7ccd..d7a7991b30b2 100644 --- a/toolchain/check/handle_pattern_list.cpp +++ b/toolchain/check/handle_pattern_list.cpp @@ -17,7 +17,7 @@ static auto HandlePatternListStart(Context& context, Parse::NodeId node_id) -> bool { context.node_stack().Push(node_id); context.param_and_arg_refs_stack().Push(); - BeginSubpattern(context); + BeginExprRegionForPattern(context); return true; } @@ -29,6 +29,9 @@ auto HandleParseNode(Context& context, Parse::ImplicitParamListStartId node_id) auto HandleParseNode(Context& context, Parse::TuplePatternStartId node_id) -> bool { + // End the pending `ExprRegion`, so that we can start a new one in + // `HandlePatternListStart`. + EndEmptyExprRegionForPattern(context); return HandlePatternListStart(context, node_id); } @@ -43,11 +46,12 @@ auto HandleParseNode(Context& context, Parse::ExplicitParamListStartId node_id) static auto HandleParamListEnd(Context& context, Parse::NodeId node_id, Parse::NodeKind start_kind) -> bool { if (context.node_stack().PeekIs(start_kind)) { - // End the subpattern started by a trailing comma, or the opening delimiter - // of an empty list. - EndEmptySubpattern(context); + // End the pending region started by a trailing comma, or the opening + // delimiter of an empty list. + EndEmptyExprRegionForPattern(context); } else { - EndSubpattern(context, context.node_stack()); + // End the pending region for the last pattern in the list. + EndExprRegionForPattern(context, context.node_stack()); } // Note the Start node remains on the stack, where the param list handler can // make use of it. @@ -71,22 +75,26 @@ auto HandleParseNode(Context& context, Parse::ExplicitParamListId node_id) } auto HandleParseNode(Context& context, Parse::ParenPatternId node_id) -> bool { - EndSubpattern(context, context.node_stack()); + EndExprRegionForPattern(context, context.node_stack()); auto pattern_id = context.node_stack().PopPattern(); context.param_and_arg_refs_stack().PopAndDiscard(); context.node_stack() .PopAndDiscardSoloNodeId(); context.node_stack().Push(node_id, pattern_id); + // Start a new pending `ExprRegion`, to maintain the invariant that one is + // pending at the end of handling for a pattern. + BeginExprRegionForPattern(context); return true; } 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. - EndEmptySubpattern(context); + // End the pending region started by a trailing comma, or the opening + // delimiter of an empty list. + EndEmptyExprRegionForPattern(context); } else { - EndSubpattern(context, context.node_stack()); + // End the pending region for the last pattern in the list. + EndExprRegionForPattern(context, context.node_stack()); } auto refs_id = context.param_and_arg_refs_stack().EndAndPop( Parse::NodeKind::TuplePatternStart); @@ -115,14 +123,17 @@ auto HandleParseNode(Context& context, Parse::TuplePatternId node_id) -> bool { node_id, AddInst( context, node_id, {.type_id = type_id, .elements_id = refs_id})); + // Start a new pending `ExprRegion`, to maintain the invariant that one is + // pending at the end of handling for a pattern. + BeginExprRegionForPattern(context); return true; } auto HandleParseNode(Context& context, Parse::PatternListCommaId /*node_id*/) -> bool { - EndSubpattern(context, context.node_stack()); + EndExprRegionForPattern(context, context.node_stack()); context.param_and_arg_refs_stack().ApplyComma(); - BeginSubpattern(context); + BeginExprRegionForPattern(context); return true; } diff --git a/toolchain/check/pattern.cpp b/toolchain/check/pattern.cpp index eda3103143db..ae638a3b249a 100644 --- a/toolchain/check/pattern.cpp +++ b/toolchain/check/pattern.cpp @@ -17,14 +17,14 @@ namespace Carbon::Check { -auto BeginSubpattern(Context& context) -> void { +auto BeginExprRegionForPattern(Context& context) -> void { context.inst_block_stack().Push(); // TODO: This allocates an InstBlockId even in the case where the pattern has // no associated expression. Find a way to avoid this. context.region_stack().PushRegion(context.inst_block_stack().PeekOrAdd()); } -static auto PopSubpatternExpr(Context& context, SemIR::InstId result_id) +static auto PopExprRegion(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 @@ -46,16 +46,16 @@ static auto PopSubpatternExpr(Context& context, SemIR::InstId result_id) .result_id = result_id}); } -auto ConsumeSubpatternExpr(Context& context, SemIR::InstId result_id) +auto ConsumeExprRegionForPattern(Context& context, SemIR::InstId result_id) -> SemIR::ExprRegionId { - auto region_id = PopSubpatternExpr(context, result_id); + auto region_id = PopExprRegion(context, result_id); // Push an empty, unreachable region so that we can later detect the region // has been consumed. context.region_stack().PushUnreachableRegion(); return region_id; } -auto EndEmptySubpattern(Context& context) -> void { +auto EndEmptyExprRegionForPattern(Context& context) -> void { if (!context.region_stack().PeekRegion().empty()) { CARBON_CHECK(context.inst_block_stack().PeekCurrentBlockContents().empty()); auto block_id = context.inst_block_stack().Pop(); @@ -65,13 +65,13 @@ auto EndEmptySubpattern(Context& context) -> void { context.region_stack().PopAndDiscardRegion(); } -auto EndSubpattern(Context& context, NodeStack& node_stack) -> void { +auto EndExprRegionForPattern(Context& context, NodeStack& node_stack) -> void { auto [node_id, maybe_expr_id] = node_stack.PopWithNodeIdIf(); if (maybe_expr_id) { // We formed an expression, not a pattern, so convert it to an expression // pattern now. - auto expr_region_id = PopSubpatternExpr(context, *maybe_expr_id); + auto expr_region_id = PopExprRegion(context, *maybe_expr_id); auto pattern_type_id = GetPatternType(context, context.insts().Get(*maybe_expr_id).type_id()); node_stack.Push(node_id, AddInst( @@ -81,10 +81,16 @@ auto EndSubpattern(Context& context, NodeStack& node_stack) -> void { } else { // The expression region should have been consumed when forming the pattern // instruction, so should now effectively be empty. - EndEmptySubpattern(context); + EndEmptyExprRegionForPattern(context); } } +auto MakeEmptyRegion(Context& context, SemIR::InstId result_id) + -> SemIR::ExprRegionId { + return context.sem_ir().expr_regions().Add( + {.block_ids = {SemIR::InstBlockId::Empty}, .result_id = result_id}); +} + auto AddBindingEntityName(Context& context, SemIR::NameId name_id, SemIR::InstId form_id, bool is_unused, BindingPhase phase) -> SemIR::EntityNameId { diff --git a/toolchain/check/pattern.h b/toolchain/check/pattern.h index 251e28fce89b..bd5384a35e30 100644 --- a/toolchain/check/pattern.h +++ b/toolchain/check/pattern.h @@ -10,32 +10,43 @@ namespace Carbon::Check { -// Marks the start of a region of insts in a pattern context that might contain -// an expression. Typically this is called when handling a parse node that can -// immediately precede a subpattern (such as `let` or a `,` in a pattern list). -// `End[Empty]Subpattern` should be called later by the consumer of the -// subpattern. -auto BeginSubpattern(Context& context) -> void; +// The following functions are used to mark the start and end of a time interval +// during pattern handling, in which we may build an `ExprRegion` to represent +// an expression. During one of these intervals, we say that an `ExprRegion` is +// _pending_. Any insts added to `inst_block_stack` while an `ExprRegion` is +// pending are treated as part of the expression. These intervals do not nest: +// we can't start an interval if an `ExprRegion` is already pending. +// +// To ensure that each start has a matching end, without nesting, callers should +// maintain the invariant that an `ExprRegion` is pending before handling the +// start of a pattern, and after handling the end of a pattern. -// Consumes the expression in a region started by the most recent -// BeginSubpattern, and returns the ID of the region. The region will not yet -// have any control-flow edges into or out of it. -auto ConsumeSubpatternExpr(Context& context, SemIR::InstId result_id) +// Marks the start of a pending `ExprRegion` (see above). +auto BeginExprRegionForPattern(Context& context) -> void; + +// Finishes building the pending `ExprRegion`, and returns its ID. It will not +// yet have any control-flow edges into or out of it. An empty `ExprRegion` will +// still be pending after the call, so `End[Empty]ExprRegionForPattern` must be +// called separately after this. +auto ConsumeExprRegionForPattern(Context& context, SemIR::InstId result_id) -> SemIR::ExprRegionId; -// Ends a region started by BeginSubpattern (in stack order), asserting that -// it either had no expression content or the expression has been consumed. -auto EndEmptySubpattern(Context& context) -> void; +// Ends the pending `ExprRegion`, and asserts that it is empty. +auto EndEmptyExprRegionForPattern(Context& context) -> void; -// Ends a region started by BeginSubpattern (in stack order). If the top of the -// node stack is an expression, the subpattern region is consumed and converted -// to an expression pattern, which replaces the expression on the node stack. -// Otherwise, the top of the node stack should be a pattern, in which case this -// asserts that the subpattern region is either empty or has been consumed. +// Ends the pending `ExprRegion`. If the top of the node stack is an expression, +// the `ExprRegion` is consumed and converted to an expression pattern, which +// replaces the expression on the node stack. Otherwise, the top of the node +// stack should be a pattern, in which case this asserts that the pending region +// is empty, and discards it. // // The node stack is passed explicitly as a reminder that this function affects -// the node stack, unlike the other *Subpattern functions. -auto EndSubpattern(Context& context, NodeStack& node_stack) -> void; +// the node stack, unlike the other `*ExprRegionForPattern` functions. +auto EndExprRegionForPattern(Context& context, NodeStack& node_stack) -> void; + +// Builds and returns an empty `ExprRegion`. +auto MakeEmptyRegion(Context& context, SemIR::InstId result_id) + -> SemIR::ExprRegionId; // Information about a created binding pattern. struct BindingPatternInfo { diff --git a/toolchain/check/pattern_match.cpp b/toolchain/check/pattern_match.cpp index e214def3ff9b..6203e855b511 100644 --- a/toolchain/check/pattern_match.cpp +++ b/toolchain/check/pattern_match.cpp @@ -531,7 +531,9 @@ auto MatchContext::DoPreWork(State state, SemIR::AnyParamPattern param_pattern, .work = PreWork{.scrutinee_id = param_id}, .allow_unmarked_ref = entry.allow_unmarked_ref}); } else { - results_stack_.AppendToTop(param_id); + if (need_subpattern_results()) { + results_stack_.AppendToTop(param_id); + } } callee_state->PushCallParamPattern(context_, loc_id, entry.pattern_id, diff --git a/toolchain/docs/check/pattern_matching.md b/toolchain/docs/check/pattern_matching.md index bb790063fee8..660751e13a3f 100644 --- a/toolchain/docs/check/pattern_matching.md +++ b/toolchain/docs/check/pattern_matching.md @@ -126,11 +126,12 @@ expressions of binding patterns, and expressions that are used as patterns themselves (although those have not been implemented yet). The parse tree doesn't mark these situations in advance: any given subpattern might turn out to be one that emits non-pattern instructions. To handle these situations, we -speculatively push an instruction block onto the (non-pattern) stack whenever we -are about to begin handling a subpattern, and then pop it at the end of the -subpattern, with different treatment depending on whether the subpattern turned -out to involve a subexpression. This is handled by `BeginSubpattern`, -`ConsumeSubpatternExpr`, `EndSubpattern`, and `EndEmptySubpattern`. +speculatively prepare to build an `ExprRegion` whenever we are about to begin +handling a pattern that might be a binding or expression pattern, and then +either consume or discard the region once we've passed the point where the +expression (if any) would appear. This is handled by `BeginExprRegionForPattern`, +`ConsumeExprRegionForPattern`, `EndExprRegionForPattern`, and +`EndEmptyExprRegionForPattern`. One further complication here is that the type expression can contain control flow (such as an `if` expression). Consequently, we can't represent the type