fix(compiler): listcomp/catch name and a same-named assignment share one binding (#633)

[Nitjsefnie]

Closes #633.

Root cause (confirmed against the code — your diagnosis was exactly right, just a few lines off current main): a comprehension variable and a catch name bind by name into the function env (OP_SET_NAME_LOCAL, the listcomp/catch binders ~L2553/L2599), while a plain x is … of the same name is slot-promoted in emit_assign_for_tos (add_local → OP_SET_LOCAL). The two bindings diverge — the env holds one i, the slot keeps a stale value, and a slot read returns the pre-comprehension value.

The local_eligible predicate keyed only on captured, so adding any closure/interrogative that merely mentions the name anywhere in the body — even dead, never-executed code — flipped the plain assignment onto the env path and silently changed an earlier line's output (rc stayed 0). That's the "later dead code changes an earlier line's result" symptom in the report.

Fix (follows the issue's sketch, and mirrors the exclusion collect_module_names_walk already applies to for-loop vars): a new scan_for_env_bound collects listcomp vars and catch names into a per-scope NameSet env_bound (populated in the AST_FUNC/AST_LAMBDA compile paths, freed in both teardowns). emit_assign_for_tos now excludes those names from slot promotion (&& !env_bound on local_eligible) and routes them to OP_SET_FN_NAME_LOCAL (|| env_bound), so the plain assignment writes the same binding the comprehension/catch reads.

Repro (issue's pairs, via ./src/eigenscript): y1 100→2 before/2 after; the dead-lambda variant y2 already gave 2 and still does; the eval double-bind and the catch pairs likewise now agree. Regression tests SS16–SS19 added to tests/test_scope_semantics.eigs.

Verification: build.sh clean; tests/run_all_tests.sh 2890/2890 on both main and this branch; make asan (ASan+UBSan) clean over the scope/closure tests (the new NameSet alloc/free is leak/UB-free). The new SS16 test bites — reverting compiler.c to main while keeping the tests fails SS16 listcomp var leaks over an earlier plain assignment, restored it passes.

One subtlety for the test file (documented in the diff): a name already bound at module scope masks the bug inside a function (it classifies as write-through-global, never slot-promoted), so the regressions use function-unique names.

Disclosure: this contribution was made with AI assistance (Claude), built + full-suite + ASan verified and mutation-tested before submission.

[InauguralPhysicist]

Merging — this is excellent work, and on the hardest thing in the repo to touch.

It fixes more than you claimed. It also closes #642, which you didn't mention and may not have seen: a comprehension whose loop var shadows a local was corrupting its own body, not just the read after it —

define f() as:
    i is 0 - 1
    return [i * 2 for i in [1, 2, 3]]

[-2, -2, -2] on main → [2, 4, 6] on your branch. Same root cause, worse symptom: #633 needs a closure to manifest and corrupts the later read; #642 needs nothing and corrupts every element of the result. Your fix handles both because de-promoting the name fixes the read side too. I'll add a regression test for that shape as a follow-up — SS16–SS19 all pin the read after the comprehension, so a future refactor could reintroduce #642 with all four still green.

Your mutation test is honest — I checked it independently. Reverting only src/compiler.c to main while keeping your tests fails SS16 listcomp var leaks over an earlier plain assignment; restoring it passes. That's the check most people skip, and it's the one that makes the other numbers mean anything.

What I verified beyond your report, since this is compiler.c slot promotion and #348 was an over-fix in this exact code:

• missing-param-binds-null still works (define p2(a, b) as: zs is [b for b in [1]]; return a → p2 of 5 = 5). That's the semantics OP_SET_LOCAL silently drops writes to slots ≥ env->count — should be a VM error (or auto-reserve) #348 broke by "fixing" a similar-looking silent path, so it was the first thing I attacked.
• the scans stop at the right boundaries — a nested define reusing the name keeps its own binding (55), and a lambda reusing it leaves the outer alone (3).
• a param sharing a listcomp var's name is unchanged from main ([3, [2,4,6]] both) — that's the documented leak, not something you altered. I checked against main before believing it.
• suite 2890/2890 on my own run, and CI 15/15 including the bench instruction-count gate — worth calling out, because de-promoting a name off the slot path is exactly the change that could have cost measurable perf, and it didn't.

Following collect_module_names_walk's for-loop exclusion as the precedent was the right instinct — that mechanism existed for this exact reason and the issue only gestured at it.

[Nitjsefnie]

Thank you for the thorough independent verification — especially re-running the mutation check and attacking the #348-shaped regression first; that's exactly the failure mode I was most worried about touching slot promotion.

Good catch on #642 — I hadn't connected it. You're right that SS16–SS19 only pin the read-after-comprehension shape, so a follow-up test pinning the body-corruption symptom ([i * 2 for i in [1,2,3]] after a prior i) is worth having; glad the de-promotion covers both sides.