wasm linker: improve error messages by making source locations more lazy

4 files changed, 77 insertions, 34 deletions

diff --git a/src/Compilation.zig b/src/Compilation.zig
index ea514ee9ec..71299bccc7 100644
--- a/src/Compilation.zig
+++ b/src/Compilation.zig
@@ -3291,7 +3291,7 @@ pub fn getAllErrorsAlloc(comp: *Compilation) !ErrorBundle {
         }));
     }
 
-    try comp.link_diags.addMessagesToBundle(&bundle);
+    try comp.link_diags.addMessagesToBundle(&bundle, comp.bin_file);
 
     if (comp.zcu) |zcu| {
         if (bundle.root_list.items.len == 0 and zcu.compile_log_sources.count() != 0) {
diff --git a/src/link.zig b/src/link.zig
index b9bcd843fb..17ccb8ac0c 100644
--- a/src/link.zig
+++ b/src/link.zig
@@ -38,6 +38,11 @@ pub const Diags = struct {
     flags: Flags,
     lld: std.ArrayListUnmanaged(Lld),
 
+    pub const SourceLocation = union(enum) {
+        none,
+        wasm: File.Wasm.SourceLocation,
+    };
+
     pub const Flags = packed struct {
         no_entry_point_found: bool = false,
         missing_libc: bool = false,
@@ -70,9 +75,25 @@ pub const Diags = struct {
     };
 
     pub const Msg = struct {
+        source_location: SourceLocation = .none,
         msg: []const u8,
         notes: []Msg = &.{},
 
+        fn string(
+            msg: *const Msg,
+            bundle: *std.zig.ErrorBundle.Wip,
+            base: ?*File,
+        ) Allocator.Error!std.zig.ErrorBundle.String {
+            return switch (msg.source_location) {
+                .none => try bundle.addString(msg.msg),
+                .wasm => |sl| {
+                    dev.check(.wasm_linker);
+                    const wasm = base.?.cast(.wasm).?;
+                    return sl.string(msg.msg, bundle, wasm);
+                },
+            };
+        }
+
         pub fn deinit(self: *Msg, gpa: Allocator) void {
             for (self.notes) |*note| note.deinit(gpa);
             gpa.free(self.notes);
@@ -326,16 +347,16 @@ pub const Diags = struct {
         diags.flags.alloc_failure_occurred = true;
     }
 
-    pub fn addMessagesToBundle(diags: *const Diags, bundle: *std.zig.ErrorBundle.Wip) Allocator.Error!void {
+    pub fn addMessagesToBundle(diags: *const Diags, bundle: *std.zig.ErrorBundle.Wip, base: ?*File) Allocator.Error!void {
         for (diags.msgs.items) |link_err| {
             try bundle.addRootErrorMessage(.{
-                .msg = try bundle.addString(link_err.msg),
+                .msg = try link_err.string(bundle, base),
                 .notes_len = @intCast(link_err.notes.len),
             });
             const notes_start = try bundle.reserveNotes(@intCast(link_err.notes.len));
             for (link_err.notes, 0..) |note, i| {
                 bundle.extra.items[notes_start + i] = @intFromEnum(try bundle.addErrorMessage(.{
-                    .msg = try bundle.addString(note.msg),
+                    .msg = try note.string(bundle, base),
                 }));
             }
         }
@@ -2224,7 +2245,7 @@ fn resolvePathInputLib(
             try wip_errors.init(gpa);
             defer wip_errors.deinit();
 
-            try diags.addMessagesToBundle(&wip_errors);
+            try diags.addMessagesToBundle(&wip_errors, null);
 
             var error_bundle = try wip_errors.toOwnedBundle("");
             defer error_bundle.deinit(gpa);
diff --git a/src/link/Wasm.zig b/src/link/Wasm.zig
index 49df77d6a8..9d0a9385fe 100644
--- a/src/link/Wasm.zig
+++ b/src/link/Wasm.zig
@@ -465,17 +465,33 @@ pub const SourceLocation = enum(u32) {
 
     pub fn addNote(
         sl: SourceLocation,
-        wasm: *Wasm,
         err: *link.Diags.ErrorWithNotes,
         comptime f: []const u8,
         args: anytype,
     ) void {
-        switch (sl.unpack(wasm)) {
-            .none => err.addNote(f, args),
-            .zig_object_nofile => err.addNote("zig compilation unit: " ++ f, args),
-            .object_index => |i| err.addNote("{}: " ++ f, .{i.ptr(wasm).path} ++ args),
+        err.addNote(f, args);
+        const err_msg = &err.diags.msgs.items[err.index];
+        err_msg.notes[err.note_slot - 1].source_location = .{ .wasm = sl };
+    }
+
+    pub fn string(
+        sl: SourceLocation,
+        msg: []const u8,
+        bundle: *std.zig.ErrorBundle.Wip,
+        wasm: *const Wasm,
+    ) Allocator.Error!std.zig.ErrorBundle.String {
+        return switch (sl.unpack(wasm)) {
+            .none => try bundle.addString(msg),
+            .zig_object_nofile => try bundle.printString("zig compilation unit: {s}", .{msg}),
+            .object_index => |i| {
+                const obj = i.ptr(wasm);
+                return if (obj.archive_member_name.slice(wasm)) |obj_name|
+                    try bundle.printString("{} ({s}): {s}", .{ obj.path, std.fs.path.basename(obj_name), msg })
+                else
+                    try bundle.printString("{}: {s}", .{ obj.path, msg });
+            },
             .source_location_index => @panic("TODO"),
-        }
+        };
     }
 };
 
@@ -3679,6 +3695,12 @@ fn defaultEntrySymbolName(
     };
 }
 
+pub fn internOptionalString(wasm: *Wasm, optional_bytes: ?[]const u8) Allocator.Error!OptionalString {
+    const bytes = optional_bytes orelse return .none;
+    const string = try internString(wasm, bytes);
+    return string.toOptional();
+}
+
 pub fn internString(wasm: *Wasm, bytes: []const u8) Allocator.Error!String {
     assert(mem.indexOfScalar(u8, bytes, 0) == null);
     wasm.string_bytes_lock.lock();
diff --git a/src/link/Wasm/Object.zig b/src/link/Wasm/Object.zig
index b42b9c8967..ea8705ceb9 100644
--- a/src/link/Wasm/Object.zig
+++ b/src/link/Wasm/Object.zig
@@ -17,7 +17,7 @@ path: Path,
 /// For error reporting purposes only.
 /// If this represents an object in an archive, it's the basename of the
 /// object, and path refers to the archive.
-archive_member_name: ?[]const u8,
+archive_member_name: Wasm.OptionalString,
 /// Represents the function ID that must be called on startup.
 /// This is `null` by default as runtimes may determine the startup
 /// function themselves. This is essentially legacy.
@@ -965,21 +965,21 @@ pub fn parse(
                 if (gop.value_ptr.type != fn_ty_index) {
                     var err = try diags.addErrorWithNotes(2);
                     try err.addMsg("symbol '{s}' mismatching function signatures", .{name.slice(wasm)});
-                    gop.value_ptr.source_location.addNote(wasm, &err, "imported as {} here", .{
+                    gop.value_ptr.source_location.addNote(&err, "imported as {} here", .{
                         gop.value_ptr.type.fmt(wasm),
                     });
-                    err.addNote("{}: imported as {} here", .{ path, fn_ty_index.fmt(wasm) });
+                    source_location.addNote(&err, "imported as {} here", .{fn_ty_index.fmt(wasm)});
                     continue;
                 }
                 if (gop.value_ptr.module_name != ptr.module_name.toOptional()) {
                     var err = try diags.addErrorWithNotes(2);
                     try err.addMsg("symbol '{s}' mismatching module names", .{name.slice(wasm)});
                     if (gop.value_ptr.module_name.slice(wasm)) |module_name| {
-                        gop.value_ptr.source_location.addNote(wasm, &err, "module '{s}' here", .{module_name});
+                        gop.value_ptr.source_location.addNote(&err, "module '{s}' here", .{module_name});
                     } else {
-                        gop.value_ptr.source_location.addNote(wasm, &err, "no module here", .{});
+                        gop.value_ptr.source_location.addNote(&err, "no module here", .{});
                     }
-                    err.addNote("{}: module '{s}' here", .{ path, ptr.module_name.slice(wasm) });
+                    source_location.addNote(&err, "module '{s}' here", .{ptr.module_name.slice(wasm)});
                     continue;
                 }
                 if (symbol.flags.binding == .strong) gop.value_ptr.flags.binding = .strong;
@@ -1008,18 +1008,18 @@ pub fn parse(
                 if (ptr.valtype != existing_ty.valtype) {
                     var err = try diags.addErrorWithNotes(2);
                     try err.addMsg("symbol '{s}' mismatching global types", .{name.slice(wasm)});
-                    gop.value_ptr.source_location.addNote(wasm, &err, "type {s} here", .{@tagName(existing_ty.valtype)});
-                    err.addNote("{}: type {s} here", .{ path, @tagName(ptr.valtype) });
+                    gop.value_ptr.source_location.addNote(&err, "type {s} here", .{@tagName(existing_ty.valtype)});
+                    source_location.addNote(&err, "type {s} here", .{@tagName(ptr.valtype)});
                     continue;
                 }
                 if (ptr.mutable != existing_ty.mutable) {
                     var err = try diags.addErrorWithNotes(2);
                     try err.addMsg("symbol '{s}' mismatching global mutability", .{name.slice(wasm)});
-                    gop.value_ptr.source_location.addNote(wasm, &err, "{s} here", .{
+                    gop.value_ptr.source_location.addNote(&err, "{s} here", .{
                         if (existing_ty.mutable) "mutable" else "not mutable",
                     });
-                    err.addNote("{}: {s} here", .{
-                        path, if (ptr.mutable) "mutable" else "not mutable",
+                    source_location.addNote(&err, "{s} here", .{
+                        if (ptr.mutable) "mutable" else "not mutable",
                     });
                     continue;
                 }
@@ -1027,11 +1027,11 @@ pub fn parse(
                     var err = try diags.addErrorWithNotes(2);
                     try err.addMsg("symbol '{s}' mismatching module names", .{name.slice(wasm)});
                     if (gop.value_ptr.module_name.slice(wasm)) |module_name| {
-                        gop.value_ptr.source_location.addNote(wasm, &err, "module '{s}' here", .{module_name});
+                        gop.value_ptr.source_location.addNote(&err, "module '{s}' here", .{module_name});
                     } else {
-                        gop.value_ptr.source_location.addNote(wasm, &err, "no module here", .{});
+                        gop.value_ptr.source_location.addNote(&err, "no module here", .{});
                     }
-                    err.addNote("{}: module '{s}' here", .{ path, ptr.module_name.slice(wasm) });
+                    source_location.addNote(&err, "module '{s}' here", .{ptr.module_name.slice(wasm)});
                     continue;
                 }
                 if (symbol.flags.binding == .strong) gop.value_ptr.flags.binding = .strong;
@@ -1063,17 +1063,17 @@ pub fn parse(
                 if (ptr.ref_type != existing_reftype) {
                     var err = try diags.addErrorWithNotes(2);
                     try err.addMsg("symbol '{s}' mismatching table reftypes", .{name.slice(wasm)});
-                    gop.value_ptr.source_location.addNote(wasm, &err, "{s} here", .{@tagName(existing_reftype)});
-                    err.addNote("{}: {s} here", .{ path, @tagName(ptr.ref_type) });
+                    gop.value_ptr.source_location.addNote(&err, "{s} here", .{@tagName(existing_reftype)});
+                    source_location.addNote(&err, "{s} here", .{@tagName(ptr.ref_type)});
                     continue;
                 }
                 if (gop.value_ptr.module_name != ptr.module_name) {
                     var err = try diags.addErrorWithNotes(2);
                     try err.addMsg("symbol '{s}' mismatching module names", .{name.slice(wasm)});
-                    gop.value_ptr.source_location.addNote(wasm, &err, "module '{s}' here", .{
+                    gop.value_ptr.source_location.addNote(&err, "module '{s}' here", .{
                         gop.value_ptr.module_name.slice(wasm),
                     });
-                    err.addNote("{}: module '{s}' here", .{ path, ptr.module_name.slice(wasm) });
+                    source_location.addNote(&err, "module '{s}' here", .{ptr.module_name.slice(wasm)});
                     continue;
                 }
                 if (symbol.flags.binding == .strong) gop.value_ptr.flags.binding = .strong;
@@ -1105,11 +1105,11 @@ pub fn parse(
                 if (gop.value_ptr.type != ptr.type_index) {
                     var err = try diags.addErrorWithNotes(2);
                     try err.addMsg("function signature mismatch: {s}", .{name.slice(wasm)});
-                    gop.value_ptr.source_location.addNote(wasm, &err, "exported as {} here", .{
+                    gop.value_ptr.source_location.addNote(&err, "exported as {} here", .{
                         ptr.type_index.fmt(wasm),
                     });
                     const word = if (gop.value_ptr.resolution == .unresolved) "imported" else "exported";
-                    err.addNote("{}: {s} as {} here", .{ path, word, gop.value_ptr.type.fmt(wasm) });
+                    source_location.addNote(&err, "{s} as {} here", .{ word, gop.value_ptr.type.fmt(wasm) });
                     continue;
                 }
                 if (gop.value_ptr.resolution == .unresolved or gop.value_ptr.flags.binding == .weak) {
@@ -1121,8 +1121,8 @@ pub fn parse(
                 }
                 var err = try diags.addErrorWithNotes(2);
                 try err.addMsg("symbol collision: {s}", .{name.slice(wasm)});
-                gop.value_ptr.source_location.addNote(wasm, &err, "exported as {} here", .{ptr.type_index.fmt(wasm)});
-                err.addNote("{}: exported as {} here", .{ path, gop.value_ptr.type.fmt(wasm) });
+                gop.value_ptr.source_location.addNote(&err, "exported as {} here", .{ptr.type_index.fmt(wasm)});
+                source_location.addNote(&err, "exported as {} here", .{gop.value_ptr.type.fmt(wasm)});
                 continue;
             } else {
                 gop.value_ptr.* = .{
@@ -1242,7 +1242,7 @@ pub fn parse(
     return .{
         .version = version,
         .path = path,
-        .archive_member_name = archive_member_name,
+        .archive_member_name = try wasm.internOptionalString(archive_member_name),
         .start_function = start_function,
         .features = features,
         .functions = .{