authorgravatar for mlugg@mlugg.co.ukMatthew Lugg <mlugg@mlugg.co.uk> 2024-10-16 15:56:48+01:00
committergravatar for mlugg@mlugg.co.ukMatthew Lugg <mlugg@mlugg.co.uk> 2024-10-16 16:30:36+01:00
log22539783ad15be3028ed07983eb8e23910171f11
tree0ace6676df00397e4ca4df098013220f95e85bf7
parentc6842b58d488c236aca74dea82082eec365eb117
signaturelock-open Commit is signed but in an unrecognized format.

incremental: introduce `file` dependencies to handle AstGen failures

The re-analysis here is a little coarse; it'd be nice in the future to have a way for an AstGen failure to preserve *all* analysis which depends on the last success, and just hide the compile errors which depend on it somehow. But I'm not sure how we'd achieve that, so this works fine for now. Resolves: #21223

8 files changed, 97 insertions(+), 23 deletions(-)

src/Compilation.zig+4
......@@ -2901,6 +2901,7 @@ pub fn makeBinFileWritable(comp: *Compilation) !void {
29012901const Header = extern struct {
29022902 intern_pool: extern struct {
29032903 thread_count: u32,
2904 file_deps_len: u32,
29042905 src_hash_deps_len: u32,
29052906 nav_val_deps_len: u32,
29062907 namespace_deps_len: u32,
......@@ -2943,6 +2944,7 @@ pub fn saveState(comp: *Compilation) !void {
29432944 const header: Header = .{
29442945 .intern_pool = .{
29452946 .thread_count = @intCast(ip.locals.len),
2947 .file_deps_len = @intCast(ip.file_deps.count()),
29462948 .src_hash_deps_len = @intCast(ip.src_hash_deps.count()),
29472949 .nav_val_deps_len = @intCast(ip.nav_val_deps.count()),
29482950 .namespace_deps_len = @intCast(ip.namespace_deps.count()),
......@@ -2969,6 +2971,8 @@ pub fn saveState(comp: *Compilation) !void {
29692971 addBuf(&bufs, mem.asBytes(&header));
29702972 addBuf(&bufs, mem.sliceAsBytes(pt_headers.items));
29712973
2974 addBuf(&bufs, mem.sliceAsBytes(ip.file_deps.keys()));
2975 addBuf(&bufs, mem.sliceAsBytes(ip.file_deps.values()));
29722976 addBuf(&bufs, mem.sliceAsBytes(ip.src_hash_deps.keys()));
29732977 addBuf(&bufs, mem.sliceAsBytes(ip.src_hash_deps.values()));
29742978 addBuf(&bufs, mem.sliceAsBytes(ip.nav_val_deps.keys()));
src/InternPool.zig+12
......@@ -17,6 +17,13 @@ tid_shift_31: if (single_threaded) u0 else std.math.Log2Int(u32),
1717/// Cached shift amount to put a `tid` in the top bits of a 32-bit value.
1818tid_shift_32: if (single_threaded) u0 else std.math.Log2Int(u32),
1919
20/// Dependencies on whether an entire file gets past AstGen.
21/// These are triggered by `@import`, so that:
22/// * if a file initially fails AstGen, triggering a transitive failure, when a future update
23/// causes it to succeed AstGen, the `@import` is re-analyzed, allowing analysis to proceed
24/// * if a file initially succeds AstGen, but a future update causes the file to fail it,
25/// the `@import` is re-analyzed, registering a transitive failure
26file_deps: std.AutoArrayHashMapUnmanaged(FileIndex, DepEntry.Index),
2027/// Dependencies on the source code hash associated with a ZIR instruction.
2128/// * For a `declaration`, this is the entire declaration body.
2229/// * For a `struct_decl`, `union_decl`, etc, this is the source of the fields (but not declarations).
......@@ -70,6 +77,7 @@ pub const empty: InternPool = .{
7077 .tid_shift_30 = if (single_threaded) 0 else 31,
7178 .tid_shift_31 = if (single_threaded) 0 else 31,
7279 .tid_shift_32 = if (single_threaded) 0 else 31,
80 .file_deps = .empty,
7381 .src_hash_deps = .empty,
7482 .nav_val_deps = .empty,
7583 .interned_deps = .empty,
......@@ -656,6 +664,7 @@ pub const Nav = struct {
656664};
657665
658666pub const Dependee = union(enum) {
667 file: FileIndex,
659668 src_hash: TrackedInst.Index,
660669 nav_val: Nav.Index,
661670 interned: Index,
......@@ -704,6 +713,7 @@ pub const DependencyIterator = struct {
704713
705714pub fn dependencyIterator(ip: *const InternPool, dependee: Dependee) DependencyIterator {
706715 const first_entry = switch (dependee) {
716 .file => |x| ip.file_deps.get(x),
707717 .src_hash => |x| ip.src_hash_deps.get(x),
708718 .nav_val => |x| ip.nav_val_deps.get(x),
709719 .interned => |x| ip.interned_deps.get(x),
......@@ -740,6 +750,7 @@ pub fn addDependency(ip: *InternPool, gpa: Allocator, depender: AnalUnit, depend
740750 const new_index: DepEntry.Index = switch (dependee) {
741751 inline else => |dependee_payload, tag| new_index: {
742752 const gop = try switch (tag) {
753 .file => ip.file_deps,
743754 .src_hash => ip.src_hash_deps,
744755 .nav_val => ip.nav_val_deps,
745756 .interned => ip.interned_deps,
......@@ -6268,6 +6279,7 @@ pub fn init(ip: *InternPool, gpa: Allocator, available_threads: usize) !void {
62686279}
62696280
62706281pub fn deinit(ip: *InternPool, gpa: Allocator) void {
6282 ip.file_deps.deinit(gpa);
62716283 ip.src_hash_deps.deinit(gpa);
62726284 ip.nav_val_deps.deinit(gpa);
62736285 ip.interned_deps.deinit(gpa);
src/Package/Module.zig+1
......@@ -454,6 +454,7 @@ pub fn create(arena: Allocator, options: CreateOptions) !*Package.Module {
454454 .tree = undefined,
455455 .zir = undefined,
456456 .status = .never_loaded,
457 .prev_status = .never_loaded,
457458 .mod = new,
458459 };
459460 break :b new;
src/Sema.zig+2-6
......@@ -6024,9 +6024,7 @@ fn zirCImport(sema: *Sema, parent_block: *Block, inst: Zir.Inst.Index) CompileEr
60246024 pt.astGenFile(result.file, path_digest) catch |err|
60256025 return sema.fail(&child_block, src, "C import failed: {s}", .{@errorName(err)});
60266026
6027 // TODO: register some kind of dependency on the file.
6028 // That way, if this returns `error.AnalysisFail`, we have the dependency banked ready to
6029 // trigger re-analysis later.
6027 try sema.declareDependency(.{ .file = result.file_index });
60306028 try pt.ensureFileAnalyzed(result.file_index);
60316029 const ty = zcu.fileRootType(result.file_index);
60326030 try sema.declareDependency(.{ .interned = ty });
......@@ -14347,9 +14345,7 @@ fn zirImport(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air.
1434714345 return sema.fail(block, operand_src, "unable to open '{s}': {s}", .{ operand, @errorName(err) });
1434814346 },
1434914347 };
14350 // TODO: register some kind of dependency on the file.
14351 // That way, if this returns `error.AnalysisFail`, we have the dependency banked ready to
14352 // trigger re-analysis later.
14348 try sema.declareDependency(.{ .file = result.file_index });
1435314349 try pt.ensureFileAnalyzed(result.file_index);
1435414350 const ty = zcu.fileRootType(result.file_index);
1435514351 try sema.declareDependency(.{ .interned = ty });
src/Zcu.zig+14-7
......@@ -424,13 +424,8 @@ pub const Namespace = struct {
424424};
425425
426426pub const File = struct {
427 status: enum {
428 never_loaded,
429 retryable_failure,
430 parse_failure,
431 astgen_failure,
432 success_zir,
433 },
427 status: Status,
428 prev_status: Status,
434429 source_loaded: bool,
435430 tree_loaded: bool,
436431 zir_loaded: bool,
......@@ -458,6 +453,14 @@ pub const File = struct {
458453 /// successful, this field is unloaded.
459454 prev_zir: ?*Zir = null,
460455
456 pub const Status = enum {
457 never_loaded,
458 retryable_failure,
459 parse_failure,
460 astgen_failure,
461 success_zir,
462 };
463
461464 /// A single reference to a file.
462465 pub const Reference = union(enum) {
463466 /// The file is imported directly (i.e. not as a package) with @import.
......@@ -3474,6 +3477,10 @@ fn formatDependee(data: struct { dependee: InternPool.Dependee, zcu: *Zcu }, com
34743477 const zcu = data.zcu;
34753478 const ip = &zcu.intern_pool;
34763479 switch (data.dependee) {
3480 .file => |file| {
3481 const file_path = zcu.fileByIndex(file).sub_file_path;
3482 return writer.print("file('{s}')", .{file_path});
3483 },
34773484 .src_hash => |ti| {
34783485 const info = ti.resolveFull(ip) orelse {
34793486 return writer.writeAll("inst(<lost>)");
src/Zcu/PerThread.zig+26-10
......@@ -179,10 +179,10 @@ pub fn astGenFile(
179179 .inode = header.stat_inode,
180180 .mtime = header.stat_mtime,
181181 };
182 file.prev_status = file.status;
182183 file.status = .success_zir;
183184 log.debug("AstGen cached success: {s}", .{file.sub_file_path});
184185
185 // TODO don't report compile errors until Sema @importFile
186186 if (file.zir.hasCompileErrors()) {
187187 {
188188 comp.mutex.lock();
......@@ -258,6 +258,7 @@ pub fn astGenFile(
258258 // Any potential AST errors are converted to ZIR errors here.
259259 file.zir = try AstGen.generate(gpa, file.tree);
260260 file.zir_loaded = true;
261 file.prev_status = file.status;
261262 file.status = .success_zir;
262263 log.debug("AstGen fresh success: {s}", .{file.sub_file_path});
263264
......@@ -350,6 +351,9 @@ pub fn updateZirRefs(pt: Zcu.PerThread) Allocator.Error!void {
350351 defer cleanupUpdatedFiles(gpa, &updated_files);
351352 for (zcu.import_table.values()) |file_index| {
352353 const file = zcu.fileByIndex(file_index);
354 if (file.prev_status != file.status and file.prev_status != .never_loaded) {
355 try zcu.markDependeeOutdated(.not_marked_po, .{ .file = file_index });
356 }
353357 const old_zir = file.prev_zir orelse continue;
354358 const new_zir = file.zir;
355359 const gop = try updated_files.getOrPut(gpa, file_index);
......@@ -551,11 +555,13 @@ pub fn ensureCauAnalyzed(pt: Zcu.PerThread, cau_index: InternPool.Cau.Index) Zcu
551555 const cau_outdated = zcu.outdated.swapRemove(anal_unit) or
552556 zcu.potentially_outdated.swapRemove(anal_unit);
553557
558 const prev_failed = zcu.failed_analysis.contains(anal_unit) or zcu.transitive_failed_analysis.contains(anal_unit);
559
554560 if (cau_outdated) {
555561 _ = zcu.outdated_ready.swapRemove(anal_unit);
556562 } else {
557563 // We can trust the current information about this `Cau`.
558 if (zcu.failed_analysis.contains(anal_unit) or zcu.transitive_failed_analysis.contains(anal_unit)) {
564 if (prev_failed) {
559565 return error.AnalysisFail;
560566 }
561567 // If it wasn't failed and wasn't marked outdated, then either...
......@@ -578,9 +584,13 @@ pub fn ensureCauAnalyzed(pt: Zcu.PerThread, cau_index: InternPool.Cau.Index) Zcu
578584 // Since it does not, this must be a transitive failure.
579585 try zcu.transitive_failed_analysis.put(gpa, anal_unit, {});
580586 }
581 // We treat errors as up-to-date, since those uses would just trigger a transitive error.
582 // The exception is types, since type declarations may require re-analysis if the type, e.g. its captures, changed.
583 const outdated = cau.owner.unwrap() == .type;
587 // We consider this `Cau` to be outdated if:
588 // * Previous analysis succeeded; in this case, we need to re-analyze dependants to ensure
589 // they hit a transitive error here, rather than reporting a different error later (which
590 // may now be invalid).
591 // * The `Cau` is a type; in this case, the declaration site may require re-analysis to
592 // construct a valid type.
593 const outdated = !prev_failed or cau.owner.unwrap() == .type;
584594 break :res .{ .{
585595 .invalidate_decl_val = outdated,
586596 .invalidate_decl_ref = outdated,
......@@ -597,10 +607,9 @@ pub fn ensureCauAnalyzed(pt: Zcu.PerThread, cau_index: InternPool.Cau.Index) Zcu
597607 );
598608 zcu.retryable_failures.appendAssumeCapacity(anal_unit);
599609 zcu.failed_analysis.putAssumeCapacityNoClobber(anal_unit, msg);
600 // We treat errors as up-to-date, since those uses would just trigger a transitive error
601610 break :res .{ .{
602 .invalidate_decl_val = false,
603 .invalidate_decl_ref = false,
611 .invalidate_decl_val = true,
612 .invalidate_decl_ref = true,
604613 }, true };
605614 },
606615 };
......@@ -707,11 +716,13 @@ pub fn ensureFuncBodyAnalyzed(pt: Zcu.PerThread, maybe_coerced_func_index: Inter
707716 const func_outdated = zcu.outdated.swapRemove(anal_unit) or
708717 zcu.potentially_outdated.swapRemove(anal_unit);
709718
719 const prev_failed = zcu.failed_analysis.contains(anal_unit) or zcu.transitive_failed_analysis.contains(anal_unit);
720
710721 if (func_outdated) {
711722 _ = zcu.outdated_ready.swapRemove(anal_unit);
712723 } else {
713724 // We can trust the current information about this function.
714 if (zcu.failed_analysis.contains(anal_unit) or zcu.transitive_failed_analysis.contains(anal_unit)) {
725 if (prev_failed) {
715726 return error.AnalysisFail;
716727 }
717728 switch (func.analysisUnordered(ip).state) {
......@@ -730,7 +741,10 @@ pub fn ensureFuncBodyAnalyzed(pt: Zcu.PerThread, maybe_coerced_func_index: Inter
730741 // Since it does not, this must be a transitive failure.
731742 try zcu.transitive_failed_analysis.put(gpa, anal_unit, {});
732743 }
733 break :res .{ false, true }; // we treat errors as up-to-date IES, since those uses would just trigger a transitive error
744 // We consider the IES to be outdated if the function previously succeeded analysis; in this case,
745 // we need to re-analyze dependants to ensure they hit a transitive error here, rather than reporting
746 // a different error later (which may now be invalid).
747 break :res .{ !prev_failed, true };
734748 },
735749 error.OutOfMemory => return error.OutOfMemory, // TODO: graceful handling like `ensureCauAnalyzed`
736750 };
......@@ -1445,6 +1459,7 @@ pub fn importPkg(pt: Zcu.PerThread, mod: *Module) !Zcu.ImportFileResult {
14451459 .tree = undefined,
14461460 .zir = undefined,
14471461 .status = .never_loaded,
1462 .prev_status = .never_loaded,
14481463 .mod = mod,
14491464 };
14501465
......@@ -1555,6 +1570,7 @@ pub fn importFile(
15551570 .tree = undefined,
15561571 .zir = undefined,
15571572 .status = .never_loaded,
1573 .prev_status = .never_loaded,
15581574 .mod = mod,
15591575 };
15601576
src/main.zig+3
......@@ -6118,6 +6118,7 @@ fn cmdAstCheck(
61186118
61196119 var file: Zcu.File = .{
61206120 .status = .never_loaded,
6121 .prev_status = .never_loaded,
61216122 .source_loaded = false,
61226123 .tree_loaded = false,
61236124 .zir_loaded = false,
......@@ -6441,6 +6442,7 @@ fn cmdDumpZir(
64416442
64426443 var file: Zcu.File = .{
64436444 .status = .never_loaded,
6445 .prev_status = .never_loaded,
64446446 .source_loaded = false,
64456447 .tree_loaded = false,
64466448 .zir_loaded = true,
......@@ -6508,6 +6510,7 @@ fn cmdChangelist(
65086510
65096511 var file: Zcu.File = .{
65106512 .status = .never_loaded,
6513 .prev_status = .never_loaded,
65116514 .source_loaded = false,
65126515 .tree_loaded = false,
65136516 .zir_loaded = false,
test/incremental/fix_astgen_failure created+35
......@@ -0,0 +1,35 @@
1#target=x86_64-linux-selfhosted
2#target=x86_64-linux-cbe
3#target=x86_64-windows-cbe
4#update=initial version with error
5#file=main.zig
6pub fn main() !void {
7 try @import("foo.zig").hello();
8}
9#file=foo.zig
10pub fn hello() !void {
11 try std.io.getStdOut().writeAll("Hello, World!\n");
12}
13#expect_error=ignored
14#update=fix the error
15#file=foo.zig
16const std = @import("std");
17pub fn hello() !void {
18 try std.io.getStdOut().writeAll("Hello, World!\n");
19}
20#expect_stdout="Hello, World!\n"
21#update=add new error
22#file=foo.zig
23const std = @import("std");
24pub fn hello() !void {
25 try std.io.getStdOut().writeAll(hello_str);
26}
27#expect_error=ignored
28#update=fix the new error
29#file=foo.zig
30const std = @import("std");
31const hello_str = "Hello, World! Again!\n";
32pub fn hello() !void {
33 try std.io.getStdOut().writeAll(hello_str);
34}
35#expect_stdout="Hello, World! Again!\n"