Commit f818098971fdafdaaa6af4b927f2cf47332ed5f7 | zig

author	Matthew Lugg <mlugg@mlugg.co.uk> 2025-01-03 23:48:27+00:00
committer	Matthew Lugg <mlugg@mlugg.co.uk> 2025-01-04 05:44:29+00:00
log	f818098971fdafdaaa6af4b927f2cf47332ed5f7
tree	68692d31a8c939887864ec302d8e8378c7c87cfd
parent	814491f59921600b8a0137733b48d4b0f6b99d78
signature	Commit is signed but in an unrecognized format.

incremental: correctly return `error.AnalysisFail` when type structure changes

`Zcu.PerThead.ensureTypeUpToDate` is set up in such a way that it only
returns the updated type the first time it is called. In general, that's
okay; however, the exception is that we want the function to continue
returning `error.AnalysisFail` when the type has been lost, or its
number of captures changed.

Therefore, the check for this case now happens before the up-to-date
success return.

For simplicity, the number of captures is now handled by intentionally
losing the instruction in `Zcu.mapOldZirToNew`, since there is nothing
to gain from tracking a type when old instances of it can never be
reused.

5 files changed, 96 insertions(+), 62 deletions(-)

lib
- std
  - zig
    Zir.zig
src

lib/std/zig/Zir.zig+32

...	@@ -5066,3 +5066,35 @@ pub fn assertTrackable(zir: Zir, inst_idx: Zir.Inst.Index) void {	...	@@ -5066,3 +5066,35 @@ pub fn assertTrackable(zir: Zir, inst_idx: Zir.Inst.Index) void {
5066	else => unreachable, // assertion failure; not trackable	5066	else => unreachable, // assertion failure; not trackable
5067	}	5067	}
5068	}	5068	}
		5069
		5070	pub fn typeCapturesLen(zir: Zir, type_decl: Inst.Index) u32 {
		5071	const inst = zir.instructions.get(@intFromEnum(type_decl));
		5072	assert(inst.tag == .extended);
		5073	switch (inst.data.extended.opcode) {
		5074	.struct_decl => {
		5075	const small: Inst.StructDecl.Small = @bitCast(inst.data.extended.small);
		5076	if (!small.has_captures_len) return 0;
		5077	const extra = zir.extraData(Inst.StructDecl, inst.data.extended.operand);
		5078	return zir.extra[extra.end];
		5079	},
		5080	.union_decl => {
		5081	const small: Inst.UnionDecl.Small = @bitCast(inst.data.extended.small);
		5082	if (!small.has_captures_len) return 0;
		5083	const extra = zir.extraData(Inst.UnionDecl, inst.data.extended.operand);
		5084	return zir.extra[extra.end + @intFromBool(small.has_tag_type)];
		5085	},
		5086	.enum_decl => {
		5087	const small: Inst.EnumDecl.Small = @bitCast(inst.data.extended.small);
		5088	if (!small.has_captures_len) return 0;
		5089	const extra = zir.extraData(Inst.EnumDecl, inst.data.extended.operand);
		5090	return zir.extra[extra.end + @intFromBool(small.has_tag_type)];
		5091	},
		5092	.opaque_decl => {
		5093	const small: Inst.OpaqueDecl.Small = @bitCast(inst.data.extended.small);
		5094	if (!small.has_captures_len) return 0;
		5095	const extra = zir.extraData(Inst.OpaqueDecl, inst.data.extended.operand);
		5096	return zir.extra[extra.end];
		5097	},
		5098	else => unreachable,
		5099	}
		5100	}

src/InternPool.zig+11-9

 @@ -2029,15 +2029,7 @@ pub const Key = union(enum) {
     pub const NamespaceType = union(enum) {
         /// This type corresponds to an actual source declaration, e.g. `struct { ... }`.
         /// It is hashed based on its ZIR instruction index and set of captures.
-        declared: struct {
+        declared: Declared,
-            /// A `struct_decl`, `union_decl`, `enum_decl`, or `opaque_decl` instruction.
-            zir_index: TrackedInst.Index,
-            /// The captured values of this type. These values must be fully resolved per the language spec.
-            captures: union(enum) {
-                owned: CaptureValue.Slice,
-                external: []const CaptureValue,
-            },
-        },
         /// This type is an automatically-generated enum tag type for a union.
         /// It is hashed based on the index of the union type it corresponds to.
         generated_tag: struct {
 @@ -2053,6 +2045,16 @@ pub const Key = union(enum) {
             /// A hash of this type's attributes, fields, etc, generated by Sema.
             type_hash: u64,
         },
+        pub const Declared = struct {
+            /// A `struct_decl`, `union_decl`, `enum_decl`, or `opaque_decl` instruction.
+            zir_index: TrackedInst.Index,
+            /// The captured values of this type. These values must be fully resolved per the language spec.
+            captures: union(enum) {
+                owned: CaptureValue.Slice,
+                external: []const CaptureValue,
+            },
+        };
     };
     pub const FuncType = struct {

src/Sema.zig+2

...	@@ -18172,7 +18172,9 @@ fn zirThis(	...	@@ -18172,7 +18172,9 @@ fn zirThis(
18172	_ = extended;	18172	_ = extended;
18173	const pt = sema.pt;	18173	const pt = sema.pt;
18174	const namespace = pt.zcu.namespacePtr(block.namespace);	18174	const namespace = pt.zcu.namespacePtr(block.namespace);
		18175
18175	const new_ty = try pt.ensureTypeUpToDate(namespace.owner_type);	18176	const new_ty = try pt.ensureTypeUpToDate(namespace.owner_type);
		18177
18176	switch (pt.zcu.intern_pool.indexToKey(new_ty)) {	18178	switch (pt.zcu.intern_pool.indexToKey(new_ty)) {
18177	.struct_type, .union_type, .enum_type => try sema.declareDependency(.{ .interned = new_ty }),	18179	.struct_type, .union_type, .enum_type => try sema.declareDependency(.{ .interned = new_ty }),
18178	.opaque_type => {},	18180	.opaque_type => {},

src/Zcu.zig+8-13

 @@ -2563,19 +2563,6 @@ pub fn findOutdatedToAnalyze(zcu: *Zcu) Allocator.Error!?AnalUnit {
         }
     }
-    if (chosen_unit == null) {
-        for (zcu.outdated.keys(), zcu.outdated.values()) |o, opod| {
-            const func = o.unwrap().func;
-            const nav = zcu.funcInfo(func).owner_nav;
-            std.io.getStdErr().writer().print("outdated: func {}, nav {}, name '{}', [p]o deps {}\n", .{ func, nav, ip.getNav(nav).fqn.fmt(ip), opod }) catch {};
-        }
-        for (zcu.potentially_outdated.keys(), zcu.potentially_outdated.values()) |o, opod| {
-            const func = o.unwrap().func;
-            const nav = zcu.funcInfo(func).owner_nav;
-            std.io.getStdErr().writer().print("po: func {}, nav {}, name '{}', [p]o deps {}\n", .{ func, nav, ip.getNav(nav).fqn.fmt(ip), opod }) catch {};
-        }
-    }
     log.debug("findOutdatedToAnalyze: heuristic returned '{}' ({d} dependers)", .{
         zcu.fmtAnalUnit(chosen_unit.?),
         chosen_unit_dependers,
 @@ -2661,6 +2648,14 @@ pub fn mapOldZirToNew(
     }
     while (match_stack.popOrNull()) |match_item| {
+        // First, a check: if the number of captures of this type has changed, we can't map it, because
+        // we wouldn't know how to correlate type information with the last update.
+        // Synchronizes with logic in `Zcu.PerThread.recreateStructType` etc.
+        if (old_zir.typeCapturesLen(match_item.old_inst) != new_zir.typeCapturesLen(match_item.new_inst)) {
+            // Don't map this type or anything within it.
+            continue;
+        }
         // Match the namespace declaration itself
         try inst_map.put(gpa, match_item.old_inst, match_item.new_inst);

src/Zcu/PerThread.zig+43-40

 @@ -3524,9 +3524,11 @@ pub fn navAlignment(pt: Zcu.PerThread, nav_index: InternPool.Nav.Index) InternPo
     return ty.abiAlignment(zcu);
 }
-/// Given a container type requiring resolution, ensures that it is up-to-date.
+/// `ty` is a container type requiring resolution (struct, union, or enum).
-/// If not, the type is recreated at a new `InternPool.Index`.
+/// If `ty` is outdated, it is recreated at a new `InternPool.Index`, which is returned.
-/// The new index is returned. This is the same as the old index if the fields were up-to-date.
+/// If the type cannot be recreated because it has been lost, `error.AnalysisFail` is returned.
+/// If `ty` is not outdated, that same `InternPool.Index` is returned.
+/// If `ty` has already been replaced by this function, the new index will not be returned again.
 pub fn ensureTypeUpToDate(pt: Zcu.PerThread, ty: InternPool.Index) Zcu.SemaError!InternPool.Index {
     const zcu = pt.zcu;
     const gpa = zcu.gpa;
 @@ -3536,13 +3538,30 @@ pub fn ensureTypeUpToDate(pt: Zcu.PerThread, ty: InternPool.Index) Zcu.SemaError
     const outdated = zcu.outdated.swapRemove(anal_unit) or
         zcu.potentially_outdated.swapRemove(anal_unit);
+    if (outdated) {
+        _ = zcu.outdated_ready.swapRemove(anal_unit);
+        try zcu.markDependeeOutdated(.marked_po, .{ .interned = ty });
+    }
+    const ty_key = switch (ip.indexToKey(ty)) {
+        .struct_type, .union_type, .enum_type => |key| key,
+        else => unreachable,
+    };
+    const declared_ty_key = switch (ty_key) {
+        .reified => unreachable, // never outdated
+        .generated_tag => unreachable, // never outdated
+        .declared => |d| d,
+    };
+    if (declared_ty_key.zir_index.resolve(ip) == null) {
+        // The instruction has been lost -- this type is dead.
+        return error.AnalysisFail;
+    }
     if (!outdated) return ty;
     // We will recreate the type at a new `InternPool.Index`.
-    _ = zcu.outdated_ready.swapRemove(anal_unit);
-    try zcu.markDependeeOutdated(.marked_po, .{ .interned = ty });
     // Delete old state which is no longer in use. Technically, this is not necessary: these exports,
     // references, etc, will be ignored because the type itself is unreferenced. However, it allows
     // reusing the memory which is currently being used to track this state.
 @@ -3555,9 +3574,9 @@ pub fn ensureTypeUpToDate(pt: Zcu.PerThread, ty: InternPool.Index) Zcu.SemaError
     zcu.intern_pool.removeDependenciesForDepender(gpa, anal_unit);
     switch (ip.indexToKey(ty)) {
-        .struct_type => |key| return pt.recreateStructType(ty, key),
+        .struct_type => return pt.recreateStructType(ty, declared_ty_key),
-        .union_type => |key| return pt.recreateUnionType(ty, key),
+        .union_type => return pt.recreateUnionType(ty, declared_ty_key),
-        .enum_type => |key| return pt.recreateEnumType(ty, key),
+        .enum_type => return pt.recreateEnumType(ty, declared_ty_key),
         else => unreachable,
     }
 }
 @@ -3565,21 +3584,15 @@ pub fn ensureTypeUpToDate(pt: Zcu.PerThread, ty: InternPool.Index) Zcu.SemaError
 fn recreateStructType(
     pt: Zcu.PerThread,
     old_ty: InternPool.Index,
-    full_key: InternPool.Key.NamespaceType,
+    key: InternPool.Key.NamespaceType.Declared,
-) Zcu.SemaError!InternPool.Index {
+) Allocator.Error!InternPool.Index {
     const zcu = pt.zcu;
     const gpa = zcu.gpa;
     const ip = &zcu.intern_pool;
-    const key = switch (full_key) {
+    const inst_info = key.zir_index.resolveFull(ip).?;
-        .reified => unreachable, // never outdated
-        .generated_tag => unreachable, // not a struct
-        .declared => |d| d,
-    };
-    const inst_info = key.zir_index.resolveFull(ip) orelse return error.AnalysisFail;
     const file = zcu.fileByIndex(inst_info.file);
-    if (file.status != .success_zir) return error.AnalysisFail;
+    assert(file.status == .success_zir); // otherwise inst tracking failed
     const zir = file.zir;
     assert(zir.instructions.items(.tag)[@intFromEnum(inst_info.inst)] == .extended);
 @@ -3600,7 +3613,7 @@ fn recreateStructType(
         break :blk fields_len;
     } else 0;
-    if (captures_len != key.captures.owned.len) return error.AnalysisFail;
+    assert(captures_len == key.captures.owned.len); // synchronises with logic in `Zcu.mapOldZirToNew`
     const struct_obj = ip.loadStructType(old_ty);
 @@ -3644,21 +3657,15 @@ fn recreateStructType(
 fn recreateUnionType(
     pt: Zcu.PerThread,
     old_ty: InternPool.Index,
-    full_key: InternPool.Key.NamespaceType,
+    key: InternPool.Key.NamespaceType.Declared,
-) Zcu.SemaError!InternPool.Index {
+) Allocator.Error!InternPool.Index {
     const zcu = pt.zcu;
     const gpa = zcu.gpa;
     const ip = &zcu.intern_pool;
-    const key = switch (full_key) {
+    const inst_info = key.zir_index.resolveFull(ip).?;
-        .reified => unreachable, // never outdated
-        .generated_tag => unreachable, // not a union
-        .declared => |d| d,
-    };
-    const inst_info = key.zir_index.resolveFull(ip) orelse return error.AnalysisFail;
     const file = zcu.fileByIndex(inst_info.file);
-    if (file.status != .success_zir) return error.AnalysisFail;
+    assert(file.status == .success_zir); // otherwise inst tracking failed
     const zir = file.zir;
     assert(zir.instructions.items(.tag)[@intFromEnum(inst_info.inst)] == .extended);
 @@ -3681,7 +3688,7 @@ fn recreateUnionType(
         break :blk fields_len;
     } else 0;
-    if (captures_len != key.captures.owned.len) return error.AnalysisFail;
+    assert(captures_len == key.captures.owned.len); // synchronises with logic in `Zcu.mapOldZirToNew`
     const union_obj = ip.loadUnionType(old_ty);
 @@ -3731,23 +3738,19 @@ fn recreateUnionType(
     return wip_ty.finish(ip, namespace_index);
 }
+// TODO: is it safe for this to return `SemaError`? enum type resolution is a bit weird...
 fn recreateEnumType(
     pt: Zcu.PerThread,
     old_ty: InternPool.Index,
-    full_key: InternPool.Key.NamespaceType,
+    key: InternPool.Key.NamespaceType.Declared,
 ) Zcu.SemaError!InternPool.Index {
     const zcu = pt.zcu;
     const gpa = zcu.gpa;
     const ip = &zcu.intern_pool;
-    const key = switch (full_key) {
+    const inst_info = key.zir_index.resolveFull(ip).?;
-        .reified, .generated_tag => unreachable, // never outdated
-        .declared => |d| d,
-    };
-    const inst_info = key.zir_index.resolveFull(ip) orelse return error.AnalysisFail;
     const file = zcu.fileByIndex(inst_info.file);
-    if (file.status != .success_zir) return error.AnalysisFail;
+    assert(file.status == .success_zir); // otherwise inst tracking failed
     const zir = file.zir;
     assert(zir.instructions.items(.tag)[@intFromEnum(inst_info.inst)] == .extended);
 @@ -3787,7 +3790,7 @@ fn recreateEnumType(
         break :blk decls_len;
     } else 0;
-    if (captures_len != key.captures.owned.len) return error.AnalysisFail;
+    assert(captures_len == key.captures.owned.len); // synchronises with logic in `Zcu.mapOldZirToNew`
     extra_index += captures_len;
     extra_index += decls_len;