Emit VarStorages eagerly (#7468)

We used to have to batch these at the end of pattern traversal in order
to avoid accidentally adding them to a block that was speculatively
pushed for an expression within a pattern, but that's no longer a
concern with the more precise handling of those speculative blocks in
#7445.
This commit is contained in:
Geoff Romer
2026-07-08 19:33:46 +00:00
committed by GitHub
parent 4261bb2dd2
commit 96c7cfe41c
6 changed files with 77 additions and 93 deletions
+1 -9
View File
@@ -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
+19 -23
View File
@@ -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<BindingInfo> bind_name_stack_;
struct VarInfo {
SemIR::InstId pattern_id;
SemIR::InstId storage_id;
};
// The `var` patterns introduced by the currently pending full-patterns.
ArrayStack<VarInfo> var_pattern_stack_;
+14 -14
View File
@@ -107,6 +107,18 @@ auto HandleParseNode(Context& context, Parse::VariablePatternId node_id)
return true;
}
auto add_local_var = [&]() -> SemIR::InstId {
auto pattern_id = AddInst<SemIR::VarPattern>(
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<SemIR::VarPattern>(
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<SemIR::VarPattern>(
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 {
@@ -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 `<element>.Get()`.
auto element_value_id =
CallOptionalAccessor(context, node_id, element_id, CoreIdentifier::Get);
+19 -19
View File
@@ -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> = specific_function %Optional.HasValue.a88, @Optional.HasValue(%Copy.facet) [concrete]
// CHECK:STDOUT: %Optional.Get.specific_fn: <specific function> = 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> = 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> = 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> = 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> = bound_method %var, constants.%Destroy.Op.1a2547.1
// CHECK:STDOUT: %Destroy.Op.bound.loc10_40.2: <bound method> = 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> = 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> = 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: <elided>
// 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:
+24 -24
View File
@@ -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)