From e266ede6e310b64adcf3912b8ef2b9e2397fde7b Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Wed, 24 Nov 2021 19:09:45 -0700 Subject: [PATCH] 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 #9939 --- src/Module.zig | 8 +++++--- src/Sema.zig | 14 +++++++------- test/compile_errors.zig | 10 +++++++++- 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/src/Module.zig b/src/Module.zig index 75786d3f6d..153c2199b3 100644 --- a/src/Module.zig +++ b/src/Module.zig @@ -3541,11 +3541,11 @@ pub fn importPkg(mod: *Module, pkg: *Package) !ImportFileResult { defer if (!keep_resolved_path) gpa.free(resolved_path); const gop = try mod.import_table.getOrPut(gpa, resolved_path); + errdefer _ = mod.import_table.pop(); if (gop.found_existing) return ImportFileResult{ .file = gop.value_ptr.*, .is_new = false, }; - keep_resolved_path = true; // It's now owned by import_table. const sub_file_path = try gpa.dupe(u8, pkg.root_src_path); errdefer gpa.free(sub_file_path); @@ -3553,6 +3553,7 @@ pub fn importPkg(mod: *Module, pkg: *Package) !ImportFileResult { const new_file = try gpa.create(File); errdefer gpa.destroy(new_file); + keep_resolved_path = true; // It's now owned by import_table. gop.value_ptr.* = new_file; new_file.* = .{ .sub_file_path = sub_file_path, @@ -3599,11 +3600,11 @@ pub fn importFile( defer if (!keep_resolved_path) gpa.free(resolved_path); const gop = try mod.import_table.getOrPut(gpa, resolved_path); + errdefer _ = mod.import_table.pop(); if (gop.found_existing) return ImportFileResult{ .file = gop.value_ptr.*, .is_new = false, }; - keep_resolved_path = true; // It's now owned by import_table. const new_file = try gpa.create(File); errdefer gpa.destroy(new_file); @@ -3622,6 +3623,7 @@ pub fn importFile( resolved_root_path, resolved_path, sub_file_path, import_string, }); + keep_resolved_path = true; // It's now owned by import_table. gop.value_ptr.* = new_file; new_file.* = .{ .sub_file_path = sub_file_path, @@ -3657,8 +3659,8 @@ pub fn embedFile(mod: *Module, cur_file: *File, rel_file_path: []const u8) !*Emb defer if (!keep_resolved_path) gpa.free(resolved_path); const gop = try mod.embed_table.getOrPut(gpa, resolved_path); - if (gop.found_existing) return gop.value_ptr.*; errdefer assert(mod.embed_table.remove(resolved_path)); + if (gop.found_existing) return gop.value_ptr.*; const new_file = try gpa.create(EmbedFile); errdefer gpa.destroy(new_file); diff --git a/src/Sema.zig b/src/Sema.zig index 413c82e005..ab8d0a4e23 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -6807,17 +6807,17 @@ fn zirImport(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air. const mod = sema.mod; const inst_data = sema.code.instructions.items(.data)[inst].str_tok; - const src = inst_data.src(); + const operand_src = inst_data.src(); const operand = inst_data.get(sema.code); const result = mod.importFile(block.getFileScope(), operand) catch |err| switch (err) { error.ImportOutsidePkgPath => { - return sema.fail(block, src, "import of file outside package path: '{s}'", .{operand}); + return sema.fail(block, operand_src, "import of file outside package path: '{s}'", .{operand}); }, else => { // TODO: these errors are file system errors; make sure an update() will // retry this and not cache the file system error, which may be transient. - return sema.fail(block, src, "unable to open '{s}': {s}", .{ operand, @errorName(err) }); + return sema.fail(block, operand_src, "unable to open '{s}': {s}", .{ operand, @errorName(err) }); }, }; try mod.semaFile(result.file); @@ -6832,17 +6832,17 @@ fn zirEmbedFile(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!A const mod = sema.mod; const inst_data = sema.code.instructions.items(.data)[inst].un_node; - const src = inst_data.src(); - const name = try sema.resolveConstString(block, src, inst_data.operand); + const operand_src: LazySrcLoc = .{ .node_offset_builtin_call_arg0 = inst_data.src_node }; + const name = try sema.resolveConstString(block, operand_src, inst_data.operand); const embed_file = mod.embedFile(block.getFileScope(), name) catch |err| switch (err) { error.ImportOutsidePkgPath => { - return sema.fail(block, src, "embed of file outside package path: '{s}'", .{name}); + return sema.fail(block, operand_src, "embed of file outside package path: '{s}'", .{name}); }, else => { // TODO: these errors are file system errors; make sure an update() will // retry this and not cache the file system error, which may be transient. - return sema.fail(block, src, "unable to open '{s}': {s}", .{ name, @errorName(err) }); + return sema.fail(block, operand_src, "unable to open '{s}': {s}", .{ name, @errorName(err) }); }, }; diff --git a/test/compile_errors.zig b/test/compile_errors.zig index 6bf7057064..21bdbfbcfb 100644 --- a/test/compile_errors.zig +++ b/test/compile_errors.zig @@ -11,7 +11,15 @@ pub fn addCases(ctx: *TestContext) !void { \\ return @embedFile("/root/foo").len; \\} , &[_][]const u8{ - ":2:12: error: embed of file outside package path: '/root/foo'", + ":2:23: error: embed of file outside package path: '/root/foo'", + }); + + case.addError( + \\export fn a() usize { + \\ return @import("../../above.zig").len; + \\} + , &[_][]const u8{ + ":2:20: error: import of file outside package path: '../../above.zig'", }); }