From 6561a98a61bb54c9d6c868f788da3eaa6f48d2c3 Mon Sep 17 00:00:00 2001 From: mlugg Date: Sun, 20 Apr 2025 02:01:46 +0100 Subject: [PATCH] incremental: correctly handle dead exporters Resolves: #23604 --- src/Zcu/PerThread.zig | 32 ++++++- test/incremental/change_exports | 161 ++++++++++++++++++++++++++++++++ 2 files changed, 190 insertions(+), 3 deletions(-) create mode 100644 test/incremental/change_exports diff --git a/src/Zcu/PerThread.zig b/src/Zcu/PerThread.zig index 095cd6c1cb..9084e8b0ca 100644 --- a/src/Zcu/PerThread.zig +++ b/src/Zcu/PerThread.zig @@ -2837,6 +2837,11 @@ pub fn processExports(pt: Zcu.PerThread) !void { const zcu = pt.zcu; const gpa = zcu.gpa; + if (zcu.single_exports.count() == 0 and zcu.multi_exports.count() == 0) { + // We can avoid a call to `resolveReferences` in this case. + return; + } + // First, construct a mapping of every exported value and Nav to the indices of all its different exports. var nav_exports: std.AutoArrayHashMapUnmanaged(InternPool.Nav.Index, std.ArrayListUnmanaged(Zcu.Export.Index)) = .empty; var uav_exports: std.AutoArrayHashMapUnmanaged(InternPool.Index, std.ArrayListUnmanaged(Zcu.Export.Index)) = .empty; @@ -2857,8 +2862,18 @@ pub fn processExports(pt: Zcu.PerThread) !void { // So, this ensureTotalCapacity serves as a reasonable (albeit very approximate) optimization. try nav_exports.ensureTotalCapacity(gpa, zcu.single_exports.count() + zcu.multi_exports.count()); - for (zcu.single_exports.values()) |export_idx| { + const unit_references = try zcu.resolveReferences(); + + for (zcu.single_exports.keys(), zcu.single_exports.values()) |exporter, export_idx| { const exp = export_idx.ptr(zcu); + if (!unit_references.contains(exporter)) { + // This export might already have been sent to the linker on a previous update, in which case we need to delete it. + // The linker export API should be modified to eliminate this call. #23616 + if (zcu.comp.bin_file) |lf| { + lf.deleteExport(exp.exported, exp.opts.name); + } + continue; + } const value_ptr, const found_existing = switch (exp.exported) { .nav => |nav| gop: { const gop = try nav_exports.getOrPut(gpa, nav); @@ -2873,8 +2888,19 @@ pub fn processExports(pt: Zcu.PerThread) !void { try value_ptr.append(gpa, export_idx); } - for (zcu.multi_exports.values()) |info| { - for (zcu.all_exports.items[info.index..][0..info.len], info.index..) |exp, export_idx| { + for (zcu.multi_exports.keys(), zcu.multi_exports.values()) |exporter, info| { + const exports = zcu.all_exports.items[info.index..][0..info.len]; + if (!unit_references.contains(exporter)) { + // This export might already have been sent to the linker on a previous update, in which case we need to delete it. + // The linker export API should be modified to eliminate this loop. #23616 + if (zcu.comp.bin_file) |lf| { + for (exports) |exp| { + lf.deleteExport(exp.exported, exp.opts.name); + } + } + continue; + } + for (exports, info.index..) |exp, export_idx| { const value_ptr, const found_existing = switch (exp.exported) { .nav => |nav| gop: { const gop = try nav_exports.getOrPut(gpa, nav); diff --git a/test/incremental/change_exports b/test/incremental/change_exports new file mode 100644 index 0000000000..f0e2ea8d34 --- /dev/null +++ b/test/incremental/change_exports @@ -0,0 +1,161 @@ +#target=x86_64-linux-selfhosted +#target=x86_64-linux-cbe +#target=x86_64-windows-cbe + +#update=initial version +#file=main.zig +export fn foo() void {} +const bar: u32 = 123; +const other: u32 = 456; +comptime { + @export(&bar, .{ .name = "bar" }); +} +pub fn main() !void { + const S = struct { + extern fn foo() void; + extern const bar: u32; + }; + S.foo(); + try std.io.getStdOut().writer().print("{}\n", .{S.bar}); +} +const std = @import("std"); +#expect_stdout="123\n" + +#update=add conflict +#file=main.zig +export fn foo() void {} +const bar: u32 = 123; +const other: u32 = 456; +comptime { + @export(&bar, .{ .name = "bar" }); + @export(&other, .{ .name = "foo" }); +} +pub fn main() !void { + const S = struct { + extern fn foo() void; + extern const bar: u32; + extern const other: u32; + }; + S.foo(); + try std.io.getStdOut().writer().print("{} {}\n", .{ S.bar, S.other }); +} +const std = @import("std"); +#expect_error=main.zig:6:5: error: exported symbol collision: foo +#expect_error=main.zig:1:1: note: other symbol here + +#update=resolve conflict +#file=main.zig +export fn foo() void {} +const bar: u32 = 123; +const other: u32 = 456; +comptime { + @export(&bar, .{ .name = "bar" }); + @export(&other, .{ .name = "other" }); +} +pub fn main() !void { + const S = struct { + extern fn foo() void; + extern const bar: u32; + extern const other: u32; + }; + S.foo(); + try std.io.getStdOut().writer().print("{} {}\n", .{ S.bar, S.other }); +} +const std = @import("std"); +#expect_stdout="123 456\n" + +#update=put exports in decl +#file=main.zig +export fn foo() void {} +const bar: u32 = 123; +const other: u32 = 456; +const does_exports = { + @export(&bar, .{ .name = "bar" }); + @export(&other, .{ .name = "other" }); +}; +comptime { + _ = does_exports; +} +pub fn main() !void { + const S = struct { + extern fn foo() void; + extern const bar: u32; + extern const other: u32; + }; + S.foo(); + try std.io.getStdOut().writer().print("{} {}\n", .{ S.bar, S.other }); +} +const std = @import("std"); +#expect_stdout="123 456\n" + +#update=remove reference to exporting decl +#file=main.zig +export fn foo() void {} +const bar: u32 = 123; +const other: u32 = 456; +const does_exports = { + @export(&bar, .{ .name = "bar" }); + @export(&other, .{ .name = "other" }); +}; +comptime { + //_ = does_exports; +} +pub fn main() !void { + const S = struct { + extern fn foo() void; + }; + S.foo(); +} +const std = @import("std"); +#expect_stdout="" + +#update=mark consts as export +#file=main.zig +export fn foo() void {} +export const bar: u32 = 123; +export const other: u32 = 456; +const does_exports = { + @export(&bar, .{ .name = "bar" }); + @export(&other, .{ .name = "other" }); +}; +comptime { + //_ = does_exports; +} +pub fn main() !void { + const S = struct { + extern fn foo() void; + extern const bar: u32; + extern const other: u32; + }; + S.foo(); + try std.io.getStdOut().writer().print("{} {}\n", .{ S.bar, S.other }); +} +const std = @import("std"); +#expect_stdout="123 456\n" + +#update=reintroduce reference to exporting decl, introducing conflict +#file=main.zig +export fn foo() void {} +export const bar: u32 = 123; +export const other: u32 = 456; +const does_exports = { + @export(&bar, .{ .name = "bar" }); + @export(&other, .{ .name = "other" }); +}; +comptime { + _ = does_exports; +} +pub fn main() !void { + const S = struct { + extern fn foo() void; + extern const bar: u32; + extern const other: u32; + }; + S.foo(); + try std.io.getStdOut().writer().print("{} {}\n", .{ S.bar, S.other }); +} +const std = @import("std"); +#expect_error=main.zig:5:5: error: exported symbol collision: bar +#expect_error=main.zig:2:1: note: other symbol here +#expect_error=main.zig:6:5: error: exported symbol collision: other +#expect_error=main.zig:3:1: note: other symbol here