fix(import): isolate module top-level bare assignments in module scope (fixes #589)#673
Conversation
…scope (InauguralSystems#589) An imported module's top-level `name is expr` was walking through mod_env's parent chain and rebinding a same-named global in the IMPORTER's scope instead of creating module state, so the module's own functions (and any later use in the importer) resolved to the wrong binding. This is the top-level counterpart of InauguralSystems#373's function-write boundary, applied one level up: only `import`'s own compile of a module's top-level statements now forces OP_SET_NAME_LOCAL (bind/update strictly within mod_env), gated by a new compile-time flag (g_compile_import_toplevel) set only around import's compile_ast call. `load_file` is untouched — it shares the InauguralSystems#373 function-boundary flag but never sets the new one, so its documented "top-level statements execute directly in the current scope" contract is unchanged. Adds tests/test_import_toplevel_scope.eigs proving both directions: import isolates module top-level state (including across repeated calls into the module's own functions) while load_file keeps clobbering same-named current-scope bindings as documented. Updates docs/SPEC.md's "Module write boundary" section to state the import top-level rule explicitly and disambiguate the load_file exception. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Reviewed and merging. The fix is correctly scoped: the flag is set only in I probed three interactions the test doesn't reach, on a local build of this branch: 1. Load order — the one I most wanted checked, since #373's bug in this same area was load-order-dependent. The test always assigns Isolated in both directions. 2. 3. Nested imports — module A imports B, both with a top-level Three levels, all separate. Full suite on this branch: 3102/3102, with both Thanks — the two-direction test is more than most PRs carry, and documenting the |
Fixes #589.
A module's top-level bare assignment leaked past the module boundary on
import: withcounter is [9,9,9]in the importer andcounter is 0at an imported module's top level, the module init rebound the importer'scounter, and the module's functions then resolvedcounterto the importer's variable — silently cross-wiring two unrelated states whenever they share a name.Fix
Mirrors the #373 "Module write boundary" rule one level up, applied only to
import:src/eigenscript.h— new per-thread flagg_compile_import_toplevel, next to the existing load_file scope asymmetry: a lib function can write a caller global declared BEFORE the load, but not one declared after #373compile_module_boundaryflag.src/vm.c(OP_IMPORT) — sets the flag only aroundcompile_ast(ast, mod_env, source)for the imported module's own top level, saved/restored beforevm_executeso nested imports are correctly bracketed.src/compiler.c(emit_assign_for_tos, top-level branch,enclosing == NULL) — a bare top-levelname is expremitsOP_SET_NAME_LOCAL(bind/update strictly in the current env, no parent-chain walk) when the flag is set, instead ofOP_SET_NAME(which walked the parent chain up to the importer's global).load_fileis deliberately untouched.builtin_load_fileexecutes its file's top level directly against the caller's scope and never sets this flag, so its top-level assignments still bind through — matching its documented "runs directly in the current scope" contract. The two paths sharecompile_ast/emit_assign_for_tosand the #373 function-boundary flag, but the new flag is exclusive toimport's top-level compile.Behavior
The
load_fileequivalent is unchanged (top-level init still binds through to the caller).SPEC
Added a sentence to the "Module write boundary" section stating the import top-level rule and naming
load_fileas the one exception (prose only — no runnable example block touched, so the byte-for-byte doc-example test stays green). Happy to adjust the wording if you prefer.Verification
tests/test_import_toplevel_scope.eigs(new, 11 assertions: 7 import-isolation incl. nested-import / import-in-function / read-not-write / self-update, 4load_file-unchanged).umask 022, rebuilt after each change.I1 importer's counter untouched by module top-level init, while the load_file scope asymmetry: a lib function can write a caller global declared BEFORE the load, but not one declared after #373test_module_scope.eigsand theload_fileassertions still pass.Generated by Claude Opus 4.8 (brief, review), Claude Sonnet 5 (implementation)