Skip to content

fix(import): isolate module top-level bare assignments in module scope (fixes #589)#673

Merged
InauguralPhysicist merged 1 commit into
InauguralSystems:mainfrom
Nitjsefnie-OSC:fix-589-module-scope-binding
Jul 19, 2026
Merged

fix(import): isolate module top-level bare assignments in module scope (fixes #589)#673
InauguralPhysicist merged 1 commit into
InauguralSystems:mainfrom
Nitjsefnie-OSC:fix-589-module-scope-binding

Conversation

@Nitjsefnie

Copy link
Copy Markdown
Contributor

Fixes #589.

A module's top-level bare assignment leaked past the module boundary on import: with counter is [9,9,9] in the importer and counter is 0 at an imported module's top level, the module init rebound the importer's counter, and the module's functions then resolved counter to 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 flag g_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 #373 compile_module_boundary flag.
  • src/vm.c (OP_IMPORT) — sets the flag only around compile_ast(ast, mod_env, source) for the imported module's own top level, saved/restored before vm_execute so nested imports are correctly bracketed.
  • src/compiler.c (emit_assign_for_tos, top-level branch, enclosing == NULL) — a bare top-level name is expr emits OP_SET_NAME_LOCAL (bind/update strictly in the current env, no parent-chain walk) when the flag is set, instead of OP_SET_NAME (which walked the parent chain up to the importer's global).

load_file is deliberately untouched. builtin_load_file executes 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 share compile_ast/emit_assign_for_tos and the #373 function-boundary flag, but the new flag is exclusive to import's top-level compile.

Behavior

# cmod.eigs:  counter is 0 ; define bump() as: counter is counter + 1 / return counter
# main.eigs:  counter is [9,9,9] ; import cmod
print of (type of counter)   # before: num   after: list
print of (cmod.bump of [])   # 1  (now from MODULE scope; repeated calls -> 1,2,3)
print of (type of counter)   # before: num   after: list

The load_file equivalent 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_file as 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, 4 load_file-unchanged).
  • Suite 3102/3102 (baseline main 3091, +11 new); ASan+UBSan 3106/3106. Under umask 022, rebuilt after each change.
  • Mutation: reverting only the three source files (keeping the test) rebuilds and fails at exactly 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 #373 test_module_scope.eigs and the load_file assertions still pass.

Generated by Claude Opus 4.8 (brief, review), Claude Sonnet 5 (implementation)

…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>
@InauguralPhysicist

Copy link
Copy Markdown
Collaborator

Reviewed and merging. The fix is correctly scoped: the flag is set only in OP_IMPORT, saved/restored tightly around compile_ast, and deliberately not set for load_file — preserving that older documented contract instead of quietly unifying the two. SPEC.md updated in the same PR, and the test asserts both directions rather than only the fixed one.

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 counter before importing; this is the reverse:

import mod_a          # module does `counter is 0` at its top level
counter is [9, 9, 9]  # importer assigns AFTER
module state: 1,2 (expect 1,2)   importer counter: [9,9,9] (expect [9,9,9])

Isolated in both directions.

2. load_file nested inside an imported module — the flag is compile-time and restored before the module body executes, so a runtime load_file should still get current-scope semantics. Confirmed: a module whose top level does load_file of "lf_target.eigs" sees shared_lf=42.

3. Nested imports — module A imports B, both with a top-level tag, importer also has one:

outer=outer  inner=inner  main=main

Three levels, all separate.

Full suite on this branch: 3102/3102, with both [#373] and the new [#589] sections green.

Thanks — the two-direction test is more than most PRs carry, and documenting the load_file exception in SPEC rather than silently changing it is the right call.

@InauguralPhysicist
InauguralPhysicist merged commit dc0d31d into InauguralSystems:main Jul 19, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

import: module TOP-LEVEL assignment binds through to a same-named importer global (module state hijacked)

2 participants