| author |  Matthew Lugg <mlugg@mlugg.co.uk> 2024-08-13 20:20:38+01:00 |
| committer |  Jacob Young <jacobly@ziglang.org> 2024-08-17 18:50:10-04:00 |
| log | 50960fac80b1d04f7858215d963fa64a7583210b |
| tree | 5d407fb1c0bd21ddfcaddadcb835084674b0376e |
| parent | 4e5834a9f247c60fcc1d1da5f3b2c00efdb8f4e4 |
compiler: be more cautious about source locations
Two fixes here.
* Prevent a crash when sorting the list of analysis errors when some
errors refer to lost source locations. These errors can be sorted
anywhere in the list, because they are (in theory) guaranteed to never
be emitted by the `resolveReferences` logic. This case occurs, for
instance, when a declaration has compile errors in the initial update
and is deleted in the second update.
* Prevent a crash when resolving the source location for `entire_file`
errors for a non-existent file. This is the bug underlying #20954.
Resolves: #20954.4 files changed, 36 insertions(+), 9 deletions(-)
src/Compilation.zig+8-2
| ... | @@ -3203,8 +3203,14 @@ pub fn getAllErrorsAlloc(comp: *Compilation) !ErrorBundle { | ... | @@ -3203,8 +3203,14 @@ pub fn getAllErrorsAlloc(comp: *Compilation) !ErrorBundle { |
| 3203 | pub fn lessThan(ctx: @This(), lhs_index: usize, rhs_index: usize) bool { | 3203 | pub fn lessThan(ctx: @This(), lhs_index: usize, rhs_index: usize) bool { |
| 3204 | if (ctx.err.*) |_| return lhs_index < rhs_index; | 3204 | if (ctx.err.*) |_| return lhs_index < rhs_index; |
| 3205 | const errors = ctx.zcu.failed_analysis.values(); | 3205 | const errors = ctx.zcu.failed_analysis.values(); |
| 3206 | const lhs_src_loc = errors[lhs_index].src_loc.upgrade(ctx.zcu); | 3206 | const lhs_src_loc = errors[lhs_index].src_loc.upgradeOrLost(ctx.zcu) orelse { |
| 3207 | const rhs_src_loc = errors[rhs_index].src_loc.upgrade(ctx.zcu); | 3207 | // LHS source location lost, so should never be referenced. Just sort it to the end. |
| 3208 | return false; | ||
| 3209 | }; | ||
| 3210 | const rhs_src_loc = errors[rhs_index].src_loc.upgradeOrLost(ctx.zcu) orelse { | ||
| 3211 | // RHS source location lost, so should never be referenced. Just sort it to the end. | ||
| 3212 | return true; | ||
| 3213 | }; | ||
| 3208 | return if (lhs_src_loc.file_scope != rhs_src_loc.file_scope) std.mem.order( | 3214 | return if (lhs_src_loc.file_scope != rhs_src_loc.file_scope) std.mem.order( |
| 3209 | u8, | 3215 | u8, |
| 3210 | lhs_src_loc.file_scope.sub_file_path, | 3216 | lhs_src_loc.file_scope.sub_file_path, |
src/Sema.zig+2-2
| ... | @@ -17729,7 +17729,7 @@ fn zirClosureGet(sema: *Sema, block: *Block, extended: Zir.Inst.Extended.InstDat | ... | @@ -17729,7 +17729,7 @@ fn zirClosureGet(sema: *Sema, block: *Block, extended: Zir.Inst.Extended.InstDat |
| 17729 | const msg = msg: { | 17729 | const msg = msg: { |
| 17730 | const name = name: { | 17730 | const name = name: { |
| 17731 | // TODO: we should probably store this name in the ZIR to avoid this complexity. | 17731 | // TODO: we should probably store this name in the ZIR to avoid this complexity. |
| 17732 | const file, const src_base_node = Module.LazySrcLoc.resolveBaseNode(block.src_base_inst, mod); | 17732 | const file, const src_base_node = Module.LazySrcLoc.resolveBaseNode(block.src_base_inst, mod).?; |
| 17733 | const tree = file.getTree(sema.gpa) catch |err| { | 17733 | const tree = file.getTree(sema.gpa) catch |err| { |
| 17734 | // In this case we emit a warning + a less precise source location. | 17734 | // In this case we emit a warning + a less precise source location. |
| 17735 | log.warn("unable to load {s}: {s}", .{ | 17735 | log.warn("unable to load {s}: {s}", .{ |
| ... | @@ -17757,7 +17757,7 @@ fn zirClosureGet(sema: *Sema, block: *Block, extended: Zir.Inst.Extended.InstDat | ... | @@ -17757,7 +17757,7 @@ fn zirClosureGet(sema: *Sema, block: *Block, extended: Zir.Inst.Extended.InstDat |
| 17757 | if (!block.is_typeof and !block.is_comptime and sema.func_index != .none) { | 17757 | if (!block.is_typeof and !block.is_comptime and sema.func_index != .none) { |
| 17758 | const msg = msg: { | 17758 | const msg = msg: { |
| 17759 | const name = name: { | 17759 | const name = name: { |
| 17760 | const file, const src_base_node = Module.LazySrcLoc.resolveBaseNode(block.src_base_inst, mod); | 17760 | const file, const src_base_node = Module.LazySrcLoc.resolveBaseNode(block.src_base_inst, mod).?; |
| 17761 | const tree = file.getTree(sema.gpa) catch |err| { | 17761 | const tree = file.getTree(sema.gpa) catch |err| { |
| 17762 | // In this case we emit a warning + a less precise source location. | 17762 | // In this case we emit a warning + a less precise source location. |
| 17763 | log.warn("unable to load {s}: {s}", .{ | 17763 | log.warn("unable to load {s}: {s}", .{ |
src/Zcu.zig+12-3
| ... | @@ -2042,10 +2042,11 @@ pub const LazySrcLoc = struct { | ... | @@ -2042,10 +2042,11 @@ pub const LazySrcLoc = struct { |
| 2042 | .offset = .unneeded, | 2042 | .offset = .unneeded, |
| 2043 | }; | 2043 | }; |
| 2044 | 2044 | ||
| 2045 | pub fn resolveBaseNode(base_node_inst: InternPool.TrackedInst.Index, zcu: *Zcu) struct { *File, Ast.Node.Index } { | 2045 | /// Returns `null` if the ZIR instruction has been lost across incremental updates. |
| 2046 | pub fn resolveBaseNode(base_node_inst: InternPool.TrackedInst.Index, zcu: *Zcu) ?struct { *File, Ast.Node.Index } { | ||
| 2046 | const ip = &zcu.intern_pool; | 2047 | const ip = &zcu.intern_pool; |
| 2047 | const file_index, const zir_inst = inst: { | 2048 | const file_index, const zir_inst = inst: { |
| 2048 | const info = base_node_inst.resolveFull(ip) orelse @panic("TODO: resolve source location relative to lost inst"); | 2049 | const info = base_node_inst.resolveFull(ip) orelse return null; |
| 2049 | break :inst .{ info.file, info.inst }; | 2050 | break :inst .{ info.file, info.inst }; |
| 2050 | }; | 2051 | }; |
| 2051 | const file = zcu.fileByIndex(file_index); | 2052 | const file = zcu.fileByIndex(file_index); |
| ... | @@ -2071,7 +2072,15 @@ pub const LazySrcLoc = struct { | ... | @@ -2071,7 +2072,15 @@ pub const LazySrcLoc = struct { |
| 2071 | /// Resolve the file and AST node of `base_node_inst` to get a resolved `SrcLoc`. | 2072 | /// Resolve the file and AST node of `base_node_inst` to get a resolved `SrcLoc`. |
| 2072 | /// The resulting `SrcLoc` should only be used ephemerally, as it is not correct across incremental updates. | 2073 | /// The resulting `SrcLoc` should only be used ephemerally, as it is not correct across incremental updates. |
| 2073 | pub fn upgrade(lazy: LazySrcLoc, zcu: *Zcu) SrcLoc { | 2074 | pub fn upgrade(lazy: LazySrcLoc, zcu: *Zcu) SrcLoc { |
| 2074 | const file, const base_node = resolveBaseNode(lazy.base_node_inst, zcu); | 2075 | return lazy.upgradeOrLost(zcu).?; |
| 2076 | } | ||
| 2077 | |||
| 2078 | /// Like `upgrade`, but returns `null` if the source location has been lost across incremental updates. | ||
| 2079 | pub fn upgradeOrLost(lazy: LazySrcLoc, zcu: *Zcu) ?SrcLoc { | ||
| 2080 | const file, const base_node: Ast.Node.Index = if (lazy.offset == .entire_file) .{ | ||
| 2081 | zcu.fileByIndex(lazy.base_node_inst.resolveFile(&zcu.intern_pool)), | ||
| 2082 | 0, | ||
| 2083 | } else resolveBaseNode(lazy.base_node_inst, zcu) orelse return null; | ||
| 2075 | return .{ | 2084 | return .{ |
| 2076 | .file_scope = file, | 2085 | .file_scope = file, |
| 2077 | .base_node = base_node, | 2086 | .base_node = base_node, |
src/crash_report.zig+14-2
| ... | @@ -78,7 +78,13 @@ fn dumpStatusReport() !void { | ... | @@ -78,7 +78,13 @@ fn dumpStatusReport() !void { |
| 78 | const block: *Sema.Block = anal.block; | 78 | const block: *Sema.Block = anal.block; |
| 79 | const zcu = anal.sema.pt.zcu; | 79 | const zcu = anal.sema.pt.zcu; |
| 80 | 80 | ||
| 81 | const file, const src_base_node = Zcu.LazySrcLoc.resolveBaseNode(block.src_base_inst, zcu); | 81 | const file, const src_base_node = Zcu.LazySrcLoc.resolveBaseNode(block.src_base_inst, zcu) orelse { |
| 82 | const file = zcu.fileByIndex(block.src_base_inst.resolveFile(&zcu.intern_pool)); | ||
| 83 | try stderr.writeAll("Analyzing lost instruction in file '"); | ||
| 84 | try writeFilePath(file, stderr); | ||
| 85 | try stderr.writeAll("'. This should not happen!\n\n"); | ||
| 86 | return; | ||
| 87 | }; | ||
| 82 | 88 | ||
| 83 | try stderr.writeAll("Analyzing "); | 89 | try stderr.writeAll("Analyzing "); |
| 84 | try writeFilePath(file, stderr); | 90 | try writeFilePath(file, stderr); |
| ... | @@ -104,7 +110,13 @@ fn dumpStatusReport() !void { | ... | @@ -104,7 +110,13 @@ fn dumpStatusReport() !void { |
| 104 | while (parent) |curr| { | 110 | while (parent) |curr| { |
| 105 | fba.reset(); | 111 | fba.reset(); |
| 106 | try stderr.writeAll(" in "); | 112 | try stderr.writeAll(" in "); |
| 107 | const cur_block_file, const cur_block_src_base_node = Zcu.LazySrcLoc.resolveBaseNode(curr.block.src_base_inst, zcu); | 113 | const cur_block_file, const cur_block_src_base_node = Zcu.LazySrcLoc.resolveBaseNode(curr.block.src_base_inst, zcu) orelse { |
| 114 | const cur_block_file = zcu.fileByIndex(curr.block.src_base_inst.resolveFile(&zcu.intern_pool)); | ||
| 115 | try writeFilePath(cur_block_file, stderr); | ||
| 116 | try stderr.writeAll("\n > [lost instruction; this should not happen]\n"); | ||
| 117 | parent = curr.parent; | ||
| 118 | continue; | ||
| 119 | }; | ||
| 108 | try writeFilePath(cur_block_file, stderr); | 120 | try writeFilePath(cur_block_file, stderr); |
| 109 | try stderr.writeAll("\n > "); | 121 | try stderr.writeAll("\n > "); |
| 110 | print_zir.renderSingleInstruction( | 122 | print_zir.renderSingleInstruction( |