From 3789add496e99755c353f8366409030271b69747 Mon Sep 17 00:00:00 2001 From: Nitjsefnie Date: Sun, 19 Jul 2026 09:40:53 +0000 Subject: [PATCH] fix(import): isolate a module's top-level bare assignment to its own scope (#589) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 #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 #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 --- docs/SPEC.md | 11 ++++-- src/compiler.c | 9 ++++- src/eigenscript.h | 9 +++++ src/vm.c | 9 +++++ tests/run_all_tests.sh | 1 + tests/test_import_toplevel_scope.eigs | 48 +++++++++++++++++++++++++++ 6 files changed, 84 insertions(+), 3 deletions(-) create mode 100644 tests/test_import_toplevel_scope.eigs diff --git a/docs/SPEC.md b/docs/SPEC.md index ee1d65e..de53bb5 100644 --- a/docs/SPEC.md +++ b/docs/SPEC.md @@ -1032,8 +1032,15 @@ can read the loader's globals and call its functions, but they can never bind a write through to them — a bare `name is expr` inside a module function that doesn't refer to a local, a captured name, or the module's own top-level state creates a fresh local, regardless of what -happens to exist in the loader's scope. (Top-level statements in the -loaded file still execute directly in the current scope.) To share +happens to exist in the loader's scope. The same boundary applies one +level up to an **imported** module's own top-level statements: a bare +`name is expr` at an imported file's top level binds in *that module's +own scope*, never walking through to a same-named binding the importer +happens to already have — `counter is 0` at a module's top level can +never rebind an importer's pre-existing `counter`. `load_file` is the +one exception, per its older, documented contract above: its top-level +statements still execute directly in the current (caller's) scope, so +a same-named top-level assignment there *does* bind through. To share mutable state across files, put it in a dict or list and mutate fields — reads cross the boundary and field/index writes are value mutations, not bindings. The standard library's UI toolkit (`lib/ui.eigs`'s `_ui` diff --git a/src/compiler.c b/src/compiler.c index 87bf62d..50aa9fa 100644 --- a/src/compiler.c +++ b/src/compiler.c @@ -1617,7 +1617,14 @@ static void emit_assign_for_tos(Compiler *c, const char *name, uint32_t name_has } if (!picked) { int idx = add_string_constant(c, name); - if (local_only) { + /* #589: at an IMPORTED module's own top level, a bare write + * must never walk past mod_env to rebind whatever the + * importer's scope already holds under the same name — + * mirrors #373's function-body rule one level up. load_file + * doesn't set g_compile_import_toplevel, so its top-level + * writes keep walking the chain into the caller's scope, + * per its documented "current scope" contract. */ + if (local_only || g_compile_import_toplevel) { set_op = OP_SET_NAME_LOCAL; set_arg = (uint16_t)idx; } else { set_op = OP_SET_NAME; set_arg = (uint16_t)idx; diff --git a/src/eigenscript.h b/src/eigenscript.h index de5d73a..3954093 100644 --- a/src/eigenscript.h +++ b/src/eigenscript.h @@ -643,6 +643,14 @@ struct EigsThread { * global that happened to exist at compile time — the write * compiles as a fresh local instead. Reads stay dynamic. */ int compile_module_boundary; + /* #589: nonzero ONLY while compiling an `import`ed module's own + * top-level statements (never for load_file, whose documented + * contract is "runs directly in the current scope"). A bare + * `name is expr` at that top level must resolve/create in the + * module's own fresh env (mod_env) and never walk through to + * whatever the importer's scope happens to already bind — the + * top-level counterpart of #373's function-body boundary. */ + int compile_import_toplevel; /* Per-import resolution base (Phase 0b). Empty = fall back to * state->script_dir. OP_IMPORT saves/restores around module body. */ char import_resolve_dir[4096]; @@ -731,6 +739,7 @@ extern __thread EigsThread *eigs_current; #define g_exe_dir (eigs_current->state->exe_dir) #define g_load_env (eigs_current->load_env) #define g_compile_module_boundary (eigs_current->compile_module_boundary) +#define g_compile_import_toplevel (eigs_current->compile_import_toplevel) #define g_import_resolve_dir (eigs_current->import_resolve_dir) #define g_vm_multithreaded (eigs_current->state->multithreaded) #define g_gc_envs (eigs_current->state->gc_envs) diff --git a/src/vm.c b/src/vm.c index 8baf962..ac30ebe 100644 --- a/src/vm.c +++ b/src/vm.c @@ -4941,8 +4941,17 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume) { int saved_boundary = g_compile_module_boundary; g_compile_module_boundary = 1; /* #373 */ + /* #589: this module's OWN top-level statements compile here too + * (mod_env's parent is g_global_env — the importer's scope). + * Mark it so a bare top-level `name is expr` binds/updates only + * within mod_env and never walks through to the importer's + * global — load_file does NOT set this (its documented contract + * keeps top-level writes in the caller's current scope). */ + int saved_import_toplevel = g_compile_import_toplevel; + g_compile_import_toplevel = 1; EigsChunk *mod_chunk = compile_ast(ast, mod_env, source); g_compile_module_boundary = saved_boundary; + g_compile_import_toplevel = saved_import_toplevel; if (g_parse_errors > 0) { g_parse_errors = saved_errors; chunk_free(mod_chunk); diff --git a/tests/run_all_tests.sh b/tests/run_all_tests.sh index 09aadc3..2017d35 100755 --- a/tests/run_all_tests.sh +++ b/tests/run_all_tests.sh @@ -1709,6 +1709,7 @@ echo "" echo "[43a2] Builtin Argument Errors (26 checks)" check_eigs_suite "builtin argument errors" test_builtin_errors.eigs "All builtin_errors tests passed" 30 check_eigs_suite "module-boundary write insulation (#373)" test_module_scope.eigs "All module-scope tests passed" 9 +check_eigs_suite "import top-level scope insulation vs load_file current-scope contract (#589)" test_import_toplevel_scope.eigs "All import top-level scope tests passed" 11 echo "" # [43a3] EigenStore header-validation / corruption error paths (ext_store.c) diff --git a/tests/test_import_toplevel_scope.eigs b/tests/test_import_toplevel_scope.eigs new file mode 100644 index 0000000..1d08d30 --- /dev/null +++ b/tests/test_import_toplevel_scope.eigs @@ -0,0 +1,48 @@ +# #589: import's own top-level bare assignment must bind in the imported +# module's own scope, never walking through to a same-named binding that +# happens to already exist in the importer's scope — the top-level +# counterpart of #373's function-write boundary (test_module_scope.eigs). +# Before the fix, a module's `counter is 0` at its own top level rebound the +# IMPORTER's `counter` instead of creating module state, and every later use +# of `counter` inside the module's functions then resolved to the importer's +# variable (the exact cross-wire from the issue's minimal repro). +# +# load_file keeps its OLDER, documented "runs directly in the current scope" +# contract (docs/SPEC.md) completely unchanged — this file proves both +# directions explicitly, since a regression in either one is the failure +# mode #589 warns about. + +# --- Case 1 (import): module top-level state is insulated from a +# same-named importer variable, both at import time and across repeated +# calls into the module's own functions. --- +write_text of ["tmp_589_cmod.eigs", "counter is 0\ndefine bump() as:\n counter is counter + 1\n return counter\n"] + +counter is [9, 9, 9] +import tmp_589_cmod +assert of [(type of counter) == "list", "I1 importer's counter untouched by module top-level init"] +assert of [counter == [9, 9, 9], "I2 importer's counter value unchanged by import"] +assert of [(tmp_589_cmod.bump of null) == 1, "I3 module state starts at 1 in its own scope"] +assert of [(tmp_589_cmod.bump of null) == 2, "I4 module state accumulates in its own scope"] +assert of [(tmp_589_cmod.bump of null) == 3, "I5 module state keeps accumulating, still module-local"] +assert of [(type of counter) == "list", "I6 importer's counter still untouched after repeated module calls"] +assert of [counter == [9, 9, 9], "I7 importer's counter value still unchanged"] + +remove_file of "tmp_589_cmod.eigs" + +# --- Case 2 (load_file): documented contract is UNCHANGED — a load_file'd +# file's top-level statements still execute directly in the CURRENT +# (caller's) scope, so a same-named top-level assignment DOES bind through +# and DOES clobber the caller's existing binding. This is load_file's older, +# intentional semantics, not the #589 bug, and the fix must not touch it. --- +write_text of ["tmp_589_lfmod.eigs", "counterLF is 0\ndefine bumpLF() as:\n counterLF is counterLF + 1\n return counterLF\n"] + +counterLF is [9, 9, 9] +load_file of "tmp_589_lfmod.eigs" +assert of [(type of counterLF) == "num", "L1 load_file's top-level init binds through to the current scope"] +assert of [counterLF == 0, "L2 current-scope counterLF now holds the loaded file's value"] +assert of [(bumpLF of null) == 1, "L3 bumpLF reads/writes the (now shared) current-scope binding"] +assert of [counterLF == 1, "L4 the write is visible in the current scope too — same binding"] + +remove_file of "tmp_589_lfmod.eigs" + +print of "All import top-level scope tests passed"