Attempted the canonicalize-path fix (realpath + cwd-relativization for display); it fixes the e2e absolute-entry duplication but ripples path-identity changes into ~8 import-subsystem unit tests + 2 cosmetic snapshots. Reverted as too broad for a drive-by rework; documented a minimal repro and a recommended deliberate approach (lexical normalize, single chokepoint, batch test updates). Issue stays OPEN.
5.8 KiB
0148 — absolute entry path duplicates transitively-imported types
Summary
When sx build is given an absolute path to the entry .sx file, a type
declared in a module that is reached by two different #import chains can be
duplicated into two distinct type identities. A later use of that type then
fails with type 'T' is not visible; #import the module that declares it, even
though the importing file does #import the declaring module. Building the same
entry by a cwd-relative path (from the same directory) resolves every import
to a single module instance and compiles cleanly.
Repro (photo project)
From the project root /Users/agra/projects/photo:
# RELATIVE entry — OK
sx build main.sx -o /tmp/m # RC=0, bundles + compiles
# ABSOLUTE entry — FAILS
sx build /Users/agra/projects/photo/main.sx -o /tmp/m
# error: type 'Layer' is not visible; #import the module that declares it
# --> ui/toolbar.sx:133:8 (a plain `hover_tool: i64;` field line)
main.sx imports doc/document.sx (which re-exports Layer from
doc/layer.sx) directly, AND via ui/toolbar.sx / ui/canvas_view.sx, AND it
also does the directory-module import #import "modules/ui". Under the absolute
entry path, doc/layer.sx's Layer ends up with two identities, so
layer_at()'s return type seen inside ui/toolbar.sx no longer matches and the
struct that uses it fails to typecheck (the error points at an unrelated field
line in the struct, the first member checked).
A test that imports BOTH ui/toolbar.sx and ui/canvas_view.sx
(tests/canvas_select.sx) builds fine by absolute path — so the trigger is not
the two-chain overlap alone; it requires the additional directory-module import
(modules/ui) present in main.sx. The duplication is therefore in how the
entry's absolute path is normalized/keyed against the module-search-path
resolution of a directory import.
Expected
Import resolution must canonicalize paths so the same source file is one module instance regardless of whether the entry was named by an absolute or a relative path. An absolute entry should compile identically to the relative one.
Workaround (in repo)
tools/sx_build.sh (the lock wrapper) now rewrites an entry path that lives
under the project root into a root-relative path and runs the compiler from the
root, so both main.sx and /abs/.../main.sx build to the same green result.
Non-entry arguments (e.g. -o /tmp/out) are left untouched.
Investigation findings (attempted fix, reverted — STILL OPEN)
Confirmed the root cause and a working mechanism, but the straightforward implementation has too broad a blast radius to land safely in one pass:
- Root cause confirmed. Import resolution keys the module cache and the
flat-import graph on the literal resolved-path string with no canonicalization.
An absolute entry makes
main's direct imports key absolutely while a sibling's transitive import of the same file falls back to a cwd-relative spelling (root_pathis passednullin the import walk) → two cache keys → two type identities → "not visible". - Minimal repro (no photo project): a deeper type is required —
doc/layer.sxdefines a type,doc/document.sximports it and references it, andmainreachesdocument.sxBOTH directly and transitively (viaui/toolbar.sx). Per the report the directory-module import inmainis also needed to trip the duplication. (PlainLayerdefined directly indocument.sx, used asdoc.Layer, does NOT reproduce.) - Mechanism that works: a
canonicalizePath(libcrealpath) applied to every resolved import path unifies the spellings. To avoid leaking absolute, machine-specific paths into diagnostics (which would break ~160.stderrsnapshots), re-relativize the realpath result against CWD when the file lives under it — yielding ONE canonical spelling that is also the cwd-relative display form. This was verified to fix the end-to-end repro (absolute and relative entries both compile) while keeping diagnostics relative. - Why it was reverted: canonicalizing the resolved path changes the path
identity used as a KEY across the whole import subsystem — module cache,
flat_import_graph,module_decls, the decl table, and namespace-author resolution. That rippled into ~8 unit tests (e.g.buildImportFactsnamespaced-target,buildDeclTablekeying,module_declsretention,collectNamespaceAuthors, shadowed-author lowering) that hard-code absoluteabsdir-based path expectations, plus 2 cosmetic corpus snapshots (examples/./XXXX→examples/XXXX). Each is individually a legitimate canonical-form update, but the breadth — and the risk of subtly changing import-graph identity matching — is more than a drive-by rework should absorb.
Recommended approach for the fix session
- Decide the canonical scheme deliberately: lexical normalize (strip
./, collapsea/../b) may suffice to unify the abs-vs-rel entry split without the symlink/relativization churn realpath introduces — and changes fewer keys. If realpath is needed, keep the cwd-relativization for display. - Apply canonicalization at a single chokepoint in
resolveImportPath(+ the directory-import per-file path) so every consumer sees one spelling. - Update ALL path-keyed unit tests in
src/imports.test.zig(and their.lower/ir.resolvertests) to compare againstcanonicalizePath(expected)rather than the rawabsdir-joined path; regenerate the 2 cosmetic snapshots (0410-protocols-impl-visibility,0411-protocols-impl-duplicate). - Add a
canonicalizePathunit test asserting abs + cwd-relative + redundant./spellings of one file unify (the e2e absolute-entry case can't be a corpus example — the runner only invokes relative paths).