| author |  Andrew Kelley <andrew@ziglang.org> 2021-11-24 19:09:45-07:00 |
| committer | Andrew Kelley <andrew@ziglang.org> 2021-11-24 19:10:58-07:00 |
| log | e266ede6e310b64adcf3912b8ef2b9e2397fde7b |
| tree | 45dc81753836fccb9343b8031214dea137bbe63a |
| parent | 0581756453a98d4e1100c684896783606dc86374 |
stage2: fix cleanup code for `@import` errors
When adding test coverage, I noticed an inconsistency in which source
location the compile error was pointing to for `@embedFile` errors vs
`@import` errors. They now both point to the same place, the string
operand.
closes #9404
closes #99393 files changed, 21 insertions(+), 11 deletions(-)
src/Module.zig+5-3
| ... | @@ -3541,11 +3541,11 @@ pub fn importPkg(mod: *Module, pkg: *Package) !ImportFileResult { | ... | @@ -3541,11 +3541,11 @@ pub fn importPkg(mod: *Module, pkg: *Package) !ImportFileResult { |
| 3541 | defer if (!keep_resolved_path) gpa.free(resolved_path); | 3541 | defer if (!keep_resolved_path) gpa.free(resolved_path); |
| 3542 | 3542 | ||
| 3543 | const gop = try mod.import_table.getOrPut(gpa, resolved_path); | 3543 | const gop = try mod.import_table.getOrPut(gpa, resolved_path); |
| 3544 | errdefer _ = mod.import_table.pop(); | ||
| 3544 | if (gop.found_existing) return ImportFileResult{ | 3545 | if (gop.found_existing) return ImportFileResult{ |
| 3545 | .file = gop.value_ptr.*, | 3546 | .file = gop.value_ptr.*, |
| 3546 | .is_new = false, | 3547 | .is_new = false, |
| 3547 | }; | 3548 | }; |
| 3548 | keep_resolved_path = true; // It's now owned by import_table. | ||
| 3549 | 3549 | ||
| 3550 | const sub_file_path = try gpa.dupe(u8, pkg.root_src_path); | 3550 | const sub_file_path = try gpa.dupe(u8, pkg.root_src_path); |
| 3551 | errdefer gpa.free(sub_file_path); | 3551 | errdefer gpa.free(sub_file_path); |
| ... | @@ -3553,6 +3553,7 @@ pub fn importPkg(mod: *Module, pkg: *Package) !ImportFileResult { | ... | @@ -3553,6 +3553,7 @@ pub fn importPkg(mod: *Module, pkg: *Package) !ImportFileResult { |
| 3553 | const new_file = try gpa.create(File); | 3553 | const new_file = try gpa.create(File); |
| 3554 | errdefer gpa.destroy(new_file); | 3554 | errdefer gpa.destroy(new_file); |
| 3555 | 3555 | ||
| 3556 | keep_resolved_path = true; // It's now owned by import_table. | ||
| 3556 | gop.value_ptr.* = new_file; | 3557 | gop.value_ptr.* = new_file; |
| 3557 | new_file.* = .{ | 3558 | new_file.* = .{ |
| 3558 | .sub_file_path = sub_file_path, | 3559 | .sub_file_path = sub_file_path, |
| ... | @@ -3599,11 +3600,11 @@ pub fn importFile( | ... | @@ -3599,11 +3600,11 @@ pub fn importFile( |
| 3599 | defer if (!keep_resolved_path) gpa.free(resolved_path); | 3600 | defer if (!keep_resolved_path) gpa.free(resolved_path); |
| 3600 | 3601 | ||
| 3601 | const gop = try mod.import_table.getOrPut(gpa, resolved_path); | 3602 | const gop = try mod.import_table.getOrPut(gpa, resolved_path); |
| 3603 | errdefer _ = mod.import_table.pop(); | ||
| 3602 | if (gop.found_existing) return ImportFileResult{ | 3604 | if (gop.found_existing) return ImportFileResult{ |
| 3603 | .file = gop.value_ptr.*, | 3605 | .file = gop.value_ptr.*, |
| 3604 | .is_new = false, | 3606 | .is_new = false, |
| 3605 | }; | 3607 | }; |
| 3606 | keep_resolved_path = true; // It's now owned by import_table. | ||
| 3607 | 3608 | ||
| 3608 | const new_file = try gpa.create(File); | 3609 | const new_file = try gpa.create(File); |
| 3609 | errdefer gpa.destroy(new_file); | 3610 | errdefer gpa.destroy(new_file); |
| ... | @@ -3622,6 +3623,7 @@ pub fn importFile( | ... | @@ -3622,6 +3623,7 @@ pub fn importFile( |
| 3622 | resolved_root_path, resolved_path, sub_file_path, import_string, | 3623 | resolved_root_path, resolved_path, sub_file_path, import_string, |
| 3623 | }); | 3624 | }); |
| 3624 | 3625 | ||
| 3626 | keep_resolved_path = true; // It's now owned by import_table. | ||
| 3625 | gop.value_ptr.* = new_file; | 3627 | gop.value_ptr.* = new_file; |
| 3626 | new_file.* = .{ | 3628 | new_file.* = .{ |
| 3627 | .sub_file_path = sub_file_path, | 3629 | .sub_file_path = sub_file_path, |
| ... | @@ -3657,8 +3659,8 @@ pub fn embedFile(mod: *Module, cur_file: *File, rel_file_path: []const u8) !*Emb | ... | @@ -3657,8 +3659,8 @@ pub fn embedFile(mod: *Module, cur_file: *File, rel_file_path: []const u8) !*Emb |
| 3657 | defer if (!keep_resolved_path) gpa.free(resolved_path); | 3659 | defer if (!keep_resolved_path) gpa.free(resolved_path); |
| 3658 | 3660 | ||
| 3659 | const gop = try mod.embed_table.getOrPut(gpa, resolved_path); | 3661 | const gop = try mod.embed_table.getOrPut(gpa, resolved_path); |
| 3660 | if (gop.found_existing) return gop.value_ptr.*; | ||
| 3661 | errdefer assert(mod.embed_table.remove(resolved_path)); | 3662 | errdefer assert(mod.embed_table.remove(resolved_path)); |
| 3663 | if (gop.found_existing) return gop.value_ptr.*; | ||
| 3662 | 3664 | ||
| 3663 | const new_file = try gpa.create(EmbedFile); | 3665 | const new_file = try gpa.create(EmbedFile); |
| 3664 | errdefer gpa.destroy(new_file); | 3666 | errdefer gpa.destroy(new_file); |
src/Sema.zig+7-7
| ... | @@ -6807,17 +6807,17 @@ fn zirImport(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air. | ... | @@ -6807,17 +6807,17 @@ fn zirImport(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air. |
| 6807 | 6807 | ||
| 6808 | const mod = sema.mod; | 6808 | const mod = sema.mod; |
| 6809 | const inst_data = sema.code.instructions.items(.data)[inst].str_tok; | 6809 | const inst_data = sema.code.instructions.items(.data)[inst].str_tok; |
| 6810 | const src = inst_data.src(); | 6810 | const operand_src = inst_data.src(); |
| 6811 | const operand = inst_data.get(sema.code); | 6811 | const operand = inst_data.get(sema.code); |
| 6812 | 6812 | ||
| 6813 | const result = mod.importFile(block.getFileScope(), operand) catch |err| switch (err) { | 6813 | const result = mod.importFile(block.getFileScope(), operand) catch |err| switch (err) { |
| 6814 | error.ImportOutsidePkgPath => { | 6814 | error.ImportOutsidePkgPath => { |
| 6815 | return sema.fail(block, src, "import of file outside package path: '{s}'", .{operand}); | 6815 | return sema.fail(block, operand_src, "import of file outside package path: '{s}'", .{operand}); |
| 6816 | }, | 6816 | }, |
| 6817 | else => { | 6817 | else => { |
| 6818 | // TODO: these errors are file system errors; make sure an update() will | 6818 | // TODO: these errors are file system errors; make sure an update() will |
| 6819 | // retry this and not cache the file system error, which may be transient. | 6819 | // retry this and not cache the file system error, which may be transient. |
| 6820 | return sema.fail(block, src, "unable to open '{s}': {s}", .{ operand, @errorName(err) }); | 6820 | return sema.fail(block, operand_src, "unable to open '{s}': {s}", .{ operand, @errorName(err) }); |
| 6821 | }, | 6821 | }, |
| 6822 | }; | 6822 | }; |
| 6823 | try mod.semaFile(result.file); | 6823 | try mod.semaFile(result.file); |
| ... | @@ -6832,17 +6832,17 @@ fn zirEmbedFile(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!A | ... | @@ -6832,17 +6832,17 @@ fn zirEmbedFile(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!A |
| 6832 | 6832 | ||
| 6833 | const mod = sema.mod; | 6833 | const mod = sema.mod; |
| 6834 | const inst_data = sema.code.instructions.items(.data)[inst].un_node; | 6834 | const inst_data = sema.code.instructions.items(.data)[inst].un_node; |
| 6835 | const src = inst_data.src(); | 6835 | const operand_src: LazySrcLoc = .{ .node_offset_builtin_call_arg0 = inst_data.src_node }; |
| 6836 | const name = try sema.resolveConstString(block, src, inst_data.operand); | 6836 | const name = try sema.resolveConstString(block, operand_src, inst_data.operand); |
| 6837 | 6837 | ||
| 6838 | const embed_file = mod.embedFile(block.getFileScope(), name) catch |err| switch (err) { | 6838 | const embed_file = mod.embedFile(block.getFileScope(), name) catch |err| switch (err) { |
| 6839 | error.ImportOutsidePkgPath => { | 6839 | error.ImportOutsidePkgPath => { |
| 6840 | return sema.fail(block, src, "embed of file outside package path: '{s}'", .{name}); | 6840 | return sema.fail(block, operand_src, "embed of file outside package path: '{s}'", .{name}); |
| 6841 | }, | 6841 | }, |
| 6842 | else => { | 6842 | else => { |
| 6843 | // TODO: these errors are file system errors; make sure an update() will | 6843 | // TODO: these errors are file system errors; make sure an update() will |
| 6844 | // retry this and not cache the file system error, which may be transient. | 6844 | // retry this and not cache the file system error, which may be transient. |
| 6845 | return sema.fail(block, src, "unable to open '{s}': {s}", .{ name, @errorName(err) }); | 6845 | return sema.fail(block, operand_src, "unable to open '{s}': {s}", .{ name, @errorName(err) }); |
| 6846 | }, | 6846 | }, |
| 6847 | }; | 6847 | }; |
| 6848 | 6848 |
test/compile_errors.zig+9-1
| ... | @@ -11,7 +11,15 @@ pub fn addCases(ctx: *TestContext) !void { | ... | @@ -11,7 +11,15 @@ pub fn addCases(ctx: *TestContext) !void { |
| 11 | \\ return @embedFile("/root/foo").len; | 11 | \\ return @embedFile("/root/foo").len; |
| 12 | \\} | 12 | \\} |
| 13 | , &[_][]const u8{ | 13 | , &[_][]const u8{ |
| 14 | ":2:12: error: embed of file outside package path: '/root/foo'", | 14 | ":2:23: error: embed of file outside package path: '/root/foo'", |
| 15 | }); | ||
| 16 | |||
| 17 | case.addError( | ||
| 18 | \\export fn a() usize { | ||
| 19 | \\ return @import("../../above.zig").len; | ||
| 20 | \\} | ||
| 21 | , &[_][]const u8{ | ||
| 22 | ":2:20: error: import of file outside package path: '../../above.zig'", | ||
| 15 | }); | 23 | }); |
| 16 | } | 24 | } |
| 17 | 25 |