diff --git a/toolchain/check/BUILD b/toolchain/check/BUILD index 5e66adf09518..a4c1684e63d3 100644 --- a/toolchain/check/BUILD +++ b/toolchain/check/BUILD @@ -43,6 +43,7 @@ cc_library( "eval.cpp", "eval_inst.cpp", "facet_type.cpp", + "full_pattern_stack.cpp", "function.cpp", "generic.cpp", "global_init.cpp", diff --git a/toolchain/check/context.h b/toolchain/check/context.h index 8c693dd01dfc..1cb0186d23f6 100644 --- a/toolchain/check/context.h +++ b/toolchain/check/context.h @@ -206,10 +206,6 @@ class Context { return bind_name_map_; } - auto var_storage_map() -> Map& { - return var_storage_map_; - } - // During Choice typechecking, each alternative turns into a name binding on // the Choice type, but this can't be done until the full Choice type is // known. This represents each binding to be done at the end of checking the @@ -531,11 +527,6 @@ class Context { // pattern-match SemIR for it. Map bind_name_map_; - // Map from VarPattern insts to the corresponding VarStorage insts. The - // VarStorage insts are allocated, emitted, and stored in the map after - // processing the enclosing full-pattern. - Map var_storage_map_; - // Each alternative in a Choice gets an entry here, they are stored in // declaration order. The vector is consumed and emptied at the end of the // Choice definition. diff --git a/toolchain/check/full_pattern_stack.cpp b/toolchain/check/full_pattern_stack.cpp new file mode 100644 index 000000000000..af1304c9413e --- /dev/null +++ b/toolchain/check/full_pattern_stack.cpp @@ -0,0 +1,58 @@ +// 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/full_pattern_stack.h" + +#include "toolchain/check/context.h" +#include "toolchain/check/pattern.h" + +namespace Carbon::Check { + +auto FullPatternStack::StartPatternInitializer() -> void { + CARBON_CHECK(kind_stack_.back() == Kind::ClassScopeVarDecl || + kind_stack_.back() == Kind::NameBindingDecl); + for (auto& [name_id, inst_id] : bind_name_stack_.PeekArray()) { + CARBON_CHECK( + inst_id == SemIR::InstId::InitTombstone, + "stashing the lookup result would overwrite an existing stash {0}", + inst_id); + auto& lookup_result = lookup_->Get(name_id); + if (!lookup_result.empty()) { + // Temporarily overwrite the result of name lookup for this binding to be + // `InitTombstone`, so that references to it in the initializer are + // diagnosed as errors. The original result of name lookup is stashed in + // `bind_name_stack_` so we can restore it later. + // + // TODO: find a way to preserve location information, so that we can + // provide good diagnostics for a redeclaration of `name_id` in + // the initializer, if that becomes possible. + std::swap(lookup_result.back().inst_id, inst_id); + } + } +} + +auto FullPatternStack::EndPatternInitializer() -> void { + for (auto& [name_id, inst_id] : bind_name_stack_.PeekArray()) { + auto& lookup_result = lookup_->Get(name_id); + if (!lookup_result.empty()) { + // Restore the original result of name lookup for this binding. + std::swap(lookup_result.back().inst_id, inst_id); + } + CARBON_CHECK(inst_id == SemIR::InstId::InitTombstone, + "name_id resolved to a non-tombstone value {0} during " + "initializer handling", + inst_id); + } +} + +auto FullPatternStack::BuildLocalVarStorage(Context& context, + bool is_returned_var) -> void { + for (auto& var_info : var_pattern_stack_.PeekArray()) { + var_info.storage_id = + GetOrAddVarStorage(context, var_info.pattern_id, is_returned_var); + } + next_var_index_stack_.back() = 0; +} + +} // namespace Carbon::Check diff --git a/toolchain/check/full_pattern_stack.h b/toolchain/check/full_pattern_stack.h index 28f8be1a48ad..855792310579 100644 --- a/toolchain/check/full_pattern_stack.h +++ b/toolchain/check/full_pattern_stack.h @@ -12,6 +12,8 @@ namespace Carbon::Check { +class Context; + // Stack of full-patterns currently being checked (a full-pattern is a pattern // that is not part of an enclosing pattern). It is structured as a stack to // handle situations like a pattern that contains an initializer, or a pattern @@ -76,18 +78,24 @@ class FullPatternStack { auto PushParameterizedDecl() -> void { kind_stack_.push_back(Kind::NotInEitherParamList); bind_name_stack_.PushArray(); + var_pattern_stack_.PushArray(); + next_var_index_stack_.push_back(-1); } // Marks the start of a new full-pattern for a name binding declaration. auto PushNameBindingDecl() -> void { kind_stack_.push_back(Kind::NameBindingDecl); bind_name_stack_.PushArray(); + var_pattern_stack_.PushArray(); + next_var_index_stack_.push_back(-1); } // Marks the start of a new full-pattern for a class `var` declaration. auto PushClassScopeVarDecl() -> void { kind_stack_.push_back(Kind::ClassScopeVarDecl); bind_name_stack_.PushArray(); + var_pattern_stack_.PushArray(); + next_var_index_stack_.push_back(-1); } // Marks the start of the current parameterized entity's implicit parameter @@ -123,37 +131,21 @@ class FullPatternStack { } // Marks the start of the initializer for the current name binding decl. - auto StartPatternInitializer() -> void { - CARBON_CHECK(kind_stack_.back() == Kind::ClassScopeVarDecl || - kind_stack_.back() == Kind::NameBindingDecl); - for (auto& [name_id, inst_id] : bind_name_stack_.PeekArray()) { - CARBON_CHECK(inst_id == SemIR::InstId::InitTombstone); - auto& lookup_result = lookup_->Get(name_id); - if (!lookup_result.empty()) { - // TODO: find a way to preserve location information, so that we can - // provide good diagnostics for a redeclaration of `name_id` in - // the initializer, if that becomes possible. - std::swap(lookup_result.back().inst_id, inst_id); - } - } - } + auto StartPatternInitializer() -> void; // Marks the end of the initializer for the current name-binding decl. - auto EndPatternInitializer() -> void { - for (auto& [name_id, inst_id] : bind_name_stack_.PeekArray()) { - auto& lookup_result = lookup_->Get(name_id); - if (!lookup_result.empty()) { - std::swap(lookup_result.back().inst_id, inst_id); - } - CARBON_CHECK(inst_id == SemIR::InstId::InitTombstone); - } - } + auto EndPatternInitializer() -> void; - // Marks the end of checking for the current full-pattern. This cannot be - // called while processing an initializer for the top pattern. + // Marks the end of checking and pattern matching for the current + // full-pattern. auto PopFullPattern() -> void { kind_stack_.pop_back(); bind_name_stack_.PopArray(); + int index = next_var_index_stack_.pop_back_val(); + CARBON_CHECK(index < 0 || static_cast(index) == + var_pattern_stack_.PeekArray().size(), + "`GetLocalVarStorage` not called for all var patterns"); + var_pattern_stack_.PopArray(); } // Records that `name_id` was introduced by the current full-pattern. @@ -162,6 +154,44 @@ class FullPatternStack { {.name_id = name_id, .inst_id = SemIR::InstId::InitTombstone}); } + // Records a `VarPattern` inst as part of the current full pattern, so that + // its `VarStorage` can be allocated and tracked. This should only be called + // when the current full-pattern is a kind that can have an initializer; + // otherwise the `VarStorage` should be allocated on demand during pattern + // matching. + auto AddLocalVarPattern(SemIR::InstId var_pattern_id) -> void { + CARBON_CHECK(kind_stack_.back() == Kind::ClassScopeVarDecl || + kind_stack_.back() == Kind::NameBindingDecl); + CARBON_CHECK(next_var_index_stack_.back() < 0); + var_pattern_stack_.AppendToTop( + {.pattern_id = var_pattern_id, .storage_id = SemIR::InstId::None}); + } + + // Creates `VarStorage` insts for all `VarPattern` insts recorded by + // `AddLocalVarPattern` for the current full-pattern. This must typically + // be called before handling the initializer (if any) for the current full- + // pattern, in order to preserve the dominance ordering (see the comments + // on `Check::Initialize` for details). + auto BuildLocalVarStorage(Context& context, bool is_returned_var) -> void; + + // Returns the `VarStorage` inst that was allocated for `pattern_id` by + // `BuildLocalVarStorage`. + // + // As an optimization, this assumes (and enforces) that it will be called + // exactly once for each inst passed to `AddLocalVarPattern`, and in the same + // order. + auto GetLocalVarStorage(SemIR::InstId var_pattern_id) -> SemIR::InstId { + CARBON_CHECK(kind_stack_.back() == Kind::ClassScopeVarDecl || + kind_stack_.back() == Kind::NameBindingDecl); + auto& index = next_var_index_stack_.back(); + CARBON_CHECK(index >= 0); + auto var_info = var_pattern_stack_.PeekArray()[index]; + CARBON_CHECK(var_info.pattern_id == var_pattern_id, + "var patterns visited in unexpected order"); + ++index; + return var_info.storage_id; + } + // Runs verification that the processing cleanly finished. auto VerifyOnFinish() const -> void { CARBON_CHECK(kind_stack_.empty(), @@ -172,13 +202,39 @@ class FullPatternStack { private: LexicalLookup* lookup_; + // The stack of pending full-patterns is organized as a struct of arrays, with + // separate stacks for separate properties of a full-pattern. + + // The kinds of the currently pending full patterns. llvm::SmallVector kind_stack_; - struct LookupEntry { + // Locally stashed name-lookup information about a binding. + struct BindingInfo { + // The name of the binding. SemIR::NameId name_id; + // While handling the initializer, name lookup for `name_id` in `lookup_` + // temporarily resolves to `InitTombstone`. During that time, this records + // the inst that it resolved to before the initializer, so that it can + // be restored afterward. This is `InitTombstone` while not handling the + // initializer, or if `name_id` doesn't resolve in `lookup_`. SemIR::InstId inst_id; }; - ArrayStack bind_name_stack_; + + // The name bindings introduced by the currently pending full-patterns. + ArrayStack bind_name_stack_; + + struct VarInfo { + SemIR::InstId pattern_id; + SemIR::InstId storage_id; + }; + + // The `var` patterns introduced by the currently pending full-patterns. + ArrayStack var_pattern_stack_; + + // For each full pattern, the index of the first un-consumed `VarInfo` in + // the corresponding frame of `var_pattern_stack_`, or -1 if the contents + // of that frame are not ready for consumption. + llvm::SmallVector next_var_index_stack_; }; } // namespace Carbon::Check diff --git a/toolchain/check/handle_let_and_var.cpp b/toolchain/check/handle_let_and_var.cpp index f2b6b46c3905..b55ae3a849f9 100644 --- a/toolchain/check/handle_let_and_var.cpp +++ b/toolchain/check/handle_let_and_var.cpp @@ -119,6 +119,7 @@ auto HandleParseNode(Context& context, Parse::VariablePatternId node_id) pattern_id = AddInst( context, node_id, {.type_id = type_id, .subpattern_id = subpattern_id}); + context.full_pattern_stack().AddLocalVarPattern(pattern_id); break; case FullPatternStack::Kind::ClassScopeVarDecl: if (InStaticClassScopeVar(context)) { @@ -126,6 +127,7 @@ auto HandleParseNode(Context& context, Parse::VariablePatternId node_id) pattern_id = AddInst( context, node_id, {.type_id = type_id, .subpattern_id = subpattern_id}); + context.full_pattern_stack().AddLocalVarPattern(pattern_id); } else { // For non-static class fields, a `FieldDecl` was created in // `AddBindingPattern`. Use that as the `pattern_id` so that @@ -167,7 +169,7 @@ static auto EndFullPattern(Context& context) -> void { bool returned = context.decl_introducer_state_stack().innermost().modifier_set.HasAnyOf( KeywordModifierSet::Returned); - AddPatternVarStorage(context, pattern_block_id, returned); + context.full_pattern_stack().BuildLocalVarStorage(context, returned); } static auto StartPatternInitializer(Context& context) -> bool { @@ -315,7 +317,6 @@ auto HandleParseNode(Context& context, Parse::LetDeclId node_id) -> bool { auto decl_info = HandleDecl(context, node_id); - context.full_pattern_stack().PopFullPattern(); context.decl_introducer_state_stack().Pop(); LimitModifiersOnDecl( @@ -337,6 +338,7 @@ auto HandleParseNode(Context& context, Parse::LetDeclId node_id) -> bool { context.emitter().Emit(LocIdForDiagnostics::TokenOnly(node_id), ExpectedInitializerAfterLet); } + context.full_pattern_stack().PopFullPattern(); return true; } diff --git a/toolchain/check/handle_loop_statement.cpp b/toolchain/check/handle_loop_statement.cpp index aa69418772f3..735057b4edbb 100644 --- a/toolchain/check/handle_loop_statement.cpp +++ b/toolchain/check/handle_loop_statement.cpp @@ -130,7 +130,6 @@ auto HandleParseNode(Context& context, Parse::ForInId node_id) -> bool { {.pattern_block_id = pattern_block_id}); context.decl_introducer_state_stack().Pop(); context.full_pattern_stack().StartPatternInitializer(); - context.node_stack().Push(node_id, pattern_block_id); return true; } @@ -147,7 +146,6 @@ static auto CallOptionalAccessor(Context& context, Parse::NodeId node_id, auto HandleParseNode(Context& context, Parse::ForHeaderId node_id) -> bool { auto range_id = context.node_stack().PopExpr(); - auto pattern_block_id = context.node_stack().Pop(); auto pattern_id = context.node_stack().PopPattern(); auto start_node_id = context.node_stack().PopForSoloNodeId(); @@ -212,15 +210,16 @@ auto HandleParseNode(Context& context, Parse::ForHeaderId node_id) -> bool { // The loop pattern's initializer is now complete, and any bindings in it // should be in scope. context.full_pattern_stack().EndPatternInitializer(); - context.full_pattern_stack().PopFullPattern(); // Create storage for var patterns now. - AddPatternVarStorage(context, pattern_block_id, /*is_returned_var=*/false); + context.full_pattern_stack().BuildLocalVarStorage(context, + /*is_returned_var=*/false); // Initialize the pattern from `.Get()`. auto element_value_id = CallOptionalAccessor(context, node_id, element_id, CoreIdentifier::Get); LocalPatternMatch(context, pattern_id, element_value_id); + context.full_pattern_stack().PopFullPattern(); return true; } diff --git a/toolchain/check/node_stack.h b/toolchain/check/node_stack.h index cba5012652f6..75b041738c6b 100644 --- a/toolchain/check/node_stack.h +++ b/toolchain/check/node_stack.h @@ -421,7 +421,6 @@ class NodeStack { case Parse::NodeKind::StructLiteralField: return Id::KindFor(); case Parse::NodeKind::ExplicitParamList: - case Parse::NodeKind::ForIn: case Parse::NodeKind::IfCondition: case Parse::NodeKind::IfExprIf: case Parse::NodeKind::ImplicitParamList: @@ -497,6 +496,7 @@ class NodeStack { case Parse::NodeKind::FileEnd: case Parse::NodeKind::FileStart: case Parse::NodeKind::ForHeader: + case Parse::NodeKind::ForIn: case Parse::NodeKind::Forall: case Parse::NodeKind::FormLiteralKeyword: case Parse::NodeKind::FormLiteralOpenParen: diff --git a/toolchain/check/pattern.cpp b/toolchain/check/pattern.cpp index a71b9f2e454c..a024628cbd92 100644 --- a/toolchain/check/pattern.cpp +++ b/toolchain/check/pattern.cpp @@ -206,11 +206,8 @@ auto AddBindingPattern(Context& context, SemIR::LocId name_loc, return {.pattern_id = binding_pattern_id, .bind_id = bind_id}; } -// Returns a VarStorage inst for the given `var` pattern. If the pattern -// is the body of a returned var, this reuses the return parameter, and -// otherwise it adds a new inst. -static auto GetOrAddVarStorage(Context& context, SemIR::InstId var_pattern_id, - bool is_returned_var) -> SemIR::InstId { +auto GetOrAddVarStorage(Context& context, SemIR::InstId var_pattern_id, + bool is_returned_var) -> SemIR::InstId { if (is_returned_var) { if (auto return_param_id = GetReturnedVarParam(context, GetCurrentFunctionForReturn(context)); @@ -227,21 +224,6 @@ static auto GetOrAddVarStorage(Context& context, SemIR::InstId var_pattern_id, .pattern_id = var_pattern_id}); } -auto AddPatternVarStorage(Context& context, SemIR::InstBlockId pattern_block_id, - bool is_returned_var) -> void { - // We need to emit the VarStorage insts early, because they may be output - // arguments for the initializer. However, we can't emit them when we emit - // the corresponding `AnyVarPattern`s because they're part of the pattern - // match, not part of the pattern. - // TODO: Find a way to do this without walking the whole pattern block. - for (auto inst_id : context.inst_blocks().Get(pattern_block_id)) { - if (context.insts().Is(inst_id)) { - context.var_storage_map().Insert( - inst_id, GetOrAddVarStorage(context, inst_id, is_returned_var)); - } - } -} - auto GetParamPatternKind(Context& context, SemIR::InstId param_inst_id) -> ParamPatternKind { auto param = context.insts().Get( diff --git a/toolchain/check/pattern.h b/toolchain/check/pattern.h index 380bfd2d5799..251e28fce89b 100644 --- a/toolchain/check/pattern.h +++ b/toolchain/check/pattern.h @@ -68,11 +68,11 @@ auto AddBindingForPattern(Context& context, SemIR::LocId name_loc, SemIR::TypeId binding_type_id, SemIR::InstId value_id) -> SemIR::InstId; -// Creates storage for `var` patterns nested within the given pattern at the -// current location in the output SemIR. For a `returned var`, this -// reuses the function's return slot when present. -auto AddPatternVarStorage(Context& context, SemIR::InstBlockId pattern_block_id, - bool is_returned_var) -> void; +// Returns a VarStorage inst for the given `var` pattern. `is_returned_var` +// indicates whether the pattern is the `var` part of a `returned var`; if so, +// this reuses the return parameter, and otherwise it adds a new inst. +auto GetOrAddVarStorage(Context& context, SemIR::InstId var_pattern_id, + bool is_returned_var) -> SemIR::InstId; // Kinds of parameters that can be added by `AddParamPattern`. enum class ParamPatternKind { diff --git a/toolchain/check/pattern_match.cpp b/toolchain/check/pattern_match.cpp index 5f6eb6127899..e214def3ff9b 100644 --- a/toolchain/check/pattern_match.cpp +++ b/toolchain/check/pattern_match.cpp @@ -674,9 +674,8 @@ auto MatchContext::DoVarPreWorkImpl(State state, // In a `var`/`let` declaration, the `VarStorage` inst is created before // we start pattern matching. - auto lookup_result = context_.var_storage_map().Lookup(entry.pattern_id); - CARBON_CHECK(lookup_result); - auto storage_id = lookup_result.value(); + auto storage_id = + context_.full_pattern_stack().GetLocalVarStorage(entry.pattern_id); if (scrutinee_id.has_value()) { auto init_id = InitializeExisting(context_, SemIR::LocId(entry.pattern_id), diff --git a/toolchain/check/testdata/var/var_pattern.carbon b/toolchain/check/testdata/var/var_pattern.carbon index b7c4f3efb39a..1c3d3d0a497b 100644 --- a/toolchain/check/testdata/var/var_pattern.carbon +++ b/toolchain/check/testdata/var/var_pattern.carbon @@ -82,10 +82,14 @@ fn G() { library "[[@TEST_NAME]]"; fn F() { - // CHECK:STDERR: fail_nested.carbon:[[@LINE+4]]:41: error: `var` nested within another `var` [NestedVar] + // CHECK:STDERR: fail_nested.carbon:[[@LINE+8]]:41: error: `var` nested within another `var` [NestedVar] // CHECK:STDERR: let (unused x: (), var (unused y: (), var unused z: ())) = ((), ((), ())); // CHECK:STDERR: ^~~ // CHECK:STDERR: + // CHECK:STDERR: fail_nested.carbon:[[@LINE+4]]:41: error: semantics TODO: `handle invalid parse trees in `check`` [SemanticsTodo] + // CHECK:STDERR: let (unused x: (), var (unused y: (), var unused z: ())) = ((), ((), ())); + // CHECK:STDERR: ^~~~~~~~~~~~~~~~ + // CHECK:STDERR: let (unused x: (), var (unused y: (), var unused z: ())) = ((), ((), ())); } @@ -755,114 +759,7 @@ fn Call() { // CHECK:STDOUT: // CHECK:STDOUT: --- fail_nested.carbon // CHECK:STDOUT: -// CHECK:STDOUT: constants { -// CHECK:STDOUT: %F.type: type = fn_type @F [concrete] -// CHECK:STDOUT: %empty_tuple.type: type = tuple_type () [concrete] -// CHECK:STDOUT: %F: %F.type = struct_value () [concrete] -// CHECK:STDOUT: %empty_tuple: %empty_tuple.type = tuple_value () [concrete] -// CHECK:STDOUT: %pattern_type.cb1: type = pattern_type %empty_tuple.type [concrete] -// CHECK:STDOUT: %x.patt: %pattern_type.cb1 = value_binding_pattern x [concrete] -// CHECK:STDOUT: %y.patt: %pattern_type.cb1 = ref_binding_pattern y [concrete] -// CHECK:STDOUT: %z.patt: %pattern_type.cb1 = ref_binding_pattern z [concrete] -// CHECK:STDOUT: %z.var_patt: %pattern_type.cb1 = var_pattern %z.patt [concrete] -// CHECK:STDOUT: %tuple.type.bcd: type = tuple_type (%empty_tuple.type, %empty_tuple.type) [concrete] -// CHECK:STDOUT: %pattern_type.5b8: type = pattern_type %tuple.type.bcd [concrete] -// CHECK:STDOUT: %.e9e: %pattern_type.5b8 = tuple_pattern (%y.patt, %z.var_patt) [concrete] -// CHECK:STDOUT: %.var_patt: %pattern_type.5b8 = var_pattern %.e9e [concrete] -// CHECK:STDOUT: %tuple.type.a21: type = tuple_type (%empty_tuple.type, %tuple.type.bcd) [concrete] -// CHECK:STDOUT: %pattern_type.c6c: type = pattern_type %tuple.type.a21 [concrete] -// CHECK:STDOUT: %.441: %pattern_type.c6c = tuple_pattern (%x.patt, %.var_patt) [concrete] -// CHECK:STDOUT: %tuple.d8f: %tuple.type.bcd = tuple_value (%empty_tuple, %empty_tuple) [concrete] -// CHECK:STDOUT: %tuple.c9e: %tuple.type.a21 = tuple_value (%empty_tuple, %tuple.d8f) [concrete] -// CHECK:STDOUT: %Destroy.type: type = facet_type <@Destroy> [concrete] -// CHECK:STDOUT: %Destroy.Op.type.1d8f74.1: type = fn_type @Destroy.Op.loc9_22.1 [concrete] -// CHECK:STDOUT: %Destroy.Op.1a2547.1: %Destroy.Op.type.1d8f74.1 = struct_value () [concrete] -// CHECK:STDOUT: %Destroy.Op.type.1d8f74.2: type = fn_type @Destroy.Op.loc9_22.2 [concrete] -// CHECK:STDOUT: %Destroy.Op.1a2547.2: %Destroy.Op.type.1d8f74.2 = struct_value () [concrete] -// CHECK:STDOUT: } -// CHECK:STDOUT: -// CHECK:STDOUT: imports { -// CHECK:STDOUT: %Core: = namespace file.%Core.import, [concrete] { -// CHECK:STDOUT: .Destroy = %Core.Destroy -// CHECK:STDOUT: import Core//prelude -// CHECK:STDOUT: import Core//prelude/... -// CHECK:STDOUT: } -// CHECK:STDOUT: %Core.Destroy: type = import_ref Core//prelude/parts/destroy, Destroy, loaded [concrete = constants.%Destroy.type] -// CHECK:STDOUT: } -// CHECK:STDOUT: -// CHECK:STDOUT: file { -// CHECK:STDOUT: package: = namespace [concrete] { -// CHECK:STDOUT: .Core = imports.%Core -// CHECK:STDOUT: .F = %F.decl -// CHECK:STDOUT: } -// CHECK:STDOUT: %Core.import = import Core -// CHECK:STDOUT: %F.decl: %F.type = fn_decl @F [concrete = constants.%F] {} {} -// CHECK:STDOUT: } -// CHECK:STDOUT: -// CHECK:STDOUT: fn @F() { -// CHECK:STDOUT: !entry: -// CHECK:STDOUT: name_binding_decl { -// CHECK:STDOUT: %x.patt: %pattern_type.cb1 = value_binding_pattern x [concrete = constants.%x.patt] -// CHECK:STDOUT: %y.patt: %pattern_type.cb1 = ref_binding_pattern y [concrete = constants.%y.patt] -// CHECK:STDOUT: %z.patt: %pattern_type.cb1 = ref_binding_pattern z [concrete = constants.%z.patt] -// CHECK:STDOUT: %z.var_patt: %pattern_type.cb1 = var_pattern %z.patt [concrete = constants.%z.var_patt] -// CHECK:STDOUT: %.loc9_57: %pattern_type.5b8 = tuple_pattern (%y.patt, %z.var_patt) [concrete = constants.%.e9e] -// CHECK:STDOUT: %.var_patt: %pattern_type.5b8 = var_pattern %.loc9_57 [concrete = constants.%.var_patt] -// CHECK:STDOUT: %.loc9_58: %pattern_type.c6c = tuple_pattern (%x.patt, %.var_patt) [concrete = constants.%.441] -// CHECK:STDOUT: } -// CHECK:STDOUT: %z.var: ref %empty_tuple.type = var %z.var_patt -// CHECK:STDOUT: %.var: ref %tuple.type.bcd = var %.var_patt -// CHECK:STDOUT: %.loc9_64.1: %empty_tuple.type = tuple_literal () [concrete = constants.%empty_tuple] -// CHECK:STDOUT: %.loc9_69.1: %empty_tuple.type = tuple_literal () [concrete = constants.%empty_tuple] -// CHECK:STDOUT: %.loc9_73.1: %empty_tuple.type = tuple_literal () [concrete = constants.%empty_tuple] -// CHECK:STDOUT: %.loc9_74.1: %tuple.type.bcd = tuple_literal (%.loc9_69.1, %.loc9_73.1) [concrete = constants.%tuple.d8f] -// CHECK:STDOUT: %.loc9_75: %tuple.type.a21 = tuple_literal (%.loc9_64.1, %.loc9_74.1) [concrete = constants.%tuple.c9e] -// CHECK:STDOUT: %empty_tuple: %empty_tuple.type = tuple_value () [concrete = constants.%empty_tuple] -// CHECK:STDOUT: %.loc9_64.2: %empty_tuple.type = converted %.loc9_64.1, %empty_tuple [concrete = constants.%empty_tuple] -// CHECK:STDOUT: %.loc9_19.1: type = splice_block %.loc9_19.3 [concrete = constants.%empty_tuple.type] { -// CHECK:STDOUT: %.loc9_19.2: %empty_tuple.type = tuple_literal () [concrete = constants.%empty_tuple] -// CHECK:STDOUT: %.loc9_19.3: type = converted %.loc9_19.2, constants.%empty_tuple.type [concrete = constants.%empty_tuple.type] -// CHECK:STDOUT: } -// CHECK:STDOUT: %x: %empty_tuple.type = value_binding x, %.loc9_64.2 -// CHECK:STDOUT: %tuple.elem0.loc9_74: ref %empty_tuple.type = tuple_access %.var, element0 -// CHECK:STDOUT: %.loc9_69.2: init %empty_tuple.type to %tuple.elem0.loc9_74 = tuple_init () [concrete = constants.%empty_tuple] -// CHECK:STDOUT: %.loc9_74.2: init %empty_tuple.type = converted %.loc9_69.1, %.loc9_69.2 [concrete = constants.%empty_tuple] -// CHECK:STDOUT: %.loc9_74.3: init %empty_tuple.type to %tuple.elem0.loc9_74 = in_place_init %.loc9_74.2 [concrete = constants.%empty_tuple] -// CHECK:STDOUT: %tuple.elem1.loc9_74: ref %empty_tuple.type = tuple_access %.var, element1 -// CHECK:STDOUT: %.loc9_73.2: init %empty_tuple.type to %tuple.elem1.loc9_74 = tuple_init () [concrete = constants.%empty_tuple] -// CHECK:STDOUT: %.loc9_74.4: init %empty_tuple.type = converted %.loc9_73.1, %.loc9_73.2 [concrete = constants.%empty_tuple] -// CHECK:STDOUT: %.loc9_74.5: init %empty_tuple.type to %tuple.elem1.loc9_74 = in_place_init %.loc9_74.4 [concrete = constants.%empty_tuple] -// CHECK:STDOUT: %.loc9_74.6: init %tuple.type.bcd to %.var = tuple_init (%.loc9_74.3, %.loc9_74.5) [concrete = constants.%tuple.d8f] -// CHECK:STDOUT: %.loc9_22.1: init %tuple.type.bcd = converted %.loc9_74.1, %.loc9_74.6 [concrete = constants.%tuple.d8f] -// CHECK:STDOUT: assign %.var, %.loc9_22.1 -// CHECK:STDOUT: %tuple.elem0.loc9_22: ref %empty_tuple.type = tuple_access %.var, element0 -// CHECK:STDOUT: %tuple.elem1.loc9_22: ref %empty_tuple.type = tuple_access %.var, element1 -// CHECK:STDOUT: %.loc9_38.1: type = splice_block %.loc9_38.3 [concrete = constants.%empty_tuple.type] { -// CHECK:STDOUT: %.loc9_38.2: %empty_tuple.type = tuple_literal () [concrete = constants.%empty_tuple] -// CHECK:STDOUT: %.loc9_38.3: type = converted %.loc9_38.2, constants.%empty_tuple.type [concrete = constants.%empty_tuple.type] -// CHECK:STDOUT: } -// CHECK:STDOUT: %y: ref %empty_tuple.type = ref_binding y, %tuple.elem0.loc9_22 -// CHECK:STDOUT: %.loc9_22.2: init %empty_tuple.type = tuple_init () [concrete = constants.%empty_tuple] -// CHECK:STDOUT: %.loc9_41: init %empty_tuple.type = converted %tuple.elem1.loc9_22, %.loc9_22.2 [concrete = constants.%empty_tuple] -// CHECK:STDOUT: assign %z.var, %.loc9_41 -// CHECK:STDOUT: %.loc9_56.1: type = splice_block %.loc9_56.3 [concrete = constants.%empty_tuple.type] { -// CHECK:STDOUT: %.loc9_56.2: %empty_tuple.type = tuple_literal () [concrete = constants.%empty_tuple] -// CHECK:STDOUT: %.loc9_56.3: type = converted %.loc9_56.2, constants.%empty_tuple.type [concrete = constants.%empty_tuple.type] -// CHECK:STDOUT: } -// CHECK:STDOUT: %z: ref %empty_tuple.type = ref_binding z, %z.var -// CHECK:STDOUT: %Destroy.Op.bound.loc9_22: = bound_method %.var, constants.%Destroy.Op.1a2547.2 -// CHECK:STDOUT: %Destroy.Op.call.loc9_22: init %empty_tuple.type = call %Destroy.Op.bound.loc9_22(%.var) -// CHECK:STDOUT: %Destroy.Op.bound.loc9_41: = bound_method %z.var, constants.%Destroy.Op.1a2547.1 -// CHECK:STDOUT: %Destroy.Op.call.loc9_41: init %empty_tuple.type = call %Destroy.Op.bound.loc9_41(%z.var) -// CHECK:STDOUT: return -// CHECK:STDOUT: } -// CHECK:STDOUT: -// CHECK:STDOUT: fn @Destroy.Op.loc9_22.1(%self.param: ref %empty_tuple.type) = "no_op"; -// CHECK:STDOUT: -// CHECK:STDOUT: fn @Destroy.Op.loc9_22.2(%self.param: ref %tuple.type.bcd) { -// CHECK:STDOUT: !entry: -// CHECK:STDOUT: return -// CHECK:STDOUT: } +// CHECK:STDOUT: fn @F(); // CHECK:STDOUT: // CHECK:STDOUT: --- fail_compile_time.carbon // CHECK:STDOUT: diff --git a/toolchain/docs/check/pattern_matching.md b/toolchain/docs/check/pattern_matching.md index 9d6456c63080..bb790063fee8 100644 --- a/toolchain/docs/check/pattern_matching.md +++ b/toolchain/docs/check/pattern_matching.md @@ -112,11 +112,13 @@ instruction IDs: `Context::bind_name_map` stores these `ValueBinding`s, keyed by the corresponding `ValueBindingPattern` instruction. - A `var` pattern allocates storage during matching, which is represented by a - `VarStorage` instruction. This instruction must be allocated during the - pattern step, so that it can be used as the output parameter of scrutinee - expression evaluation during the scrutinee step. `Context::var_storage_map` - stores these `VarStorage` instructions, keyed by the corresponding - `VarPattern` instruction. + `VarStorage` instruction. For local and class-scope `var` patterns, this + instruction must be allocated at the end of the pattern step, so that it can + be used as the output parameter of scrutinee expression evaluation during + the scrutinee step, but doesn't get added to the instruction block that's + meant to capture sub-expressions (see below). `FullPatternStack` is + responsible for the mapping from `VarPattern` insts to the corresponding + `VarStorage` insts. As noted earlier, the pattern step can also emit non-pattern instructions to evaluate expressions that are embedded in the pattern, such as the type diff --git a/toolchain/parse/handle_var.cpp b/toolchain/parse/handle_var.cpp index 71aefbf9035c..fa0541cc4934 100644 --- a/toolchain/parse/handle_var.cpp +++ b/toolchain/parse/handle_var.cpp @@ -93,7 +93,8 @@ auto HandleVariablePattern(Context& context) -> void { context.emitter().Emit(*context.position(), NestedVar); state.has_error = true; } - context.PushState(StateKind::FinishVariablePattern); + state.kind = StateKind::FinishVariablePattern; + context.PushState(state); context.ConsumeChecked(Lex::TokenKind::Var); context.PushStateForPattern(StateKind::Pattern, /*in_var_pattern=*/true, diff --git a/toolchain/parse/testdata/var/var_pattern.carbon b/toolchain/parse/testdata/var/var_pattern.carbon index b6b5649d0f7c..b55618845197 100644 --- a/toolchain/parse/testdata/var/var_pattern.carbon +++ b/toolchain/parse/testdata/var/var_pattern.carbon @@ -110,8 +110,8 @@ let (x: (), var (y: (), var z: ())) = ((), ((), ())); // CHECK:STDOUT: {kind: 'TupleLiteralStart', text: '('}, // CHECK:STDOUT: {kind: 'TupleLiteral', text: ')', subtree_size: 2}, // CHECK:STDOUT: {kind: 'VarBindingPattern', text: ':', subtree_size: 4}, -// CHECK:STDOUT: {kind: 'VariablePattern', text: 'var', subtree_size: 5}, -// CHECK:STDOUT: {kind: 'TuplePattern', text: ')', subtree_size: 12}, +// CHECK:STDOUT: {kind: 'VariablePattern', text: 'var', has_error: yes, subtree_size: 5}, +// CHECK:STDOUT: {kind: 'TuplePattern', text: ')', has_error: yes, subtree_size: 12}, // CHECK:STDOUT: {kind: 'VariablePattern', text: 'var', subtree_size: 13}, // CHECK:STDOUT: {kind: 'TuplePattern', text: ')', subtree_size: 20}, // CHECK:STDOUT: {kind: 'LetInitializer', text: '='},