imports: dedup flat decl list by node identity (issue 0056 FIXED)
Impl blocks are anonymous (no declName), so a parameterised-protocol impl in a module reached via a diamond import was appended once per path and registered twice — 'duplicate impl Into for source s64'. mergeFlat and the directory-import merge loop now also dedup by node pointer; a physical AST node is lowered once regardless of how many import paths reach it. Regression: examples/issue-0056-diamond-param-impl.sx.
This commit is contained in:
19
examples/issue-0056-diamond-param-impl.sx
Normal file
19
examples/issue-0056-diamond-param-impl.sx
Normal file
@@ -0,0 +1,19 @@
|
||||
// Regression (issue 0056): a parameterised-protocol (`Into`) impl living in a
|
||||
// module reached through more than one import path must register exactly once.
|
||||
//
|
||||
// main ─┬─ issue-0056/mid_a ─┐
|
||||
// └─ issue-0056/mid_b ─┴─ issue-0056/common (holds `impl Into(Wrapped) for s64`)
|
||||
//
|
||||
// Impl blocks are anonymous (no `declName`), so before the fix the diamond
|
||||
// dedup in imports.zig appended the cached node once per path and the second
|
||||
// registration tripped: `duplicate impl 'Into' for source 's64'`. Now the flat
|
||||
// decl list also dedups by node identity, so this builds and prints 7.
|
||||
#import "modules/std.sx";
|
||||
#import "issue-0056/mid_a.sx";
|
||||
#import "issue-0056/mid_b.sx";
|
||||
|
||||
main :: () -> s32 {
|
||||
w : Wrapped = xx 7;
|
||||
print("{}\n", w.v);
|
||||
0;
|
||||
}
|
||||
12
examples/issue-0056/common.sx
Normal file
12
examples/issue-0056/common.sx
Normal file
@@ -0,0 +1,12 @@
|
||||
// A parameterised-protocol (`Into`) impl living in a module reachable through
|
||||
// more than one import path. Each path must NOT re-register the impl.
|
||||
// Impl blocks are anonymous (`declName() == null`), so the diamond-import
|
||||
// dedup in imports.zig (`mergeFlat`) skips them and appends the node once per
|
||||
// path — `registerParamImpl` then trips its same-file duplicate check.
|
||||
Wrapped :: struct { v: s64; }
|
||||
|
||||
impl Into(Wrapped) for s64 {
|
||||
convert :: (self: s64) -> Wrapped {
|
||||
return .{ v = self };
|
||||
}
|
||||
}
|
||||
3
examples/issue-0056/mid_a.sx
Normal file
3
examples/issue-0056/mid_a.sx
Normal file
@@ -0,0 +1,3 @@
|
||||
#import "common.sx";
|
||||
|
||||
mid_a_marker :: () -> s64 { 1; }
|
||||
3
examples/issue-0056/mid_b.sx
Normal file
3
examples/issue-0056/mid_b.sx
Normal file
@@ -0,0 +1,3 @@
|
||||
#import "common.sx";
|
||||
|
||||
mid_b_marker :: () -> s64 { 2; }
|
||||
79
issues/0056-param-impl-not-deduped-across-diamond-import.md
Normal file
79
issues/0056-param-impl-not-deduped-across-diamond-import.md
Normal file
@@ -0,0 +1,79 @@
|
||||
# 0056 — parameterised-protocol impl not deduped across a diamond import
|
||||
|
||||
**FIXED** (`examples/issue-0056-diamond-param-impl.sx`, helpers in
|
||||
`examples/issue-0056/`). The flat decl list in
|
||||
[src/imports.zig](../src/imports.zig) now dedups by **node identity** as well
|
||||
as by name. `ResolvedModule.mergeFlat` and the directory-import merge loop each
|
||||
carry a `seen_nodes: std.AutoHashMap(*Node, void)` alongside the existing
|
||||
`seen_in_list` name set, and skip a decl whose pointer was already appended.
|
||||
|
||||
## Symptom
|
||||
|
||||
A module containing a parameterised-protocol impl (`impl Into(T) for S`) could
|
||||
not be imported through more than one path. Under a diamond —
|
||||
|
||||
```
|
||||
main ─┬─ mid_a ─┐
|
||||
└─ mid_b ─┴─ common (holds `impl Into(Wrapped) for s64`)
|
||||
```
|
||||
|
||||
— compilation failed with:
|
||||
|
||||
```
|
||||
error: duplicate impl 'Into' for source 's64' in .../common.sx
|
||||
```
|
||||
|
||||
This bit the moment `modules/std/objc.sx` (imported by `main.sx`,
|
||||
`platform/uikit.sx`, and `gpu/metal.sx` — a diamond) gained an
|
||||
`impl Into(*NSString) for string`.
|
||||
|
||||
## Root cause
|
||||
|
||||
`mergeFlat`/`addOwnDecl` dedup the global flat decl list by
|
||||
`decl.data.declName()`. Named decls (structs, fns, foreign classes) dedup
|
||||
fine across diamonds. But `impl_block` is anonymous — `declName()` returns
|
||||
`null` (see [src/ast.zig](../src/ast.zig) `Data.declName`) — so the dedup
|
||||
guard was skipped and the **same cached** impl node (modules are cached in
|
||||
`ModuleCache`, so both paths share one node pointer) was appended once per
|
||||
path. `registerParamImpl` in [src/ir/lower.zig](../src/ir/lower.zig) then saw
|
||||
two entries with the same `defining_module` and raised the same-file
|
||||
"duplicate impl" diagnostic.
|
||||
|
||||
The pre-existing `impl Into(Block) for Closure(...)` in
|
||||
`modules/std/objc_block.sx` never tripped this because that module is not
|
||||
imported through any diamond.
|
||||
|
||||
## Reproduction
|
||||
|
||||
`examples/issue-0056/common.sx`:
|
||||
|
||||
```sx
|
||||
Wrapped :: struct { v: s64; }
|
||||
|
||||
impl Into(Wrapped) for s64 {
|
||||
convert :: (self: s64) -> Wrapped {
|
||||
return .{ v = self };
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
`mid_a.sx` / `mid_b.sx` each `#import "common.sx";`. The diamond main:
|
||||
|
||||
```sx
|
||||
#import "modules/std.sx";
|
||||
#import "issue-0056/mid_a.sx";
|
||||
#import "issue-0056/mid_b.sx";
|
||||
|
||||
main :: () -> s32 {
|
||||
w : Wrapped = xx 7; // pre-fix: duplicate impl 'Into' for source 's64'
|
||||
print("{}\n", w.v); // post-fix: prints 7
|
||||
0;
|
||||
}
|
||||
```
|
||||
|
||||
## Fix
|
||||
|
||||
Dedup the flat decl list by node identity in addition to name — a physical
|
||||
AST node must be lowered once regardless of how many import paths reach it.
|
||||
Named-decl first-wins behaviour is unchanged. Regression test:
|
||||
`examples/issue-0056-diamond-param-impl.sx` (in `tests/run_examples.sh`).
|
||||
@@ -315,20 +315,29 @@ pub const ResolvedModule = struct {
|
||||
/// `self.scope` — visibility joins per-file scopes at lookup time via
|
||||
/// `import_graph`. We only need to append `other.decls` (the full
|
||||
/// transitive list) to the global `list` so the lowering pass can
|
||||
/// still resolve transitively-imported callees. Deduped by name.
|
||||
/// still resolve transitively-imported callees.
|
||||
///
|
||||
/// Deduped two ways: named decls by name (first-wins on cross-module
|
||||
/// collisions), and EVERY decl by node identity. The latter matters for
|
||||
/// anonymous decls — `impl` blocks have no `declName`, so under a diamond
|
||||
/// import the same cached node would otherwise be appended once per path
|
||||
/// and registered twice (e.g. `duplicate impl 'Into'`).
|
||||
pub fn mergeFlat(
|
||||
self: *ResolvedModule,
|
||||
allocator: std.mem.Allocator,
|
||||
list: *std.ArrayList(*Node),
|
||||
seen_list: *std.StringHashMap(void),
|
||||
seen_nodes: *std.AutoHashMap(*Node, void),
|
||||
other: ResolvedModule,
|
||||
) !void {
|
||||
_ = self;
|
||||
for (other.decls) |decl| {
|
||||
if (seen_nodes.contains(decl)) continue;
|
||||
if (decl.data.declName()) |name| {
|
||||
if (seen_list.contains(name)) continue;
|
||||
try seen_list.put(name, {});
|
||||
}
|
||||
try seen_nodes.put(decl, {});
|
||||
try list.append(allocator, decl);
|
||||
}
|
||||
}
|
||||
@@ -420,6 +429,9 @@ pub fn resolveImports(
|
||||
// by `mergeFlat` to dedupe across diamond imports now that `mod.scope`
|
||||
// is non-transitive and can no longer serve as the dedup key.
|
||||
var seen_in_list = std.StringHashMap(void).init(allocator);
|
||||
// Node-identity set for the same purpose, covering anonymous decls
|
||||
// (impl blocks) that carry no name to dedupe on.
|
||||
var seen_nodes = std.AutoHashMap(*Node, void).init(allocator);
|
||||
|
||||
for (flat_decls) |decl| {
|
||||
if (decl.data == .c_import_decl) {
|
||||
@@ -559,7 +571,7 @@ pub fn resolveImports(
|
||||
if (imp.name) |ns_name| {
|
||||
try mod.addNamespace(allocator, &decl_list, &own_decl_list, &seen_in_list, ns_name, imported_mod, decl.span);
|
||||
} else {
|
||||
try mod.mergeFlat(allocator, &decl_list, &seen_in_list, imported_mod);
|
||||
try mod.mergeFlat(allocator, &decl_list, &seen_in_list, &seen_nodes, imported_mod);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -622,6 +634,7 @@ fn resolveDirectoryImport(
|
||||
var decl_list = std.ArrayList(*Node).empty;
|
||||
var own_decl_list = std.ArrayList(*Node).empty;
|
||||
var seen_in_list = std.StringHashMap(void).init(allocator);
|
||||
var seen_nodes = std.AutoHashMap(*Node, void).init(allocator);
|
||||
|
||||
for (file_names.items) |file_name| {
|
||||
const file_path = try std.fmt.allocPrint(allocator, "{s}/{s}", .{ dir_path, file_name });
|
||||
@@ -668,10 +681,12 @@ fn resolveDirectoryImport(
|
||||
// register `Event` (a tagged_union) before `handle_event(e: *Event)`
|
||||
// triggers the placeholder-struct fallback in `resolveTypeName`.
|
||||
for (file_mod.decls) |decl| {
|
||||
if (seen_nodes.contains(decl)) continue;
|
||||
if (decl.data.declName()) |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);
|
||||
}
|
||||
// Separately track which decls the directory `re-exports` to its
|
||||
|
||||
1
tests/expected/issue-0056-diamond-param-impl.exit
Normal file
1
tests/expected/issue-0056-diamond-param-impl.exit
Normal file
@@ -0,0 +1 @@
|
||||
0
|
||||
1
tests/expected/issue-0056-diamond-param-impl.txt
Normal file
1
tests/expected/issue-0056-diamond-param-impl.txt
Normal file
@@ -0,0 +1 @@
|
||||
7
|
||||
Reference in New Issue
Block a user