diff --git a/toolchain/check/full_pattern_stack.cpp b/toolchain/check/full_pattern_stack.cpp index af1304c9413e..cfb7fbaafb6f 100644 --- a/toolchain/check/full_pattern_stack.cpp +++ b/toolchain/check/full_pattern_stack.cpp @@ -30,6 +30,7 @@ auto FullPatternStack::StartPatternInitializer() -> void { std::swap(lookup_result.back().inst_id, inst_id); } } + next_var_index_stack_.back() = 0; } auto FullPatternStack::EndPatternInitializer() -> void { @@ -46,13 +47,4 @@ auto FullPatternStack::EndPatternInitializer() -> void { } } -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 855792310579..677d26dc3443 100644 --- a/toolchain/check/full_pattern_stack.h +++ b/toolchain/check/full_pattern_stack.h @@ -154,32 +154,33 @@ 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 { + // Information about a `var` pattern. + struct VarInfo { + // The `VarPattern` inst + SemIR::InstId pattern_id; + // The corresponding `VarStorage` inst + SemIR::InstId storage_id; + }; + + // Records a `VarPattern` and its associated `VarStorage` as part of the + // current full pattern, so that the `VarStorage` can be found during pattern + // matching. This should only be called when the current full-pattern is a + // kind that can have an initializer, because in that context the `VarStorage` + // needs to be emitted early; otherwise the `VarStorage` should be emitted on + // demand during pattern matching. + auto AddLocalVarPattern(VarInfo var_info) -> 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}); + var_pattern_stack_.AppendToTop(var_info); } - // 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 + // Returns the `VarStorage` inst that was emitted 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. + // exactly once for each inst pair 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); @@ -223,11 +224,6 @@ class FullPatternStack { // 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_; diff --git a/toolchain/check/handle_let_and_var.cpp b/toolchain/check/handle_let_and_var.cpp index eb3c6b7d1062..9770afe9b0c5 100644 --- a/toolchain/check/handle_let_and_var.cpp +++ b/toolchain/check/handle_let_and_var.cpp @@ -107,6 +107,18 @@ auto HandleParseNode(Context& context, Parse::VariablePatternId node_id) return true; } + auto add_local_var = [&]() -> SemIR::InstId { + auto pattern_id = AddInst( + context, node_id, {.type_id = type_id, .subpattern_id = subpattern_id}); + bool returned = + context.decl_introducer_state_stack().innermost().modifier_set.HasAnyOf( + KeywordModifierSet::Returned); + auto storage_id = GetOrAddVarStorage(context, pattern_id, returned); + context.full_pattern_stack().AddLocalVarPattern( + {.pattern_id = pattern_id, .storage_id = storage_id}); + return pattern_id; + }; + auto pattern_id = SemIR::InstId::None; // In a parameter list, a `var` pattern is always a single `Call` parameter, // even if it contains multiple binding patterns. @@ -118,18 +130,12 @@ auto HandleParseNode(Context& context, Parse::VariablePatternId node_id) {.type_id = type_id, .subpattern_id = subpattern_id}); break; case FullPatternStack::Kind::NameBindingDecl: - pattern_id = AddInst( - context, node_id, - {.type_id = type_id, .subpattern_id = subpattern_id}); - context.full_pattern_stack().AddLocalVarPattern(pattern_id); + pattern_id = add_local_var(); break; case FullPatternStack::Kind::ClassScopeVarDecl: if (InStaticClassScopeVar(context)) { // Handle static class fields the same as NameBindingDecls. - pattern_id = AddInst( - context, node_id, - {.type_id = type_id, .subpattern_id = subpattern_id}); - context.full_pattern_stack().AddLocalVarPattern(pattern_id); + pattern_id = add_local_var(); } else { // For non-static class fields, a `FieldDecl` was created in // `AddBindingPattern`. Use that as the `pattern_id` so that @@ -163,12 +169,6 @@ static auto EndFullPattern(Context& context) -> void { if (InNonStaticFieldDecl(context)) { return; } - - // Emit storage for any `var`s in the pattern now. - bool returned = - context.decl_introducer_state_stack().innermost().modifier_set.HasAnyOf( - KeywordModifierSet::Returned); - context.full_pattern_stack().BuildLocalVarStorage(context, returned); } static auto StartPatternInitializer(Context& context) -> bool { diff --git a/toolchain/check/handle_loop_statement.cpp b/toolchain/check/handle_loop_statement.cpp index d3b788901b3c..4e679cd0c1e8 100644 --- a/toolchain/check/handle_loop_statement.cpp +++ b/toolchain/check/handle_loop_statement.cpp @@ -211,10 +211,6 @@ auto HandleParseNode(Context& context, Parse::ForHeaderId node_id) -> bool { // should be in scope. context.full_pattern_stack().EndPatternInitializer(); - // Create storage for var patterns now. - 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); diff --git a/toolchain/check/testdata/for/pattern.carbon b/toolchain/check/testdata/for/pattern.carbon index c071838bd8d1..a12d130ca07c 100644 --- a/toolchain/check/testdata/for/pattern.carbon +++ b/toolchain/check/testdata/for/pattern.carbon @@ -362,11 +362,11 @@ fn Run() { // CHECK:STDOUT: %struct_type.has_value.value.9e7: type = struct_type {.has_value: bool, .value: %C} [concrete] // CHECK:STDOUT: %Optional.HasValue.specific_fn: = specific_function %Optional.HasValue.a88, @Optional.HasValue(%Copy.facet) [concrete] // CHECK:STDOUT: %Optional.Get.specific_fn: = specific_function %Optional.Get.d5d, @Optional.Get(%Copy.facet) [concrete] -// CHECK:STDOUT: %Destroy.Op.type.1d8f74.1: type = fn_type @Destroy.Op.loc10_8.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.loc10_8.2 [concrete] +// CHECK:STDOUT: %Destroy.Op.type.1d8f74.2: type = fn_type @Destroy.Op.loc10_40.2 [concrete] // CHECK:STDOUT: %Destroy.Op.1a2547.2: %Destroy.Op.type.1d8f74.2 = struct_value () [concrete] -// CHECK:STDOUT: %Destroy.Op.type.1d8f74.5: type = fn_type @Destroy.Op.loc10_40.3 [concrete] +// CHECK:STDOUT: %Destroy.Op.type.1d8f74.3: type = fn_type @Destroy.Op.loc10_40.3 [concrete] +// CHECK:STDOUT: %Destroy.Op.1a2547.3: %Destroy.Op.type.1d8f74.3 = struct_value () [concrete] +// CHECK:STDOUT: %Destroy.Op.type.1d8f74.5: type = fn_type @Destroy.Op.loc10_40.5 [concrete] // CHECK:STDOUT: %Destroy.Op.1a2547.5: %Destroy.Op.type.1d8f74.5 = struct_value () [concrete] // CHECK:STDOUT: %Destroy.Op.type.1d8f74.6: type = fn_type @Destroy.Op.loc10_39 [concrete] // CHECK:STDOUT: %Destroy.Op.1a2547.6: %Destroy.Op.type.1d8f74.6 = struct_value () [concrete] @@ -391,6 +391,7 @@ fn Run() { // CHECK:STDOUT: // CHECK:STDOUT: fn @Run() { // CHECK:STDOUT: !entry: +// CHECK:STDOUT: %c.var: ref %C = var_storage %c.var_patt // CHECK:STDOUT: name_binding_decl { // CHECK:STDOUT: %c.patt: %pattern_type.98b = ref_binding_pattern c [concrete = constants.%c.patt.dd0] // CHECK:STDOUT: %c.var_patt: %pattern_type.98b = var_pattern %c.patt [concrete = constants.%c.var_patt] @@ -438,7 +439,6 @@ fn Run() { // CHECK:STDOUT: if %.loc10_40.6 br !for.body else br !for.done // CHECK:STDOUT: // CHECK:STDOUT: !for.body: -// CHECK:STDOUT: %c.var: ref %C = var_storage %c.var_patt // CHECK:STDOUT: %.loc10_40.7: %Optional.Get.type.0a8 = specific_constant imports.%Core.import_ref.73b, @Optional(constants.%Copy.facet) [concrete = constants.%Optional.Get.d5d] // CHECK:STDOUT: %Get.ref: %Optional.Get.type.0a8 = name_ref Get, %.loc10_40.7 [concrete = constants.%Optional.Get.d5d] // CHECK:STDOUT: %Optional.Get.bound: = bound_method %.loc10_40.2, %Get.ref @@ -457,38 +457,38 @@ fn Run() { // CHECK:STDOUT: br !for.next // CHECK:STDOUT: // CHECK:STDOUT: !for.done: -// CHECK:STDOUT: %Destroy.Op.bound.loc10_8: = bound_method %c.var, constants.%Destroy.Op.1a2547.2 -// CHECK:STDOUT: %Destroy.Op.call.loc10_8: init %empty_tuple.type = call %Destroy.Op.bound.loc10_8(%c.var) // CHECK:STDOUT: %Destroy.Op.bound.loc10_40.1: = bound_method %.loc10_40.2, constants.%Destroy.Op.1a2547.5 // CHECK:STDOUT: %Destroy.Op.call.loc10_40.1: init %empty_tuple.type = call %Destroy.Op.bound.loc10_40.1(%.loc10_40.2) -// CHECK:STDOUT: %Destroy.Op.bound.loc10_40.2: = bound_method %var, constants.%Destroy.Op.1a2547.1 +// CHECK:STDOUT: %Destroy.Op.bound.loc10_40.2: = bound_method %var, constants.%Destroy.Op.1a2547.2 // CHECK:STDOUT: %Destroy.Op.call.loc10_40.2: init %empty_tuple.type = call %Destroy.Op.bound.loc10_40.2(%var) // CHECK:STDOUT: %Destroy.Op.bound.loc10_39: = bound_method %.loc10_39.2, constants.%Destroy.Op.1a2547.6 // CHECK:STDOUT: %Destroy.Op.call.loc10_39: init %empty_tuple.type = call %Destroy.Op.bound.loc10_39(%.loc10_39.2) +// CHECK:STDOUT: %Destroy.Op.bound.loc10_8: = bound_method %c.var, constants.%Destroy.Op.1a2547.3 +// CHECK:STDOUT: %Destroy.Op.call.loc10_8: init %empty_tuple.type = call %Destroy.Op.bound.loc10_8(%c.var) // CHECK:STDOUT: // CHECK:STDOUT: // CHECK:STDOUT: !observes: // CHECK:STDOUT: } // CHECK:STDOUT: -// CHECK:STDOUT: fn @Destroy.Op.loc10_8.1(%self.param: ref %empty_struct_type) = "no_op"; -// CHECK:STDOUT: -// CHECK:STDOUT: fn @Destroy.Op.loc10_8.2(%self.param: ref %C) { -// CHECK:STDOUT: !entry: -// CHECK:STDOUT: return -// CHECK:STDOUT: -// CHECK:STDOUT: !observes: -// CHECK:STDOUT: } -// CHECK:STDOUT: // CHECK:STDOUT: fn @Destroy.Op.loc10_40.1(%self.param: ref bool) = "no_op"; // CHECK:STDOUT: -// CHECK:STDOUT: fn @Destroy.Op.loc10_40.2(%self.param: ref %struct_type.has_value.value.9e7) { +// CHECK:STDOUT: fn @Destroy.Op.loc10_40.2(%self.param: ref %empty_struct_type) = "no_op"; +// CHECK:STDOUT: +// CHECK:STDOUT: fn @Destroy.Op.loc10_40.3(%self.param: ref %C) { // CHECK:STDOUT: !entry: // CHECK:STDOUT: return // CHECK:STDOUT: // CHECK:STDOUT: !observes: // CHECK:STDOUT: } // CHECK:STDOUT: -// CHECK:STDOUT: fn @Destroy.Op.loc10_40.3(%self.param: ref %Optional.f82) { +// CHECK:STDOUT: fn @Destroy.Op.loc10_40.4(%self.param: ref %struct_type.has_value.value.9e7) { +// CHECK:STDOUT: !entry: +// CHECK:STDOUT: return +// CHECK:STDOUT: +// CHECK:STDOUT: !observes: +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: fn @Destroy.Op.loc10_40.5(%self.param: ref %Optional.f82) { // CHECK:STDOUT: !entry: // CHECK:STDOUT: return // CHECK:STDOUT: diff --git a/toolchain/lower/testdata/for/bindings.carbon b/toolchain/lower/testdata/for/bindings.carbon index cb1fd7c028b2..d5041a54f762 100644 --- a/toolchain/lower/testdata/for/bindings.carbon +++ b/toolchain/lower/testdata/for/bindings.carbon @@ -43,38 +43,38 @@ fn For() { // CHECK:STDOUT: define void @_CFor.Main() #0 !dbg !4 { // CHECK:STDOUT: entry: // CHECK:STDOUT: %r.var = alloca {}, align 1, !dbg !7 -// CHECK:STDOUT: %var = alloca {}, align 1, !dbg !8 -// CHECK:STDOUT: %.loc29_33.1.temp = alloca <{ { i32, i32 }, i1 }>, align 4, !dbg !8 -// CHECK:STDOUT: %n.var = alloca i32, align 4, !dbg !9 -// CHECK:STDOUT: %.loc29_33.8.temp = alloca { i32, i32 }, align 4, !dbg !8 +// CHECK:STDOUT: %n.var = alloca i32, align 4, !dbg !8 +// CHECK:STDOUT: %var = alloca {}, align 1, !dbg !9 +// CHECK:STDOUT: %.loc29_33.1.temp = alloca <{ { i32, i32 }, i1 }>, align 4, !dbg !9 +// CHECK:STDOUT: %.loc29_33.8.temp = alloca { i32, i32 }, align 4, !dbg !9 // CHECK:STDOUT: call void @llvm.lifetime.start.p0(ptr %r.var), !dbg !7 // CHECK:STDOUT: call void @llvm.memcpy.p0.p0.i64(ptr align 1 %r.var, ptr align 1 @EmptyRange.val.loc27_3, i64 0, i1 false), !dbg !7 -// CHECK:STDOUT: call void @"_CNewCursor.EmptyRange.a081b942542726db.Main:Iterate.Core.c9f733d9bf35d2e5"(ptr %r.var), !dbg !8 -// CHECK:STDOUT: call void @llvm.lifetime.start.p0(ptr %var), !dbg !8 -// CHECK:STDOUT: br label %for.next, !dbg !8 +// CHECK:STDOUT: call void @llvm.lifetime.start.p0(ptr %n.var), !dbg !8 +// CHECK:STDOUT: call void @"_CNewCursor.EmptyRange.a081b942542726db.Main:Iterate.Core.c9f733d9bf35d2e5"(ptr %r.var), !dbg !9 +// CHECK:STDOUT: call void @llvm.lifetime.start.p0(ptr %var), !dbg !9 +// CHECK:STDOUT: br label %for.next, !dbg !9 // CHECK:STDOUT: // CHECK:STDOUT: for.next: ; preds = %for.body, %entry -// CHECK:STDOUT: call void @llvm.lifetime.start.p0(ptr %.loc29_33.1.temp), !dbg !8 -// CHECK:STDOUT: call void @"_CNext.EmptyRange.a081b942542726db.Main:Iterate.Core.c9f733d9bf35d2e5"(ptr %.loc29_33.1.temp, ptr %r.var, ptr %var), !dbg !8 -// CHECK:STDOUT: %Optional.HasValue.call = call i1 @_CHasValue.Optional.Core.04b328ae73286f7d(ptr %.loc29_33.1.temp), !dbg !8 -// CHECK:STDOUT: br i1 %Optional.HasValue.call, label %for.body, label %for.done, !dbg !8 +// CHECK:STDOUT: call void @llvm.lifetime.start.p0(ptr %.loc29_33.1.temp), !dbg !9 +// CHECK:STDOUT: call void @"_CNext.EmptyRange.a081b942542726db.Main:Iterate.Core.c9f733d9bf35d2e5"(ptr %.loc29_33.1.temp, ptr %r.var, ptr %var), !dbg !9 +// CHECK:STDOUT: %Optional.HasValue.call = call i1 @_CHasValue.Optional.Core.04b328ae73286f7d(ptr %.loc29_33.1.temp), !dbg !9 +// CHECK:STDOUT: br i1 %Optional.HasValue.call, label %for.body, label %for.done, !dbg !9 // CHECK:STDOUT: // CHECK:STDOUT: for.body: ; preds = %for.next -// CHECK:STDOUT: call void @llvm.lifetime.start.p0(ptr %n.var), !dbg !9 -// CHECK:STDOUT: call void @llvm.lifetime.start.p0(ptr %.loc29_33.8.temp), !dbg !8 -// CHECK:STDOUT: call void @_CGet.Optional.Core.04b328ae73286f7d(ptr %.loc29_33.8.temp, ptr %.loc29_33.1.temp), !dbg !8 -// CHECK:STDOUT: %tuple.elem0.tuple.elem = getelementptr inbounds nuw { i32, i32 }, ptr %.loc29_33.8.temp, i32 0, i32 0, !dbg !8 -// CHECK:STDOUT: %tuple.elem1.tuple.elem = getelementptr inbounds nuw { i32, i32 }, ptr %.loc29_33.8.temp, i32 0, i32 1, !dbg !8 -// CHECK:STDOUT: %.loc29_33.11 = load i32, ptr %tuple.elem0.tuple.elem, align 4, !dbg !8 -// CHECK:STDOUT: %.loc29_33.12 = load i32, ptr %tuple.elem1.tuple.elem, align 4, !dbg !8 -// CHECK:STDOUT: store i32 %.loc29_33.12, ptr %n.var, align 4, !dbg !9 +// CHECK:STDOUT: call void @llvm.lifetime.start.p0(ptr %.loc29_33.8.temp), !dbg !9 +// CHECK:STDOUT: call void @_CGet.Optional.Core.04b328ae73286f7d(ptr %.loc29_33.8.temp, ptr %.loc29_33.1.temp), !dbg !9 +// CHECK:STDOUT: %tuple.elem0.tuple.elem = getelementptr inbounds nuw { i32, i32 }, ptr %.loc29_33.8.temp, i32 0, i32 0, !dbg !9 +// CHECK:STDOUT: %tuple.elem1.tuple.elem = getelementptr inbounds nuw { i32, i32 }, ptr %.loc29_33.8.temp, i32 0, i32 1, !dbg !9 +// CHECK:STDOUT: %.loc29_33.11 = load i32, ptr %tuple.elem0.tuple.elem, align 4, !dbg !9 +// CHECK:STDOUT: %.loc29_33.12 = load i32, ptr %tuple.elem1.tuple.elem, align 4, !dbg !9 +// CHECK:STDOUT: store i32 %.loc29_33.12, ptr %n.var, align 4, !dbg !8 // CHECK:STDOUT: call void @_CF.Main(i32 %.loc29_33.11, ptr %n.var), !dbg !10 // CHECK:STDOUT: br label %for.next, !dbg !11 // CHECK:STDOUT: // CHECK:STDOUT: for.done: ; preds = %for.next -// CHECK:STDOUT: call void @"_COp.eff30dc40f4a53d2:core.Destroy.Core"(ptr %.loc29_33.8.temp), !dbg !8 -// CHECK:STDOUT: call void @"_COp.5e27612b9dd31a14:core.Destroy.Core"(ptr %n.var), !dbg !9 -// CHECK:STDOUT: call void @"_COp.a79511dff093d1a6:core.Destroy.Core"(ptr %.loc29_33.1.temp), !dbg !8 +// CHECK:STDOUT: call void @"_COp.eff30dc40f4a53d2:core.Destroy.Core"(ptr %.loc29_33.8.temp), !dbg !9 +// CHECK:STDOUT: call void @"_COp.a79511dff093d1a6:core.Destroy.Core"(ptr %.loc29_33.1.temp), !dbg !9 +// CHECK:STDOUT: call void @"_COp.5e27612b9dd31a14:core.Destroy.Core"(ptr %n.var), !dbg !8 // CHECK:STDOUT: call void @"_COp.88c97397fb274b67:core.Destroy.Core"(ptr %r.var), !dbg !7 // CHECK:STDOUT: ret void, !dbg !12 // CHECK:STDOUT: } @@ -218,8 +218,8 @@ fn For() { // CHECK:STDOUT: !5 = !DISubroutineType(types: !6) // CHECK:STDOUT: !6 = !{null} // CHECK:STDOUT: !7 = !DILocation(line: 27, column: 3, scope: !4) -// CHECK:STDOUT: !8 = !DILocation(line: 29, column: 7, scope: !4) -// CHECK:STDOUT: !9 = !DILocation(line: 29, column: 17, scope: !4) +// CHECK:STDOUT: !8 = !DILocation(line: 29, column: 17, scope: !4) +// CHECK:STDOUT: !9 = !DILocation(line: 29, column: 7, scope: !4) // CHECK:STDOUT: !10 = !DILocation(line: 30, column: 5, scope: !4) // CHECK:STDOUT: !11 = !DILocation(line: 29, column: 3, scope: !4) // CHECK:STDOUT: !12 = !DILocation(line: 26, column: 1, scope: !4)