Commit d979dd9b58

Andrew Kelley <andrew@ziglang.org>
2021-07-03 00:27:00
stage2: improve AstGen FileNotFound error message
Partially addresses #9203. It fixes the first case, but not the second one mentioned in the issue.
1 parent 5103053
lib/std/mem.zig
@@ -2297,14 +2297,14 @@ pub fn replaceOwned(comptime T: type, allocator: *Allocator, input: []const T, n
 }
 
 test "replaceOwned" {
-    const allocator = std.heap.page_allocator;
+    const gpa = std.testing.allocator;
 
-    const base_replace = replaceOwned(u8, allocator, "All your base are belong to us", "base", "Zig") catch unreachable;
-    defer allocator.free(base_replace);
+    const base_replace = replaceOwned(u8, gpa, "All your base are belong to us", "base", "Zig") catch @panic("out of memory");
+    defer gpa.free(base_replace);
     try testing.expect(eql(u8, base_replace, "All your Zig are belong to us"));
 
-    const zen_replace = replaceOwned(u8, allocator, "Favor reading code over writing code.", " code", "") catch unreachable;
-    defer allocator.free(zen_replace);
+    const zen_replace = replaceOwned(u8, gpa, "Favor reading code over writing code.", " code", "") catch @panic("out of memory");
+    defer gpa.free(zen_replace);
     try testing.expect(eql(u8, zen_replace, "Favor reading over writing."));
 }
 
src/AstGen.zig
@@ -34,8 +34,9 @@ string_table: std.StringHashMapUnmanaged(u32) = .{},
 compile_errors: ArrayListUnmanaged(Zir.Inst.CompileErrors.Item) = .{},
 /// The topmost block of the current function.
 fn_block: ?*GenZir = null,
-/// String table indexes, keeps track of all `@import` operands.
-imports: std.AutoArrayHashMapUnmanaged(u32, void) = .{},
+/// Maps string table indexes to the first `@import` ZIR instruction
+/// that uses this string as the operand.
+imports: std.AutoArrayHashMapUnmanaged(u32, Zir.Inst.Index) = .{},
 
 const InnerError = error{ OutOfMemory, AnalysisFail };
 
@@ -154,7 +155,7 @@ pub fn generate(gpa: *Allocator, tree: ast.Tree) Allocator.Error!Zir {
         astgen.extra.items[imports_index] = astgen.addExtraAssumeCapacity(Zir.Inst.Imports{
             .imports_len = @intCast(u32, astgen.imports.count()),
         });
-        astgen.extra.appendSliceAssumeCapacity(astgen.imports.keys());
+        astgen.extra.appendSliceAssumeCapacity(astgen.imports.values());
     }
 
     return Zir{
@@ -6863,8 +6864,13 @@ fn builtinCall(
             }
             const str_lit_token = main_tokens[operand_node];
             const str = try astgen.strLitAsString(str_lit_token);
-            try astgen.imports.put(astgen.gpa, str.index, {});
             const result = try gz.addStrTok(.import, str.index, str_lit_token);
+            if (gz.refToIndex(result)) |import_inst_index| {
+                const gop = try astgen.imports.getOrPut(astgen.gpa, str.index);
+                if (!gop.found_existing) {
+                    gop.value_ptr.* = import_inst_index;
+                }
+            }
             return rvalue(gz, rl, result, node);
         },
         .compile_log => {
src/Compilation.zig
@@ -1958,7 +1958,7 @@ pub fn performAllTheWork(self: *Compilation) error{ TimerUnsupported, OutOfMemor
         while (self.astgen_work_queue.readItem()) |file| {
             self.astgen_wait_group.start();
             try self.thread_pool.spawn(workerAstGenFile, .{
-                self, file, &zir_prog_node, &self.astgen_wait_group,
+                self, file, &zir_prog_node, &self.astgen_wait_group, .root,
             });
         }
 
@@ -2310,11 +2310,20 @@ pub fn performAllTheWork(self: *Compilation) error{ TimerUnsupported, OutOfMemor
     };
 }
 
+const AstGenSrc = union(enum) {
+    root,
+    import: struct {
+        importing_file: *Module.Scope.File,
+        import_inst: Zir.Inst.Index,
+    },
+};
+
 fn workerAstGenFile(
     comp: *Compilation,
     file: *Module.Scope.File,
     prog_node: *std.Progress.Node,
     wg: *WaitGroup,
+    src: AstGenSrc,
 ) void {
     defer wg.finish();
 
@@ -2327,7 +2336,7 @@ fn workerAstGenFile(
         error.AnalysisFail => return,
         else => {
             file.status = .retryable_failure;
-            comp.reportRetryableAstGenError(file, err) catch |oom| switch (oom) {
+            comp.reportRetryableAstGenError(src, file, err) catch |oom| switch (oom) {
                 // Swallowing this error is OK because it's implied to be OOM when
                 // there is a missing `failed_files` error message.
                 error.OutOfMemory => {},
@@ -2344,8 +2353,9 @@ fn workerAstGenFile(
     if (imports_index != 0) {
         const imports_len = file.zir.extra[imports_index];
 
-        for (file.zir.extra[imports_index + 1 ..][0..imports_len]) |str_index| {
-            const import_path = file.zir.nullTerminatedString(str_index);
+        for (file.zir.extra[imports_index + 1 ..][0..imports_len]) |import_inst| {
+            const inst_data = file.zir.instructions.items(.data)[import_inst].str_tok;
+            const import_path = inst_data.get(file.zir);
 
             const import_result = blk: {
                 const lock = comp.mutex.acquire();
@@ -2357,9 +2367,13 @@ fn workerAstGenFile(
                 log.debug("AstGen of {s} has import '{s}'; queuing AstGen of {s}", .{
                     file.sub_file_path, import_path, import_result.file.sub_file_path,
                 });
+                const sub_src: AstGenSrc = .{ .import = .{
+                    .importing_file = file,
+                    .import_inst = import_inst,
+                } };
                 wg.start();
                 comp.thread_pool.spawn(workerAstGenFile, .{
-                    comp, import_result.file, prog_node, wg,
+                    comp, import_result.file, prog_node, wg, sub_src,
                 }) catch {
                     wg.finish();
                     continue;
@@ -2570,6 +2584,7 @@ fn reportRetryableCObjectError(
 
 fn reportRetryableAstGenError(
     comp: *Compilation,
+    src: AstGenSrc,
     file: *Module.Scope.File,
     err: anyerror,
 ) error{OutOfMemory}!void {
@@ -2578,22 +2593,38 @@ fn reportRetryableAstGenError(
 
     file.status = .retryable_failure;
 
-    const src_loc: Module.SrcLoc = .{
-        .file_scope = file,
-        .parent_decl_node = 0,
-        .lazy = .entire_file,
+    const src_loc: Module.SrcLoc = switch (src) {
+        .root => .{
+            .file_scope = file,
+            .parent_decl_node = 0,
+            .lazy = .entire_file,
+        },
+        .import => |info| blk: {
+            const importing_file = info.importing_file;
+            const import_inst = info.import_inst;
+            const inst_data = importing_file.zir.instructions.items(.data)[import_inst].str_tok;
+            break :blk .{
+                .file_scope = importing_file,
+                .parent_decl_node = 0,
+                .lazy = .{ .token_offset = inst_data.src_tok },
+            };
+        },
     };
 
     const err_msg = if (file.pkg.root_src_directory.path) |dir_path|
         try Module.ErrorMsg.create(
             gpa,
             src_loc,
-            "unable to load {s}" ++ std.fs.path.sep_str ++ "{s}: {s}",
-            .{ dir_path, file.sub_file_path, @errorName(err) },
+            "unable to load '{'}" ++ std.fs.path.sep_str ++ "{'}': {s}",
+            .{
+                std.zig.fmtEscapes(dir_path),
+                std.zig.fmtEscapes(file.sub_file_path),
+                @errorName(err),
+            },
         )
     else
-        try Module.ErrorMsg.create(gpa, src_loc, "unable to load {s}: {s}", .{
-            file.sub_file_path, @errorName(err),
+        try Module.ErrorMsg.create(gpa, src_loc, "unable to load '{'}': {s}", .{
+            std.zig.fmtEscapes(file.sub_file_path), @errorName(err),
         });
     errdefer err_msg.destroy(gpa);
 
src/Module.zig
@@ -2485,7 +2485,7 @@ pub fn astGenFile(mod: *Module, file: *Scope.File) !void {
                 .file_scope = file,
                 .parent_decl_node = 0,
                 .lazy = .{ .byte_abs = byte_abs },
-            }, err_msg, "invalid byte: '{'}'", .{ std.zig.fmtEscapes(source[byte_abs..][0..1]) });
+            }, err_msg, "invalid byte: '{'}'", .{std.zig.fmtEscapes(source[byte_abs..][0..1])});
         }
 
         {
src/test.zig
@@ -699,6 +699,11 @@ pub const TestContext = struct {
             arena,
             &[_][]const u8{ ".", "zig-cache", "tmp", &tmp.sub_path },
         );
+        const tmp_dir_path_plus_slash = try std.fmt.allocPrint(
+            arena,
+            "{s}" ++ std.fs.path.sep_str,
+            .{tmp_dir_path},
+        );
         const local_cache_path = try std.fs.path.join(
             arena,
             &[_][]const u8{ tmp_dir_path, "zig-cache" },
@@ -773,7 +778,14 @@ pub const TestContext = struct {
                         var i: usize = 0;
                         ok = while (err_iter.next()) |line| : (i += 1) {
                             if (i >= case_error_list.len) break false;
-                            const expected = try std.fmt.allocPrint(arena, "{s}", .{case_error_list[i]});
+                            const expected = try std.mem.replaceOwned(
+                                u8,
+                                arena,
+                                try std.fmt.allocPrint(arena, "{s}", .{case_error_list[i]}),
+                                "${DIR}",
+                                tmp_dir_path_plus_slash,
+                            );
+
                             if (std.mem.indexOf(u8, line, expected) == null) break false;
                             continue;
                         } else true;
@@ -789,7 +801,13 @@ pub const TestContext = struct {
                         }
                     } else {
                         for (case_error_list) |msg| {
-                            const expected = try std.fmt.allocPrint(arena, "{s}", .{msg});
+                            const expected = try std.mem.replaceOwned(
+                                u8,
+                                arena,
+                                try std.fmt.allocPrint(arena, "{s}", .{msg}),
+                                "${DIR}",
+                                tmp_dir_path_plus_slash,
+                            );
                             if (std.mem.indexOf(u8, result.stderr, expected) == null) {
                                 print(
                                     \\
@@ -971,12 +989,20 @@ pub const TestContext = struct {
                                     const src_path_ok = case_msg.src.src_path.len == 0 or
                                         std.mem.eql(u8, case_msg.src.src_path, actual_msg.src_path);
 
+                                    const expected_msg = try std.mem.replaceOwned(
+                                        u8,
+                                        arena,
+                                        case_msg.src.msg,
+                                        "${DIR}",
+                                        tmp_dir_path_plus_slash,
+                                    );
+
                                     if (src_path_ok and
                                         (case_msg.src.line == std.math.maxInt(u32) or
                                         actual_msg.line == case_msg.src.line) and
                                         (case_msg.src.column == std.math.maxInt(u32) or
                                         actual_msg.column == case_msg.src.column) and
-                                        std.mem.eql(u8, case_msg.src.msg, actual_msg.msg) and
+                                        std.mem.eql(u8, expected_msg, actual_msg.msg) and
                                         case_msg.src.kind == .@"error")
                                     {
                                         handled_errors[i] = true;
@@ -1012,11 +1038,19 @@ pub const TestContext = struct {
                                     }
                                     if (ex_tag != .src) continue;
 
+                                    const expected_msg = try std.mem.replaceOwned(
+                                        u8,
+                                        arena,
+                                        case_msg.src.msg,
+                                        "${DIR}",
+                                        tmp_dir_path_plus_slash,
+                                    );
+
                                     if ((case_msg.src.line == std.math.maxInt(u32) or
                                         actual_msg.line == case_msg.src.line) and
                                         (case_msg.src.column == std.math.maxInt(u32) or
                                         actual_msg.column == case_msg.src.column) and
-                                        std.mem.eql(u8, case_msg.src.msg, actual_msg.msg) and
+                                        std.mem.eql(u8, expected_msg, actual_msg.msg) and
                                         case_msg.src.kind == .note)
                                     {
                                         handled_errors[i] = true;
src/Zir.zig
@@ -139,9 +139,15 @@ pub fn renderAsTextToFile(
     if (imports_index != 0) {
         try fs_file.writeAll("Imports:\n");
         const imports_len = scope_file.zir.extra[imports_index];
-        for (scope_file.zir.extra[imports_index + 1 ..][0..imports_len]) |str_index| {
-            const import_path = scope_file.zir.nullTerminatedString(str_index);
-            try fs_file.writer().print("  {s}\n", .{import_path});
+        for (scope_file.zir.extra[imports_index + 1 ..][0..imports_len]) |import_inst| {
+            const inst_data = writer.code.instructions.items(.data)[import_inst].str_tok;
+            const src = inst_data.src();
+            const import_path = inst_data.get(writer.code);
+            try fs_file.writer().print("  @import(\"{}\") ", .{
+                std.zig.fmtEscapes(import_path),
+            });
+            try writer.writeSrc(fs_file.writer(), src);
+            try fs_file.writer().writeAll("\n");
         }
     }
 }
@@ -2767,9 +2773,10 @@ pub const Inst = struct {
         };
     };
 
-    /// Trailing: for each `imports_len` there is a string table index.
+    /// Trailing: for each `imports_len` there is an instruction index
+    /// to an import instruction.
     pub const Imports = struct {
-        imports_len: u32,
+        imports_len: Zir.Inst.Index,
     };
 };
 
test/compile_errors.zig
@@ -4976,9 +4976,8 @@ pub fn addCases(ctx: *TestContext) !void {
 
     ctx.objErrStage1("bad import",
         \\const bogus = @import("bogus-does-not-exist.zig",);
-        \\export fn entry() void { bogus.bogo(); }
     , &[_][]const u8{
-        "tmp.zig:1:15: error: unable to find 'bogus-does-not-exist.zig'",
+        "tmp.zig:1:23: error: unable to load '${DIR}bogus-does-not-exist.zig': FileNotFound",
     });
 
     ctx.objErrStage1("undeclared identifier",