[PR #1941] [MERGED] fix(javascript): bound value evaluation through nested closure aliases #4924

New issue

Closed

opened 2026-10-31 00:56:48 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 00:56:48 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1941
Author: @ranmori
Created: 10/11/2026
Status: ✅ Merged
Merged: 10/12/2026
Merged by: @baron

Base: main ← Head: fix/javascript-closure-alias-evaluation

📝 Commits (2)

ec28a5f fix(javascript): bound value evaluation through nested closure aliases
72a88db fix(javascript): keep the expression depth budget for retained values

📊 Changes

2 files changed (+212 additions, -43 deletions)

View changed files

📝 src/domain/javascript/javascriptSemanticAnalysis.scope_depth.test.ts (+120 -34)
📝 src/domain/javascript/javascriptSemanticValues.ts (+92 -9)

📄 Description

Problem

A closure that aliases a binding from its enclosing scope makes semantic value
evaluation exponential in nesting depth:

function f0(seed){ var s0 = seed;
  function f1(){ var s1 = s0;
    function f2(){ var s2 = s1; /* ... */ } } }

Each closure evaluates the outer binding once live and once at its own capture
point. The capture cannot order a binding declared in another function, so it
falls back to a second live evaluation. Only primitive results were kept, so
every level doubled the work.

Observed on Windows 11 x64, Node 24.13.0, at main d802304c:

Case	Before	After
`analyzeJavaScriptSemantics`, 20 nested closures (~3 KB)	~14 s	8 ms
`analyze-javascript-application` on one 443 KB generated wrapper (24-deep closures)	`provider_timeout` at 300 s, exit 1	exit 0 in 23 s

The same doubling occurs when the aliased value is an object, a member read,
a logical fallback, or an unknown parameter. A chain ending in a literal was
already fast because primitives were cached.

Change

Keep a live binding value when no caller could change it: no alias cycle closes
on or above the binding, and the expression depth budget was not reached while
evaluating it. Bindings that sit on a cycle are still re-evaluated, so their
values and reasons are unchanged. The existing cache-clearing on mutation and
escape effects covers the new entries.

Validation

New cases in javascriptSemanticAnalysis.scope_depth.test.ts (object root,
self-referential namespace root, unknown parameter root) fail without the fix
(589,823 ancestry reads against a bound of 18,360) and pass with it.
vitest run --project domain: 136 files, 2,002 tests pass.
npm run check:fast passes.
Differential check: serialized analyzeJavaScriptSemantics output is
byte-identical before and after on 600 JavaScript files sampled from
node_modules plus 12 inline alias-cycle, closure, mutation and escape cases.
Total analysis time over that corpus showed no consistent difference across
three alternating runs.

Not verified

Linux and macOS; all runs were on one Windows host, using the published 6.4.0
Windows native addon with a source-built runtime.
npm run check, check:pr, and the MCP workflow. Only the CLI was run end to end.
Alias chains longer than the 256-level expression depth budget. Values that
reach that budget are deliberately not kept, so such a chain evaluated from
its deep end first is not bounded by this change.
Four tests in JavaScriptModuleSpecifierPaths.test.ts and
JavaScriptFileUrlPaths.test.ts fail on this Windows host with and without
the change (they create file names containing ? and #).

Found while building a source-owned reproduction for #1847. That report's worker
heap exhaustion is a separate, already closed defect; this PR does not claim to
address it.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/1941 **Author:** [@ranmori](https://github.com/ranmori) **Created:** 10/11/2026 **Status:** ✅ Merged **Merged:** 10/12/2026 **Merged by:** [@baron](https://github.com/baron) **Base:** `main` ← **Head:** `fix/javascript-closure-alias-evaluation` --- ### 📝 Commits (2) - [`ec28a5f`](https://github.com/morluto/rea/commit/ec28a5fceccc5f1b2fafb623051518fd99ea66b5) fix(javascript): bound value evaluation through nested closure aliases - [`72a88db`](https://github.com/morluto/rea/commit/72a88db4c4b12f652060d51df223a14f00f8096c) fix(javascript): keep the expression depth budget for retained values ### 📊 Changes **2 files changed** (+212 additions, -43 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/javascript/javascriptSemanticAnalysis.scope_depth.test.ts` (+120 -34) 📝 `src/domain/javascript/javascriptSemanticValues.ts` (+92 -9) </details> ### 📄 Description ## Problem A closure that aliases a binding from its enclosing scope makes semantic value evaluation exponential in nesting depth: function f0(seed){ var s0 = seed; function f1(){ var s1 = s0; function f2(){ var s2 = s1; /* ... */ } } } Each closure evaluates the outer binding once live and once at its own capture point. The capture cannot order a binding declared in another function, so it falls back to a second live evaluation. Only primitive results were kept, so every level doubled the work. Observed on Windows 11 x64, Node 24.13.0, at main `d802304c`: | Case | Before | After | |---|---|---| | `analyzeJavaScriptSemantics`, 20 nested closures (~3 KB) | ~14 s | 8 ms | | `analyze-javascript-application` on one 443 KB generated wrapper (24-deep closures) | `provider_timeout` at 300 s, exit 1 | exit 0 in 23 s | The same doubling occurs when the aliased value is an object, a member read, a logical fallback, or an unknown parameter. A chain ending in a literal was already fast because primitives were cached. ## Change Keep a live binding value when no caller could change it: no alias cycle closes on or above the binding, and the expression depth budget was not reached while evaluating it. Bindings that sit on a cycle are still re-evaluated, so their values and reasons are unchanged. The existing cache-clearing on mutation and escape effects covers the new entries. ## Validation - New cases in `javascriptSemanticAnalysis.scope_depth.test.ts` (object root, self-referential namespace root, unknown parameter root) fail without the fix (589,823 ancestry reads against a bound of 18,360) and pass with it. - `vitest run --project domain`: 136 files, 2,002 tests pass. - `npm run check:fast` passes. - Differential check: serialized `analyzeJavaScriptSemantics` output is byte-identical before and after on 600 JavaScript files sampled from `node_modules` plus 12 inline alias-cycle, closure, mutation and escape cases. Total analysis time over that corpus showed no consistent difference across three alternating runs. ## Not verified - Linux and macOS; all runs were on one Windows host, using the published 6.4.0 Windows native addon with a source-built runtime. - `npm run check`, `check:pr`, and the MCP workflow. Only the CLI was run end to end. - Alias chains longer than the 256-level expression depth budget. Values that reach that budget are deliberately not kept, so such a chain evaluated from its deep end first is not bounded by this change. - Four tests in `JavaScriptModuleSpecifierPaths.test.ts` and `JavaScriptFileUrlPaths.test.ts` fail on this Windows host with and without the change (they create file names containing `?` and `#`). Found while building a source-owned reproduction for #1847. That report's worker heap exhaustion is a separate, already closed defect; this PR does not claim to address it. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 00:56:48 +00:00