| author |  Matthew Lugg <mlugg@mlugg.co.uk> 2026-04-15 23:55:52+01:00 |
| committer | Matthew Lugg <mlugg@mlugg.co.uk> 2026-04-16 08:52:02+01:00 |
| log | 73fcba1ef448f369cc4d8f6824f224414cb850ba |
| tree | 7e0a8c180bc7622f0e4ad7b9542018373a7703cb |
| parent | 019395bd8b4f4d15a92bc75fbdcd5868f3770635 |
| signature |  Commit is signed but in an unrecognized format. |
incremental: fix tracking of nested container declarations
...but not in the way you'd expect. We were actually tracking them in
cases where we shouldn't have been! We cannot track a declaration if its
parent namespace has been lost, because that will cause analysis
failures immediately, but if we excluded a type from the mapping due to
a major change (such as a struct turning into a union, or a field being
added), we were still including any trackable instructions inside the
container's field expressions (e.g. struct field type expressions). This
meant we were tracking a type declaration while losing tracking on its
parent namespace, with predictably disastrous results.
Oh, also, tracking for opaque types was just totally wrong (I think this
was a typo from a while back). We could map it to things other than
opaque declarations, and we never mapped declarations inside opaques.
So, uh, I fixed that too.4 files changed, 162 insertions(+), 91 deletions(-)
lib/std/zig/Zir.zig+50-47
| ... | ... | @@ -4033,30 +4033,30 @@ pub const DeclContents = struct { |
| 4033 | 4033 | /// This is a simple optional because ZIR guarantees that a `func`/`func_inferred`/`func_fancy` instruction |
| 4034 | 4034 | /// can only occur once per `declaration`. |
| 4035 | 4035 | func_decl: ?Inst.Index, |
| 4036 | explicit_types: std.ArrayList(Inst.Index), | |
| 4036 | type_decls: std.ArrayList(Inst.Index), | |
| 4037 | 4037 | other: std.ArrayList(Inst.Index), |
| 4038 | 4038 | |
| 4039 | 4039 | pub const init: DeclContents = .{ |
| 4040 | 4040 | .func_decl = null, |
| 4041 | .explicit_types = .empty, | |
| 4041 | .type_decls = .empty, | |
| 4042 | 4042 | .other = .empty, |
| 4043 | 4043 | }; |
| 4044 | 4044 | |
| 4045 | 4045 | pub fn clear(contents: *DeclContents) void { |
| 4046 | 4046 | contents.func_decl = null; |
| 4047 | contents.explicit_types.clearRetainingCapacity(); | |
| 4047 | contents.type_decls.clearRetainingCapacity(); | |
| 4048 | 4048 | contents.other.clearRetainingCapacity(); |
| 4049 | 4049 | } |
| 4050 | 4050 | |
| 4051 | 4051 | pub fn deinit(contents: *DeclContents, gpa: Allocator) void { |
| 4052 | contents.explicit_types.deinit(gpa); | |
| 4052 | contents.type_decls.deinit(gpa); | |
| 4053 | 4053 | contents.other.deinit(gpa); |
| 4054 | 4054 | } |
| 4055 | 4055 | }; |
| 4056 | 4056 | |
| 4057 | 4057 | /// Find all tracked ZIR instructions, recursively, within a `declaration` instruction. Does not recurse through |
| 4058 | 4058 | /// nested declarations; to find all declarations, call this function recursively on the type declarations discovered |
| 4059 | /// in `contents.explicit_types`. | |
| 4059 | /// in `contents.type_decls`. | |
| 4060 | 4060 | /// |
| 4061 | 4061 | /// This populates an `ArrayList` because an iterator would need to allocate memory anyway. |
| 4062 | 4062 | pub fn findTrackable(zir: Zir, gpa: Allocator, contents: *DeclContents, decl_inst: Zir.Inst.Index) !void { |
| ... | ... | @@ -4076,15 +4076,49 @@ pub fn findTrackable(zir: Zir, gpa: Allocator, contents: *DeclContents, decl_ins |
| 4076 | 4076 | if (decl.value_body) |b| try zir.findTrackableBody(gpa, contents, &found_defers, b); |
| 4077 | 4077 | } |
| 4078 | 4078 | |
| 4079 | /// Like `findTrackable`, but only considers the `main_struct_inst` instruction. This may return more than | |
| 4080 | /// just that instruction because it will also traverse fields. | |
| 4081 | pub fn findTrackableRoot(zir: Zir, gpa: Allocator, contents: *DeclContents) !void { | |
| 4079 | /// `findTrackable` does not recurse into field expressions in a type. Instead, this function will | |
| 4080 | /// scan specifically field expressions in a given type declaration for trackable ZIR instructions. | |
| 4081 | pub fn findTrackableFields( | |
| 4082 | zir: *const Zir, | |
| 4083 | gpa: Allocator, | |
| 4084 | contents: *DeclContents, | |
| 4085 | type_decl_inst: Zir.Inst.Index, | |
| 4086 | ) Allocator.Error!void { | |
| 4082 | 4087 | contents.clear(); |
| 4083 | 4088 | |
| 4084 | 4089 | var found_defers: std.AutoHashMapUnmanaged(u32, void) = .empty; |
| 4085 | 4090 | defer found_defers.deinit(gpa); |
| 4086 | 4091 | |
| 4087 | try zir.findTrackableInner(gpa, contents, &found_defers, .main_struct_inst); | |
| 4092 | assert(zir.instructions.items(.tag)[@intFromEnum(type_decl_inst)] == .extended); | |
| 4093 | switch (zir.instructions.items(.data)[@intFromEnum(type_decl_inst)].extended.opcode) { | |
| 4094 | .struct_decl => { | |
| 4095 | const struct_decl = zir.getStructDecl(type_decl_inst); | |
| 4096 | var it = struct_decl.iterateFields(); | |
| 4097 | while (it.next()) |field| { | |
| 4098 | try zir.findTrackableBody(gpa, contents, &found_defers, field.type_body); | |
| 4099 | if (field.align_body) |b| try zir.findTrackableBody(gpa, contents, &found_defers, b); | |
| 4100 | if (field.default_body) |b| try zir.findTrackableBody(gpa, contents, &found_defers, b); | |
| 4101 | } | |
| 4102 | }, | |
| 4103 | .union_decl => { | |
| 4104 | const union_decl = zir.getUnionDecl(type_decl_inst); | |
| 4105 | var it = union_decl.iterateFields(); | |
| 4106 | while (it.next()) |field| { | |
| 4107 | if (field.type_body) |b| try zir.findTrackableBody(gpa, contents, &found_defers, b); | |
| 4108 | if (field.align_body) |b| try zir.findTrackableBody(gpa, contents, &found_defers, b); | |
| 4109 | if (field.value_body) |b| try zir.findTrackableBody(gpa, contents, &found_defers, b); | |
| 4110 | } | |
| 4111 | }, | |
| 4112 | .enum_decl => { | |
| 4113 | const enum_decl = zir.getEnumDecl(type_decl_inst); | |
| 4114 | var it = enum_decl.iterateFields(); | |
| 4115 | while (it.next()) |field| { | |
| 4116 | if (field.value_body) |b| try zir.findTrackableBody(gpa, contents, &found_defers, b); | |
| 4117 | } | |
| 4118 | }, | |
| 4119 | .opaque_decl => {}, | |
| 4120 | else => unreachable, | |
| 4121 | } | |
| 4088 | 4122 | } |
| 4089 | 4123 | |
| 4090 | 4124 | fn findTrackableInner( |
| ... | ... | @@ -4411,49 +4445,18 @@ fn findTrackableInner( |
| 4411 | 4445 | try zir.findTrackableBody(gpa, contents, defers, body); |
| 4412 | 4446 | }, |
| 4413 | 4447 | |
| 4414 | // Reifications and opaque declarations need tracking, but have no bodies. | |
| 4448 | // Reifications need tracking. | |
| 4415 | 4449 | .reify_enum, |
| 4416 | 4450 | .reify_struct, |
| 4417 | 4451 | .reify_union, |
| 4418 | .opaque_decl, | |
| 4419 | 4452 | => return contents.other.append(gpa, inst), |
| 4420 | 4453 | |
| 4421 | // Struct declarations need tracking and have bodies. | |
| 4422 | .struct_decl => { | |
| 4423 | try contents.explicit_types.append(gpa, inst); | |
| 4424 | ||
| 4425 | const struct_decl = zir.getStructDecl(inst); | |
| 4426 | var it = struct_decl.iterateFields(); | |
| 4427 | while (it.next()) |field| { | |
| 4428 | try zir.findTrackableBody(gpa, contents, defers, field.type_body); | |
| 4429 | if (field.align_body) |b| try zir.findTrackableBody(gpa, contents, defers, b); | |
| 4430 | if (field.default_body) |b| try zir.findTrackableBody(gpa, contents, defers, b); | |
| 4431 | } | |
| 4432 | }, | |
| 4433 | ||
| 4434 | // Union declarations need tracking and have bodies. | |
| 4435 | .union_decl => { | |
| 4436 | try contents.explicit_types.append(gpa, inst); | |
| 4437 | ||
| 4438 | const union_decl = zir.getUnionDecl(inst); | |
| 4439 | var it = union_decl.iterateFields(); | |
| 4440 | while (it.next()) |field| { | |
| 4441 | if (field.type_body) |b| try zir.findTrackableBody(gpa, contents, defers, b); | |
| 4442 | if (field.align_body) |b| try zir.findTrackableBody(gpa, contents, defers, b); | |
| 4443 | if (field.value_body) |b| try zir.findTrackableBody(gpa, contents, defers, b); | |
| 4444 | } | |
| 4445 | }, | |
| 4446 | ||
| 4447 | // Enum declarations need tracking and have bodies. | |
| 4448 | .enum_decl => { | |
| 4449 | try contents.explicit_types.append(gpa, inst); | |
| 4450 | ||
| 4451 | const enum_decl = zir.getEnumDecl(inst); | |
| 4452 | var it = enum_decl.iterateFields(); | |
| 4453 | while (it.next()) |field| { | |
| 4454 | if (field.value_body) |b| try zir.findTrackableBody(gpa, contents, defers, b); | |
| 4455 | } | |
| 4456 | }, | |
| 4454 | // Type declarations need tracking. | |
| 4455 | .struct_decl, | |
| 4456 | .union_decl, | |
| 4457 | .enum_decl, | |
| 4458 | .opaque_decl, | |
| 4459 | => return contents.type_decls.append(gpa, inst), | |
| 4457 | 4460 | } |
| 4458 | 4461 | }, |
| 4459 | 4462 |
src/Zcu.zig+46-44
| ... | ... | @@ -3361,8 +3361,8 @@ pub fn mapOldZirToNew( |
| 3361 | 3361 | old_inst: Zir.Inst.Index, |
| 3362 | 3362 | new_inst: Zir.Inst.Index, |
| 3363 | 3363 | }; |
| 3364 | var match_stack: std.ArrayList(MatchedZirDecl) = .empty; | |
| 3365 | defer match_stack.deinit(gpa); | |
| 3364 | var pending_matched_type_decls: std.ArrayList(MatchedZirDecl) = .empty; | |
| 3365 | defer pending_matched_type_decls.deinit(gpa); | |
| 3366 | 3366 | |
| 3367 | 3367 | // Used as temporary buffers for namespace declaration instructions |
| 3368 | 3368 | var old_contents: Zir.DeclContents = .init; |
| ... | ... | @@ -3370,42 +3370,13 @@ pub fn mapOldZirToNew( |
| 3370 | 3370 | var new_contents: Zir.DeclContents = .init; |
| 3371 | 3371 | defer new_contents.deinit(gpa); |
| 3372 | 3372 | |
| 3373 | // Map the main struct inst (and anything in its fields) | |
| 3374 | { | |
| 3375 | try old_zir.findTrackableRoot(gpa, &old_contents); | |
| 3376 | try new_zir.findTrackableRoot(gpa, &new_contents); | |
| 3377 | ||
| 3378 | assert(old_contents.explicit_types.items[0] == .main_struct_inst); | |
| 3379 | assert(new_contents.explicit_types.items[0] == .main_struct_inst); | |
| 3380 | ||
| 3381 | assert(old_contents.func_decl == null); | |
| 3382 | assert(new_contents.func_decl == null); | |
| 3383 | ||
| 3384 | // We don't have any smart way of matching up these instructions, so we correlate them based on source order | |
| 3385 | // in their respective arrays. | |
| 3386 | ||
| 3387 | const num_explicit_types = @min(old_contents.explicit_types.items.len, new_contents.explicit_types.items.len); | |
| 3388 | try match_stack.ensureUnusedCapacity(gpa, @intCast(num_explicit_types)); | |
| 3389 | for ( | |
| 3390 | old_contents.explicit_types.items[0..num_explicit_types], | |
| 3391 | new_contents.explicit_types.items[0..num_explicit_types], | |
| 3392 | ) |old_inst, new_inst| { | |
| 3393 | // Here we use `match_stack`, so that we will recursively consider declarations on these types. | |
| 3394 | match_stack.appendAssumeCapacity(.{ .old_inst = old_inst, .new_inst = new_inst }); | |
| 3395 | } | |
| 3396 | ||
| 3397 | const num_other = @min(old_contents.other.items.len, new_contents.other.items.len); | |
| 3398 | try inst_map.ensureUnusedCapacity(gpa, @intCast(num_other)); | |
| 3399 | for ( | |
| 3400 | old_contents.other.items[0..num_other], | |
| 3401 | new_contents.other.items[0..num_other], | |
| 3402 | ) |old_inst, new_inst| { | |
| 3403 | // These instructions don't have declarations, so we just modify `inst_map` directly. | |
| 3404 | inst_map.putAssumeCapacity(old_inst, new_inst); | |
| 3405 | } | |
| 3406 | } | |
| 3373 | // Map the main struct inst to start off with. | |
| 3374 | try pending_matched_type_decls.append(gpa, .{ | |
| 3375 | .old_inst = .main_struct_inst, | |
| 3376 | .new_inst = .main_struct_inst, | |
| 3377 | }); | |
| 3407 | 3378 | |
| 3408 | while (match_stack.pop()) |match_item| { | |
| 3379 | while (pending_matched_type_decls.pop()) |match_item| { | |
| 3409 | 3380 | // There are some properties of type declarations which cannot change across incremental |
| 3410 | 3381 | // updates. If they have, we need to ignore this mapping. These properties are essentially |
| 3411 | 3382 | // everything passed into `InternPool.getDeclaredStructType` (likewise for unions, enums, |
| ... | ... | @@ -3461,9 +3432,41 @@ pub fn mapOldZirToNew( |
| 3461 | 3432 | else => unreachable, |
| 3462 | 3433 | } |
| 3463 | 3434 | |
| 3464 | // Match the namespace declaration itself | |
| 3435 | // Match the container declaration itself | |
| 3465 | 3436 | try inst_map.put(gpa, match_item.old_inst, match_item.new_inst); |
| 3466 | 3437 | |
| 3438 | { | |
| 3439 | // First, map the fields... | |
| 3440 | try old_zir.findTrackableFields(gpa, &old_contents, match_item.old_inst); | |
| 3441 | try new_zir.findTrackableFields(gpa, &new_contents, match_item.new_inst); | |
| 3442 | ||
| 3443 | // This isn't a `.declaration`, so we shouldn't see a function declaration. | |
| 3444 | assert(old_contents.func_decl == null); | |
| 3445 | assert(new_contents.func_decl == null); | |
| 3446 | ||
| 3447 | // We don't have any smart way of matching up these instructions, so we correlate them based on source order | |
| 3448 | // in their respective arrays. | |
| 3449 | ||
| 3450 | const num_type_decls = @min(old_contents.type_decls.items.len, new_contents.type_decls.items.len); | |
| 3451 | try pending_matched_type_decls.ensureUnusedCapacity(gpa, @intCast(num_type_decls)); | |
| 3452 | for ( | |
| 3453 | old_contents.type_decls.items[0..num_type_decls], | |
| 3454 | new_contents.type_decls.items[0..num_type_decls], | |
| 3455 | ) |old_inst, new_inst| { | |
| 3456 | pending_matched_type_decls.appendAssumeCapacity(.{ .old_inst = old_inst, .new_inst = new_inst }); | |
| 3457 | } | |
| 3458 | ||
| 3459 | const num_other = @min(old_contents.other.items.len, new_contents.other.items.len); | |
| 3460 | try inst_map.ensureUnusedCapacity(gpa, @intCast(num_other)); | |
| 3461 | for ( | |
| 3462 | old_contents.other.items[0..num_other], | |
| 3463 | new_contents.other.items[0..num_other], | |
| 3464 | ) |old_inst, new_inst| { | |
| 3465 | // These instructions don't have declarations, so we just modify `inst_map` directly. | |
| 3466 | inst_map.putAssumeCapacity(old_inst, new_inst); | |
| 3467 | } | |
| 3468 | } | |
| 3469 | ||
| 3467 | 3470 | // Maps decl name to `declaration` instruction. |
| 3468 | 3471 | var named_decls: std.StringHashMapUnmanaged(Zir.Inst.Index) = .empty; |
| 3469 | 3472 | defer named_decls.deinit(gpa); |
| ... | ... | @@ -3537,14 +3540,13 @@ pub fn mapOldZirToNew( |
| 3537 | 3540 | // We don't have any smart way of matching up these instructions, so we correlate them based on source order |
| 3538 | 3541 | // in their respective arrays. |
| 3539 | 3542 | |
| 3540 | const num_explicit_types = @min(old_contents.explicit_types.items.len, new_contents.explicit_types.items.len); | |
| 3541 | try match_stack.ensureUnusedCapacity(gpa, @intCast(num_explicit_types)); | |
| 3543 | const num_type_decls = @min(old_contents.type_decls.items.len, new_contents.type_decls.items.len); | |
| 3544 | try pending_matched_type_decls.ensureUnusedCapacity(gpa, @intCast(num_type_decls)); | |
| 3542 | 3545 | for ( |
| 3543 | old_contents.explicit_types.items[0..num_explicit_types], | |
| 3544 | new_contents.explicit_types.items[0..num_explicit_types], | |
| 3546 | old_contents.type_decls.items[0..num_type_decls], | |
| 3547 | new_contents.type_decls.items[0..num_type_decls], | |
| 3545 | 3548 | ) |old_inst, new_inst| { |
| 3546 | // Here we use `match_stack`, so that we will recursively consider declarations on these types. | |
| 3547 | match_stack.appendAssumeCapacity(.{ .old_inst = old_inst, .new_inst = new_inst }); | |
| 3549 | pending_matched_type_decls.appendAssumeCapacity(.{ .old_inst = old_inst, .new_inst = new_inst }); | |
| 3548 | 3550 | } |
| 3549 | 3551 | |
| 3550 | 3552 | const num_other = @min(old_contents.other.items.len, new_contents.other.items.len); |
test/incremental/add_field_and_nested_struct_uses_changed_decl created+25
| ... | ... | @@ -0,0 +1,25 @@ |
| 1 | #update=initial version | |
| 2 | #file=main.zig | |
| 3 | pub fn main() void { | |
| 4 | _ = @as(S, undefined); | |
| 5 | } | |
| 6 | // To reproduce the original bug, the inner struct must perform a namespace lookup | |
| 7 | // or a scope lookup when resolving its field type. | |
| 8 | const SomeType = u8; | |
| 9 | const S = struct { | |
| 10 | foo: struct { inner: SomeType }, | |
| 11 | }; | |
| 12 | #expect_stdout="" | |
| 13 | #update=add field to outer struct, change decl used by inner struct | |
| 14 | #file=main.zig | |
| 15 | pub fn main() void { | |
| 16 | _ = @as(S, undefined); | |
| 17 | } | |
| 18 | // To reproduce the original bug, the inner struct must perform a namespace lookup | |
| 19 | // or a scope lookup when resolving its field type. | |
| 20 | const SomeType = u16; | |
| 21 | const S = struct { | |
| 22 | foo: struct { inner: SomeType }, | |
| 23 | bar: u32, | |
| 24 | }; | |
| 25 | #expect_stdout="" |
test/incremental/do_nothing created+41
| ... | ... | @@ -0,0 +1,41 @@ |
| 1 | // TODO: it'd be great if we could actually check that no analysis happened! | |
| 2 | #update=initial version | |
| 3 | #file=main.zig | |
| 4 | pub fn main() void { | |
| 5 | const ptr: *const O = @ptrFromInt(0x1000); | |
| 6 | _ = ptr; | |
| 7 | } | |
| 8 | const S = struct { foo: u32, nested: struct { x: u16 } }; | |
| 9 | const U = union(enum) { a, b, c: S }; | |
| 10 | const E = enum(u8) { a = @typeInfo(U).@"union".fields.len, b = 0, c }; | |
| 11 | const O = opaque { | |
| 12 | comptime { | |
| 13 | _ = @as(S, undefined); | |
| 14 | _ = @as(U, undefined); | |
| 15 | _ = @as(E, undefined); | |
| 16 | const Wrapper = struct { val: S }; | |
| 17 | const wrapper: Wrapper = .{ .val = .{ .foo = 123, .nested = .{ .x = 456 } } }; | |
| 18 | _ = wrapper; | |
| 19 | } | |
| 20 | }; | |
| 21 | #expect_stdout="" | |
| 22 | #update=do literally nothing | |
| 23 | #file=main.zig | |
| 24 | pub fn main() void { | |
| 25 | const ptr: *const O = @ptrFromInt(0x1000); | |
| 26 | _ = ptr; | |
| 27 | } | |
| 28 | const S = struct { foo: u32, nested: struct { x: u16 } }; | |
| 29 | const U = union(enum) { a, b, c: S }; | |
| 30 | const E = enum(u8) { a = @typeInfo(U).@"union".fields.len, b = 0, c }; | |
| 31 | const O = opaque { | |
| 32 | comptime { | |
| 33 | _ = @as(S, undefined); | |
| 34 | _ = @as(U, undefined); | |
| 35 | _ = @as(E, undefined); | |
| 36 | const Wrapper = struct { val: S }; | |
| 37 | const wrapper: Wrapper = .{ .val = .{ .foo = 123, .nested = .{ .x = 456 } } }; | |
| 38 | _ = wrapper; | |
| 39 | } | |
| 40 | }; | |
| 41 | #expect_stdout="" |