| author |  Matthew Lugg <mlugg@mlugg.co.uk> 2025-01-01 13:30:27+00:00 |
| committer | Matthew Lugg <mlugg@mlugg.co.uk> 2025-01-01 15:49:37+00:00 |
| log | f0d5e0df4d95c06287e0fe3fe48b69daf5306c8f |
| tree | b4f177522ebba9e27ce9745255f321d194de6e82 |
| parent | ba78d79228e6b62a3b5fe9d1fafac18da252e4d6 |
| signature |  Commit is signed but in an unrecognized format. |
incremental: fix errors not being deleted upon re-analysis
Previously, logic in `Compilation.getAllErrorsAlloc` was corrupting the
`failed_analysis` hashmap. This meant that on updates after the initial
update, attempts to remove entries from this map (because the `AnalUnit`
in question is being re-analyzed) silently failed. This resulted in
compile errors from earlier updates wrongly getting "stuck", i.e. never
being removed.
This commit also adds a few log calls which helped me to find this bug.3 files changed, 83 insertions(+), 15 deletions(-)
test- incremental
src/Compilation.zig+25-9
| ... | @@ -3102,9 +3102,10 @@ pub fn getAllErrorsAlloc(comp: *Compilation) !ErrorBundle { | ... | @@ -3102,9 +3102,10 @@ pub fn getAllErrorsAlloc(comp: *Compilation) !ErrorBundle { |
| 3102 | for (zcu.failed_embed_files.values()) |error_msg| { | 3102 | for (zcu.failed_embed_files.values()) |error_msg| { |
| 3103 | try addModuleErrorMsg(zcu, &bundle, error_msg.*); | 3103 | try addModuleErrorMsg(zcu, &bundle, error_msg.*); |
| 3104 | } | 3104 | } |
| 3105 | { | 3105 | var sorted_failed_analysis: std.AutoArrayHashMapUnmanaged(InternPool.AnalUnit, *Zcu.ErrorMsg).DataList.Slice = s: { |
| 3106 | const SortOrder = struct { | 3106 | const SortOrder = struct { |
| 3107 | zcu: *Zcu, | 3107 | zcu: *Zcu, |
| 3108 | errors: []const *Zcu.ErrorMsg, | ||
| 3108 | err: *?Error, | 3109 | err: *?Error, |
| 3109 | 3110 | ||
| 3110 | const Error = @typeInfo( | 3111 | const Error = @typeInfo( |
| ... | @@ -3113,12 +3114,11 @@ pub fn getAllErrorsAlloc(comp: *Compilation) !ErrorBundle { | ... | @@ -3113,12 +3114,11 @@ pub fn getAllErrorsAlloc(comp: *Compilation) !ErrorBundle { |
| 3113 | 3114 | ||
| 3114 | pub fn lessThan(ctx: @This(), lhs_index: usize, rhs_index: usize) bool { | 3115 | pub fn lessThan(ctx: @This(), lhs_index: usize, rhs_index: usize) bool { |
| 3115 | if (ctx.err.*) |_| return lhs_index < rhs_index; | 3116 | if (ctx.err.*) |_| return lhs_index < rhs_index; |
| 3116 | const errors = ctx.zcu.failed_analysis.values(); | 3117 | const lhs_src_loc = ctx.errors[lhs_index].src_loc.upgradeOrLost(ctx.zcu) orelse { |
| 3117 | const lhs_src_loc = errors[lhs_index].src_loc.upgradeOrLost(ctx.zcu) orelse { | ||
| 3118 | // LHS source location lost, so should never be referenced. Just sort it to the end. | 3118 | // LHS source location lost, so should never be referenced. Just sort it to the end. |
| 3119 | return false; | 3119 | return false; |
| 3120 | }; | 3120 | }; |
| 3121 | const rhs_src_loc = errors[rhs_index].src_loc.upgradeOrLost(ctx.zcu) orelse { | 3121 | const rhs_src_loc = ctx.errors[rhs_index].src_loc.upgradeOrLost(ctx.zcu) orelse { |
| 3122 | // RHS source location lost, so should never be referenced. Just sort it to the end. | 3122 | // RHS source location lost, so should never be referenced. Just sort it to the end. |
| 3123 | return true; | 3123 | return true; |
| 3124 | }; | 3124 | }; |
| ... | @@ -3135,13 +3135,24 @@ pub fn getAllErrorsAlloc(comp: *Compilation) !ErrorBundle { | ... | @@ -3135,13 +3135,24 @@ pub fn getAllErrorsAlloc(comp: *Compilation) !ErrorBundle { |
| 3135 | }).main; | 3135 | }).main; |
| 3136 | } | 3136 | } |
| 3137 | }; | 3137 | }; |
| 3138 | |||
| 3139 | // We can't directly sort `zcu.failed_analysis.entries`, because that would leave the map | ||
| 3140 | // in an invalid state, and we need it intact for future incremental updates. The amount | ||
| 3141 | // of data here is only as large as the number of analysis errors, so just dupe it all. | ||
| 3142 | var entries = try zcu.failed_analysis.entries.clone(gpa); | ||
| 3143 | errdefer entries.deinit(gpa); | ||
| 3144 | |||
| 3138 | var err: ?SortOrder.Error = null; | 3145 | var err: ?SortOrder.Error = null; |
| 3139 | // This leaves `zcu.failed_analysis` an invalid state, but we do not | 3146 | entries.sort(SortOrder{ |
| 3140 | // need lookups anymore anyway. | 3147 | .zcu = zcu, |
| 3141 | zcu.failed_analysis.entries.sort(SortOrder{ .zcu = zcu, .err = &err }); | 3148 | .errors = entries.items(.value), |
| 3149 | .err = &err, | ||
| 3150 | }); | ||
| 3142 | if (err) |e| return e; | 3151 | if (err) |e| return e; |
| 3143 | } | 3152 | break :s entries.slice(); |
| 3144 | for (zcu.failed_analysis.keys(), zcu.failed_analysis.values()) |anal_unit, error_msg| { | 3153 | }; |
| 3154 | defer sorted_failed_analysis.deinit(gpa); | ||
| 3155 | for (sorted_failed_analysis.items(.key), sorted_failed_analysis.items(.value)) |anal_unit, error_msg| { | ||
| 3145 | if (comp.incremental) { | 3156 | if (comp.incremental) { |
| 3146 | const refs = try zcu.resolveReferences(); | 3157 | const refs = try zcu.resolveReferences(); |
| 3147 | if (!refs.contains(anal_unit)) continue; | 3158 | if (!refs.contains(anal_unit)) continue; |
| ... | @@ -3158,6 +3169,11 @@ pub fn getAllErrorsAlloc(comp: *Compilation) !ErrorBundle { | ... | @@ -3158,6 +3169,11 @@ pub fn getAllErrorsAlloc(comp: *Compilation) !ErrorBundle { |
| 3158 | // We'll try again once parsing succeeds. | 3169 | // We'll try again once parsing succeeds. |
| 3159 | if (!zcu.fileByIndex(file_index).okToReportErrors()) continue; | 3170 | if (!zcu.fileByIndex(file_index).okToReportErrors()) continue; |
| 3160 | 3171 | ||
| 3172 | std.log.scoped(.zcu).debug("analysis error '{s}' reported from unit '{}'", .{ | ||
| 3173 | error_msg.msg, | ||
| 3174 | zcu.fmtAnalUnit(anal_unit), | ||
| 3175 | }); | ||
| 3176 | |||
| 3161 | try addModuleErrorMsg(zcu, &bundle, error_msg.*); | 3177 | try addModuleErrorMsg(zcu, &bundle, error_msg.*); |
| 3162 | if (zcu.cimport_errors.get(anal_unit)) |errors| { | 3178 | if (zcu.cimport_errors.get(anal_unit)) |errors| { |
| 3163 | for (errors.getMessages()) |err_msg_index| { | 3179 | for (errors.getMessages()) |err_msg_index| { |
src/Zcu.zig+6-6
| ... | @@ -3590,17 +3590,17 @@ fn formatAnalUnit(data: struct { unit: AnalUnit, zcu: *Zcu }, comptime fmt: []co | ... | @@ -3590,17 +3590,17 @@ fn formatAnalUnit(data: struct { unit: AnalUnit, zcu: *Zcu }, comptime fmt: []co |
| 3590 | const cu = ip.getComptimeUnit(cu_id); | 3590 | const cu = ip.getComptimeUnit(cu_id); |
| 3591 | if (cu.zir_index.resolveFull(ip)) |resolved| { | 3591 | if (cu.zir_index.resolveFull(ip)) |resolved| { |
| 3592 | const file_path = zcu.fileByIndex(resolved.file).sub_file_path; | 3592 | const file_path = zcu.fileByIndex(resolved.file).sub_file_path; |
| 3593 | return writer.print("comptime(inst=('{s}', %{}))", .{ file_path, @intFromEnum(resolved.inst) }); | 3593 | return writer.print("comptime(inst=('{s}', %{}) [{}])", .{ file_path, @intFromEnum(resolved.inst), @intFromEnum(cu_id) }); |
| 3594 | } else { | 3594 | } else { |
| 3595 | return writer.writeAll("comptime(inst=<list>)"); | 3595 | return writer.print("comptime(inst=<lost> [{}])", .{@intFromEnum(cu_id)}); |
| 3596 | } | 3596 | } |
| 3597 | }, | 3597 | }, |
| 3598 | .nav_val => |nav| return writer.print("nav_val('{}')", .{ip.getNav(nav).fqn.fmt(ip)}), | 3598 | .nav_val => |nav| return writer.print("nav_val('{}' [{}])", .{ ip.getNav(nav).fqn.fmt(ip), @intFromEnum(nav) }), |
| 3599 | .nav_ty => |nav| return writer.print("nav_ty('{}')", .{ip.getNav(nav).fqn.fmt(ip)}), | 3599 | .nav_ty => |nav| return writer.print("nav_ty('{}' [{}])", .{ ip.getNav(nav).fqn.fmt(ip), @intFromEnum(nav) }), |
| 3600 | .type => |ty| return writer.print("ty('{}')", .{Type.fromInterned(ty).containerTypeName(ip).fmt(ip)}), | 3600 | .type => |ty| return writer.print("ty('{}' [{}])", .{ Type.fromInterned(ty).containerTypeName(ip).fmt(ip), @intFromEnum(ty) }), |
| 3601 | .func => |func| { | 3601 | .func => |func| { |
| 3602 | const nav = zcu.funcInfo(func).owner_nav; | 3602 | const nav = zcu.funcInfo(func).owner_nav; |
| 3603 | return writer.print("func('{}')", .{ip.getNav(nav).fqn.fmt(ip)}); | 3603 | return writer.print("func('{}' [{}])", .{ ip.getNav(nav).fqn.fmt(ip), @intFromEnum(func) }); |
| 3604 | }, | 3604 | }, |
| 3605 | } | 3605 | } |
| 3606 | } | 3606 | } |
test/incremental/fix_many_errors created+52
| ... | @@ -0,0 +1,52 @@ | ||
| 1 | #target=x86_64-linux-selfhosted | ||
| 2 | #target=x86_64-linux-cbe | ||
| 3 | #target=x86_64-windows-cbe | ||
| 4 | #update=initial version | ||
| 5 | #file=main.zig | ||
| 6 | pub fn main() !void {} | ||
| 7 | comptime { @compileError("c0"); } | ||
| 8 | comptime { @compileError("c1"); } | ||
| 9 | comptime { @compileError("c2"); } | ||
| 10 | comptime { @compileError("c3"); } | ||
| 11 | comptime { @compileError("c4"); } | ||
| 12 | comptime { @compileError("c5"); } | ||
| 13 | comptime { @compileError("c6"); } | ||
| 14 | comptime { @compileError("c7"); } | ||
| 15 | comptime { @compileError("c8"); } | ||
| 16 | comptime { @compileError("c9"); } | ||
| 17 | export fn f0() void { @compileError("f0"); } | ||
| 18 | export fn f1() void { @compileError("f1"); } | ||
| 19 | export fn f2() void { @compileError("f2"); } | ||
| 20 | export fn f3() void { @compileError("f3"); } | ||
| 21 | export fn f4() void { @compileError("f4"); } | ||
| 22 | export fn f5() void { @compileError("f5"); } | ||
| 23 | export fn f6() void { @compileError("f6"); } | ||
| 24 | export fn f7() void { @compileError("f7"); } | ||
| 25 | export fn f8() void { @compileError("f8"); } | ||
| 26 | export fn f9() void { @compileError("f9"); } | ||
| 27 | #expect_error=ignored | ||
| 28 | #update=fix all the errors | ||
| 29 | #file=main.zig | ||
| 30 | pub fn main() !void {} | ||
| 31 | comptime {} | ||
| 32 | comptime {} | ||
| 33 | comptime {} | ||
| 34 | comptime {} | ||
| 35 | comptime {} | ||
| 36 | comptime {} | ||
| 37 | comptime {} | ||
| 38 | comptime {} | ||
| 39 | comptime {} | ||
| 40 | comptime {} | ||
| 41 | export fn f0() void {} | ||
| 42 | export fn f1() void {} | ||
| 43 | export fn f2() void {} | ||
| 44 | export fn f3() void {} | ||
| 45 | export fn f4() void {} | ||
| 46 | export fn f5() void {} | ||
| 47 | export fn f6() void {} | ||
| 48 | export fn f7() void {} | ||
| 49 | export fn f8() void {} | ||
| 50 | export fn f9() void {} | ||
| 51 | const std = @import("std"); | ||
| 52 | #expect_stdout="" | ||