diff --git a/src/module_resolver.cpp b/src/module_resolver.cpp index ce082ae..5ca3324 100644 --- a/src/module_resolver.cpp +++ b/src/module_resolver.cpp @@ -147,6 +147,44 @@ std::string ModuleResolver::resolve(const std::string &importPath) const { return ""; // Not found } +std::string +ModuleResolver::moduleIdentity(const std::string &resolvedFile) const { + // Map a resolved (canonical) module file to the stable identity used + // as both the `loadedModules` cache key and the `modulePath` + // mangling prefix: the path relative to the entry base dir, with the + // `.jam` extension stripped and forward slashes. It is the + // project-root-relative path that names a module regardless of which + // relative spelling reached it, so `import("lib/b")` and + // `import("./b")` from `lib/a` agree on the identity `lib/b`. + // + // Returns "" when the file sits outside the base dir — std-library + // modules resolve under their own root, and the caller keeps the + // original import spelling, which `resolve` already maps through the + // std root. + std::error_code ec; + fs::path baseAbs = fs::canonical(baseDir, ec); + if (ec) return ""; + fs::path rel = fs::relative(resolvedFile, baseAbs, ec); + if (ec || rel.empty()) return ""; + std::string id = rel.generic_string(); + if (id.rfind("..", 0) == 0) return ""; // escapes the base dir + static const std::string ext = ".jam"; + if (id.size() > ext.size() && + id.compare(id.size() - ext.size(), ext.size(), ext) == 0) { + id.resize(id.size() - ext.size()); + } + // A `/mod.jam` index file shares its directory's identity, so + // `import("foo")` and `import("foo/mod")` name one module — matching + // `resolve`, which maps a bare `import("foo")` to `foo/mod.jam`. + static const std::string modSuffix = "/mod"; + if (id.size() > modSuffix.size() && + id.compare(id.size() - modSuffix.size(), modSuffix.size(), + modSuffix) == 0) { + id.resize(id.size() - modSuffix.size()); + } + return id; +} + std::string ModuleResolver::readFile(const std::string &path) const { std::ifstream file(path); if (!file.is_open()) { return ""; } @@ -198,11 +236,10 @@ ModuleAST *ModuleResolver::getOrLoadModule(const std::string &importPath) { } // Register the parsed module in the cache BEFORE recursing into its - // imports. Mirrors Zig's Module.importFile (Module.zig:4946 — - // import_table.getOrPut returns the cached File* as soon as it - // exists, no cycle check). A cyclic import (`bus.jam` imports - // `dma.jam` imports `bus.jam`) now hits the cache and returns this - // same partially-initialised ModuleAST instead of erroring. The + // imports: the entry is published as soon as the module exists, with + // no cycle check. A cyclic import (`bus.jam` imports `dma.jam` + // imports `bus.jam`) then hits the cache and returns this same + // partially-initialised ModuleAST instead of erroring. The // post-parse passes below (loadNested + module-path stamping) mutate // the module in place — by the time codegen / semantic analysis // touches a cyclic-import target, it's complete. @@ -210,29 +247,42 @@ ModuleAST *ModuleResolver::getOrLoadModule(const std::string &importPath) { loadedModules[importPath] = std::move(module); // Recursively load both regular imports (`const x = import(...)`) - // and destructuring imports (`const { X } = import(...)`). The - // nested resolver runs in the source module's directory so relative - // paths work; we then forward to the top-level `getOrLoadModule` so - // every resolved module ends up in the shared `loadedModules` map - // and gets its `pub` symbols registered by main.cpp. - auto loadNested = [&](const std::string &importPath) { + // and destructuring imports (`const { X } = import(...)`). + // + // Each nested import is resolved against THIS module's directory, so + // a relative `./b` in `lib/a.jam` means `lib/b.jam` — not a same- + // named file beside the entry module. We then rewrite the import's + // path in place to its canonical entry-relative identity (see + // `moduleIdentity`) and recurse on that. Together these resolve each + // import against the importing file's directory and key the cache by + // the resolved identity, so the same file reached via different + // spellings is one module. + // + // The rewrite is load-bearing: without it `lib/b.jam` imported as + // both `lib/b` (from the entry) and `./b` (from `lib/a`) would key + // the cache under two strings, load twice, and register its `pub` + // types twice — colliding in the global by-name type registry that + // `main.cpp` builds. Collapsing to one identity dedupes the module + // and keeps `modulePath` (the mangling prefix) stable. + auto loadNested = [&](std::string &importPath) { if (importPath == "test") return; - fs::path modulePath(resolvedPath); - std::string moduleDir = modulePath.parent_path().string(); - ModuleResolver nestedResolver(moduleDir, *typePool, *stringPool, - *nodeStore); + fs::path moduleDir = fs::path(resolvedPath).parent_path(); + ModuleResolver nestedResolver(moduleDir.string(), *typePool, + *stringPool, *nodeStore); std::string nestedResolved = nestedResolver.resolve(importPath); - if (!nestedResolved.empty() && nestedResolved != "test") { - getOrLoadModule(importPath); - } + if (nestedResolved.empty() || nestedResolved == "test") return; + std::string id = moduleIdentity(nestedResolved); + if (!id.empty()) importPath = id; + getOrLoadModule(importPath); }; - for (const auto &import : modPtr->Imports) { loadNested(import->Path); } - for (const auto &destImport : modPtr->DestructuringImports) { + for (auto &import : modPtr->Imports) { loadNested(import->Path); } + for (auto &destImport : modPtr->DestructuringImports) { loadNested(destImport->Path); } // Stamp the import path on every function/method so the mangler - // can build Zig-style FQNs (`timer.Timer.read32`). Without this, + // can build dotted fully-qualified names (`timer.Timer.read32`). + // Without this, // two modules that both define `pub fn helper()` or `pub const // Counter = struct { pub fn init() }` would emit the same LLVM // symbol and the linker would silently merge them. diff --git a/src/module_resolver.h b/src/module_resolver.h index 34f158f..02ead32 100644 --- a/src/module_resolver.h +++ b/src/module_resolver.h @@ -51,6 +51,11 @@ class ModuleResolver { std::vector> *sharedAnonEnums_ = nullptr; std::unordered_map> loadedModules; + // Canonical entry-relative identity for a resolved module file, used + // as the cache key and `modulePath` prefix. Empty when the file lies + // outside the base dir (e.g. std-library modules). + std::string moduleIdentity(const std::string &resolvedFile) const; + std::string readFile(const std::string &path) const; std::unique_ptr parseSource(const std::string &source) const; diff --git a/src/parser.cpp b/src/parser.cpp index fecb8c2..b514550 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -177,7 +177,9 @@ NodeIdx Parser::parsePrimary() { // `match (…) { … }` is also valid in expression position so it can // produce a value. The same call works for both statement and // expression forms; the codegen builds a phi over arm values. - if (check(TOK_MATCH)) { return parseMatch(); } + if (check(TOK_MATCH)) { + return parseMatch(); + } // `@name(arg, ...)` — compiler intrinsic invocation. Two encoding // shapes: diff --git a/tests/unit/import_resolution/leaf.jam b/tests/unit/import_resolution/leaf.jam new file mode 100644 index 0000000..9582d8c --- /dev/null +++ b/tests/unit/import_resolution/leaf.jam @@ -0,0 +1,19 @@ +// Root-level shadow. Shares the basename `leaf` with +// nested/leaf.jam and sits beside the entry test file. A relative +// `./leaf` from inside nested/ must NOT land here. Its `Thing` carries +// an extra `onlyInRoot` field so that, if a relative import ever +// mis-resolves to this file while nested/leaf.jam is also loaded, the +// two `pub Thing`s collide in the global by-name type registry and the +// build fails loudly — the original repro's symptom. +pub const Thing = struct { + value: u64, + onlyInRoot: bool, +}; + +pub fn makeThing() Thing { + return Thing { value: 1, onlyInRoot: true }; +} + +pub fn leafValue() u64 { + return 1; +} diff --git a/tests/unit/import_resolution/nested/deep/via_dotdot.jam b/tests/unit/import_resolution/nested/deep/via_dotdot.jam new file mode 100644 index 0000000..c486f0d --- /dev/null +++ b/tests/unit/import_resolution/nested/deep/via_dotdot.jam @@ -0,0 +1,8 @@ +// `../leaf` climbs from nested/deep/ to nested/leaf.jam — the same +// file via_dot.jam reaches as `./leaf`. Exercises parent-relative +// resolution and dedup against a different spelling of one file. +const { leafValue } = import("../leaf"); + +pub fn fromDotDot() u64 { + return leafValue(); +} diff --git a/tests/unit/import_resolution/nested/leaf.jam b/tests/unit/import_resolution/nested/leaf.jam new file mode 100644 index 0000000..e9c6c18 --- /dev/null +++ b/tests/unit/import_resolution/nested/leaf.jam @@ -0,0 +1,16 @@ +// The correct target of `./leaf` (from nested/via_dot.jam) and +// `../leaf` (from nested/deep/via_dotdot.jam), and of the entry's +// `import("nested/leaf")`. All three spellings name this one file, so +// it must load exactly once. Returns 7 to distinguish it from the +// root-level shadow (which returns 1). +pub const Thing = struct { + value: u64, +}; + +pub fn makeThing() Thing { + return Thing { value: 7 }; +} + +pub fn leafValue() u64 { + return 7; +} diff --git a/tests/unit/import_resolution/nested/via_dot.jam b/tests/unit/import_resolution/nested/via_dot.jam new file mode 100644 index 0000000..ebf894e --- /dev/null +++ b/tests/unit/import_resolution/nested/via_dot.jam @@ -0,0 +1,8 @@ +// `./leaf` is relative to THIS file's directory (nested/), so it +// resolves to nested/leaf.jam — not the same-named leaf.jam beside the +// entry test file. +const { leafValue } = import("./leaf"); + +pub fn fromDot() u64 { + return leafValue(); +} diff --git a/tests/unit/import_resolution/nested/via_pkg.jam b/tests/unit/import_resolution/nested/via_pkg.jam new file mode 100644 index 0000000..601cea8 --- /dev/null +++ b/tests/unit/import_resolution/nested/via_pkg.jam @@ -0,0 +1,7 @@ +// `../pkg` from nested/ resolves to the directory-index module +// pkg/mod.jam, exercising relative resolution into a directory module. +const { pkgValue } = import("../pkg"); + +pub fn fromPkg() u64 { + return pkgValue(); +} diff --git a/tests/unit/import_resolution/pkg/mod.jam b/tests/unit/import_resolution/pkg/mod.jam new file mode 100644 index 0000000..57a0dc3 --- /dev/null +++ b/tests/unit/import_resolution/pkg/mod.jam @@ -0,0 +1,6 @@ +// Directory-index module: `import("pkg")` resolves here (pkg/mod.jam), +// as does `import("../pkg")` from nested/. Its identity collapses to +// `pkg` either way, so both spellings name one module. +pub fn pkgValue() u64 { + return 42; +} diff --git a/tests/unit/import_resolution/test_relative_imports.jam b/tests/unit/import_resolution/test_relative_imports.jam new file mode 100644 index 0000000..92bad8f --- /dev/null +++ b/tests/unit/import_resolution/test_relative_imports.jam @@ -0,0 +1,51 @@ +// Relative-import resolution regression test. +// +// Mirrors github.com/olup/jam-import-resolution-repro: a relative +// import from a nested module must resolve against that module's own +// directory, not the entry module's. Before the fix, `./leaf` inside +// nested/via_dot.jam wrongly resolved to leaf.jam beside this entry +// file, dragging the root-level shadow into the build alongside +// nested/leaf.jam; their same-named `pub Thing`s then collided in the +// global by-name type registry and the build failed with +// `struct literal missing field onlyInRoot`. +// +// The resolver now keys modules by their project-root-relative +// identity, so `./leaf`, `../leaf`, and `nested/leaf` all name +// nested/leaf.jam — one module, no collision. + +const { assert } = import("test"); +const dot = import("nested/via_dot"); +const deep = import("nested/deep/via_dotdot"); +const { Thing, makeThing } = import("nested/leaf"); +const pkg = import("pkg"); +const viaPkg = import("nested/via_pkg"); + +// `./leaf` from nested/via_dot.jam means nested/leaf.jam (value 7), +// not the same-named leaf.jam beside this file (value 1). +tfn relativeDotResolvesToNestedSibling() { + assert(dot.fromDot(), 7); +} + +// `../leaf` from nested/deep/via_dotdot.jam climbs one directory to +// reach nested/leaf.jam. +tfn relativeParentResolvesAcrossDir() { + assert(deep.fromDotDot(), 7); +} + +// nested/leaf.jam is reached three ways — `./leaf`, `../leaf`, and +// `nested/leaf` — but is one module. Its `pub Thing` therefore +// registers once and its own literal type-checks against its own +// fields. A double-load would re-trigger the global-registry collision +// and fail to compile. +tfn sameFileViaDistinctSpellingsIsOneModule() { + var t: Thing = makeThing(); + assert(t.value, 7); +} + +// A directory-index module (pkg/mod.jam) imported as `pkg` from the +// entry and as `../pkg` from nested/ collapses to one identity, so +// relative resolution reaches directory modules too. +tfn directoryModuleResolvesByDirAndRelatively() { + assert(pkg.pkgValue(), 42); + assert(viaPkg.fromPkg(), 42); +}