From 3dbc6f8434a2d995be018da0e5e9d8093d7bd9e8 Mon Sep 17 00:00:00 2001 From: agra Date: Sat, 6 Jun 2026 11:53:16 +0300 Subject: [PATCH] fix(imports): keep merged scope first-wins; index dups in module_fns only [0102a] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Attempt-1 retained a same-name cross-module FUNCTION author in the merged decl list (mergeFlat + the directory merge), which is the list the existing first-wins resolver consumes. That changed the data feeding resolution (`mod.decls` carried two `greet`), violating this step's purpose: additive indexes with ZERO resolution change. Revert both merge sites to byte-for-byte first-wins, exactly as on wt-fix-0102-base. The dropped same-name author is still retained — but only in the SEPARATE `module_fns` index, which is built from each module's `own_decls` (un-deduped, per-path) and which nothing reads yet. The `flat_import_graph` side data is likewise untouched. Both are foundation for fix-0102c's bare-name disambiguation; current resolution is unchanged. Drop the now-unused `declAuthorsFn` helper (its only callers were the two merge sites). `fnDeclOf` stays — it feeds the index. Tests: the existing unit test now asserts the merged scope stays first-wins (one `greet`, a.sx's author) while `module_fns` still retains BOTH authors and `flat_import_graph` excludes the namespaced edge. Add a mixed non-fn/fn collision test asserting the merged scope keeps a.sx's struct (first-wins), unchanged by the function author. --- src/imports.test.zig | 93 ++++++++++++++++++++++++++++++++++++++++---- src/imports.zig | 38 ++++-------------- 2 files changed, 93 insertions(+), 38 deletions(-) diff --git a/src/imports.test.zig b/src/imports.test.zig index 81f45a4..73a34a2 100644 --- a/src/imports.test.zig +++ b/src/imports.test.zig @@ -14,8 +14,10 @@ fn testIo() std.Io { } // Two flat-imported modules each author `greet`; a third is namespaced. The -// step retains BOTH `greet` authors under their own paths in `module_fns`, and -// records the namespaced import in `import_graph` but NOT in `flat_import_graph`. +// step retains BOTH `greet` authors under their own paths in `module_fns` and +// records the namespaced import in `import_graph` but NOT in `flat_import_graph` +// — WITHOUT touching the merged scope: `mod.decls` stays byte-for-byte +// first-wins (one `greet`, a.sx's), exactly as on `wt-fix-0102-base`. test "imports: module_fns retains same-name cross-module fns; flat_import_graph excludes namespaced edge" { var arena = std.heap.ArenaAllocator.init(std.testing.allocator); defer arena.deinit(); @@ -76,22 +78,29 @@ test "imports: module_fns retains same-name cross-module fns; flat_import_graph var module_fns = imports.ModuleFns.init(alloc); try imports.buildModuleFns(alloc, main_path, mod, &cache, &module_fns); - // mergeFlat no longer first-wins-drops the second `greet`: the global flat - // decl list the lowering pass walks now carries BOTH authors. + // The MERGED scope the first-wins resolver consumes is unchanged: mergeFlat + // still drops the second `greet`, so `mod.decls` carries exactly ONE — and + // it is a.sx's author (the first flat import), not b.sx's. var greet_count: usize = 0; + var merged_greet: ?*const ast.FnDecl = null; for (mod.decls) |decl| { const name = decl.data.declName() orelse continue; - if (std.mem.eql(u8, name, "greet")) greet_count += 1; + if (!std.mem.eql(u8, name, "greet")) continue; + greet_count += 1; + if (decl.data == .fn_decl) merged_greet = &decl.data.fn_decl; } - try std.testing.expectEqual(@as(usize, 2), greet_count); + try std.testing.expectEqual(@as(usize, 1), greet_count); - // module_fns retains BOTH authors of `greet`, keyed by their own paths. + // module_fns retains BOTH authors of `greet`, keyed by their own paths — + // the dropped author is recorded here (side index), not in the merged scope. const a_fns = module_fns.get(a_path) orelse return error.MissingAFns; const b_fns = module_fns.get(b_path) orelse return error.MissingBFns; const a_greet = a_fns.get("greet") orelse return error.MissingAGreet; const b_greet = b_fns.get("greet") orelse return error.MissingBGreet; // Distinct authoring decls — not the same node deduped down to one. try std.testing.expect(a_greet != b_greet); + // First-wins: the surviving merged-scope `greet` is a.sx's author. + try std.testing.expect(merged_greet == a_greet); // flat_import_graph carries the two bare `#import` edges, NOT the // namespaced `ns :: #import` edge. @@ -107,3 +116,73 @@ test "imports: module_fns retains same-name cross-module fns; flat_import_graph try std.testing.expect(full.contains(b_path)); try std.testing.expect(full.contains(ns_path)); } + +// Mixed collision: a.sx authors `Widget` as a STRUCT (non-fn), b.sx authors it +// as a FUNCTION. fix-0102a must NOT let the function-author retention shift the +// merged scope — first-wins keeps a.sx's struct and drops b.sx's function, +// exactly as on `wt-fix-0102-base`. (The fn author may still be indexed in +// module_fns; resolution is what must be untouched.) +test "imports: mixed non-fn/fn same-name collision stays first-wins in merged scope" { + var arena = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena.deinit(); + const alloc = arena.allocator(); + const io = testIo(); + + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + + try tmp.dir.writeFile(io, .{ .sub_path = "a.sx", .data = "Widget :: struct { x: s64 }\n" }); + try tmp.dir.writeFile(io, .{ .sub_path = "b.sx", .data = "Widget :: () -> s64 { 7 }\n" }); + const main_src = + \\#import "a.sx"; + \\#import "b.sx"; + \\main :: () -> s32 { 0 } + \\ + ; + try tmp.dir.writeFile(io, .{ .sub_path = "main.sx", .data = main_src }); + + var dirbuf: [4096]u8 = undefined; + const dirlen = try tmp.dir.realPath(io, &dirbuf); + const absdir = dirbuf[0..dirlen]; + const main_path = try std.fmt.allocPrint(alloc, "{s}/main.sx", .{absdir}); + + const main_bytes = try std.Io.Dir.readFileAlloc(.cwd(), io, main_path, alloc, .limited(1 << 20)); + const main_source = try alloc.dupeZ(u8, main_bytes); + var p = parser.Parser.init(alloc, main_source); + const root = p.parse() catch return error.ParseFailed; + + var chain = std.StringHashMap(void).init(alloc); + var cache = imports.ModuleCache.init(alloc); + var import_graph = std.StringHashMap(std.StringHashMap(void)).init(alloc); + var flat_import_graph = std.StringHashMap(std.StringHashMap(void)).init(alloc); + const stdlib_paths = [_][]const u8{}; + + const mod = try imports.resolveImports( + alloc, + io, + root, + absdir, + main_path, + &chain, + &cache, + null, + null, + &stdlib_paths, + &import_graph, + &flat_import_graph, + .{}, + ); + + // Exactly ONE `Widget` survives the merged scope, and it is a.sx's STRUCT — + // the function author did not displace or duplicate it. + var widget_count: usize = 0; + var merged_is_struct = false; + for (mod.decls) |decl| { + const name = decl.data.declName() orelse continue; + if (!std.mem.eql(u8, name, "Widget")) continue; + widget_count += 1; + merged_is_struct = decl.data == .struct_decl; + } + try std.testing.expectEqual(@as(usize, 1), widget_count); + try std.testing.expect(merged_is_struct); +} diff --git a/src/imports.zig b/src/imports.zig index 34e71cd..a2943be 100644 --- a/src/imports.zig +++ b/src/imports.zig @@ -5,24 +5,10 @@ const errors = @import("errors.zig"); const c_import = @import("c_import.zig"); const Node = ast.Node; -/// True iff `decl` authors a top-level FUNCTION — either a bare `fn_decl` -/// (`f :: (…) -> T { … }`) or a `const_decl` whose value is a function -/// (`f :: (…) { … }` parsed as a const-bound fn). Used by the flat / directory -/// merge to retain a same-name function authored by a DIFFERENT module instead -/// of first-wins dropping it (fix-0102a): a flat importer that pulls two modules -/// each authoring `f` needs BOTH decls reachable downstream. Non-function decls -/// keep first-wins dedup. -fn declAuthorsFn(decl: *const Node) bool { - return switch (decl.data) { - .fn_decl => true, - .const_decl => |cd| cd.value.data == .fn_decl, - else => false, - }; -} - /// The `*const ast.FnDecl` a function-authoring decl carries, or null when the -/// decl is not a function (mirrors `declAuthorsFn`). Drives the per-module -/// `module_fns` identity index (fix-0102a). +/// decl is not a function — either a bare `fn_decl` (`f :: (…) -> T { … }`) or a +/// `const_decl` whose value is a function. Drives the per-module `module_fns` +/// identity index (fix-0102a). fn fnDeclOf(decl: *const Node) ?*const ast.FnDecl { return switch (decl.data) { .fn_decl => &decl.data.fn_decl, @@ -360,13 +346,8 @@ pub const ResolvedModule = struct { for (other.decls) |decl| { if (seen_nodes.contains(decl)) continue; if (decl.data.declName()) |name| { - if (seen_list.contains(name)) { - // First-wins dedup for non-functions; retain a same-name - // FUNCTION authored by a different module (fix-0102a). - if (!declAuthorsFn(decl)) continue; - } else { - try seen_list.put(name, {}); - } + if (seen_list.contains(name)) continue; + try seen_list.put(name, {}); } try seen_nodes.put(decl, {}); try list.append(allocator, decl); @@ -787,13 +768,8 @@ fn resolveDirectoryImport( for (file_mod.decls) |decl| { if (seen_nodes.contains(decl)) continue; if (decl.data.declName()) |name| { - if (seen_in_list.contains(name)) { - // First-wins dedup for non-functions; retain a same-name - // FUNCTION authored by a different file (fix-0102a). - if (!declAuthorsFn(decl)) continue; - } else { - try seen_in_list.put(name, {}); - } + if (seen_in_list.contains(name)) continue; + try seen_in_list.put(name, {}); } try seen_nodes.put(decl, {}); try decl_list.append(allocator, decl);