Merge train: #9720, #9722 + stale TLS allowlist entry

TYPE_LOWERING.md

@@ -654,7 +654,7 @@ Every allocation is preceded by an 8-byte `GcHeader`: `obj_type` (u8), `gc_flags
 
 ### Closures
 
-`ClosureHeader`: `func_ptr` (usize), `capture_count` (u32, high bit = `CAPTURES_THIS_FLAG`), `type_tag` (`CLOSURE_MAGIC 0x434C_4F53`), variadic `captures[]` (u64 slots). Mutable captures are heap-boxed. Side-tables: `CLOSURE_REST_REGISTRY`, `CLOSURE_ARITY_REGISTRY`, `DISPATCH_CACHE`. Public closure dispatch still uses the generic closure pointer plus boxed `double` argument/return model. Eligible typed closure clones now use an internal `i64 this_closure, typed args...` ABI so immutable f64/i1 captures can be loaded as native values before the body is lowered.
+`ClosureHeader`: `func_ptr` (usize), `capture_count` (u32, high bit = `CAPTURES_THIS_FLAG`), `type_tag` (`CLOSURE_MAGIC 0x434C_4F53`), variadic `captures[]` (u64 slots). Mutable captures are heap-boxed. Side-table: `CLOSURE_BODY_REGISTRY` (one packed record per body: rest/arity/length/flags, #9707) plus the `DISPATCH_RECENT` four-entry cache. Public closure dispatch still uses the generic closure pointer plus boxed `double` argument/return model. Eligible typed closure clones now use an internal `i64 this_closure, typed args...` ABI so immutable f64/i1 captures can be loaded as native values before the body is lowered.
 
 ### Async/Await
 

changelog.d/9720-initializer-self-binding.md

@@ -0,0 +1,39 @@
+**A closure in a `let`/`const` declarator's own initializer can now see the
+binding that declarator introduces** — `const off = ev.on(() => off())`,
+`const { unmount } = await render({ onDone: () => unmount() })`. It previously
+threw `ReferenceError: <name> is not defined` (#9718).
+
+The function-body forward-capture pre-pass (`pre_register_forward_captured_lets`)
+decides whether a `let`/`const` binding needs a boxed, TDZ-seeded forward
+declaration by asking whether any closure *seen so far* references its name. It
+recorded a declarator's own initializer into that set only **after** making that
+decision — which is all the later-declarator case needs (`let z = (w) => { … O … },
+O = setTimeout(z, K)`, the minified `new Promise` executor shape), but left the
+self-referential shape unregistered. The reference then fell through to
+`js_global_get_or_throw_unresolved`. Recording the initializer's closure refs
+before the decision is a strict superset: later declarators still see them.
+
+The hole was reachable from every declarator form, because the pre-pass is what
+the destructuring path relies on — `destructuring/var_decl.rs`'s own
+`is_function_expr_init` pre-registration covers only simple `Pat::Ident`
+bindings, and its `ast_expr_contains_function_expr` scan does not descend
+through `await`. So `const O = await mk({ cb: () => O() })` (plain binding
+behind an `await`), every object/array/nested pattern, and `{ key }` shorthand
+all failed; only a closure created *after* the declaration worked.
+
+Found in claude-code: the `install` subcommand is
+
+```js
+let { unmount: O } = await eB(el(wMA, { onDone: (w, $) => { O(), q(w, $) }, … }))
+```
+
+so `claude install <bad-channel>` threw an uncaught `ReferenceError: O is not
+defined` mid-teardown, losing the final newline and the exit code — perry exited
+0 where node exits 1 (the `B60_install_badtarget` divergence carried in #9575).
+
+`test-files/test_gap_9718_initializer_self_binding.ts` pins all of it against
+node: plain binding, object pattern, `{ key }` shorthand, array pattern, nested
+pattern, `let` and `const`, with and without `await`, plus a multi-declarator
+statement so the reordered scan's pre-existing earlier-refs-later behavior stays
+covered. Unpatched, 7 of its 12 lines are `ReferenceError`; patched, the output
+is byte-identical to node 26.5.1.

changelog.d/9722-closure-body-registry-record.md

@@ -0,0 +1,50 @@
+**Closure body registries: one packed record per function instead of ten
+maps keyed by the same pointer (#9707).**
+
+Module init used to record what it knows about each closure body — rest
+arity + kind, declared ABI arity, ECMAScript `.length`, arrow / strict /
+async / generator / async-generator flags, the two compiler-private
+direct-call bodies of an eligible arrow — into ten separate thread-local
+`PtrHashMap`s, every one keyed by the same `func_ptr`, plus an eleventh map
+memoizing the dispatch strategy those answers imply. Ten key copies, ten
+tables' worth of hashbrown load-factor slack, and a dispatch-strategy miss
+that probed up to four of them in sequence. On cc's validated heap census
+(~59k functions) the family summed to 7.24 MB with the current estimator
+(the 11.8 MB the issue quotes was the earlier one, which double-counted
+exactly-sized tables).
+
+`CLOSURE_BODY_REGISTRY` is now the single table: `func_ptr →
+ClosureBodyRecord`, a 16-byte record holding `.length`, the declared arity
+and the rest arity as integers, and every boolean attribute plus the 2-bit
+rest kind as flag bits; `(usize, ClosureBodyRecord)` is a 24-byte bucket,
+pinned by a size test. The rare `TrustedDirectTarget` pair (direct-call body
+and versioned-loop body) moves to a dense append-only side array
+(`TRUSTED_TARGETS`) that only eligible arrows index into, so a body without
+them pays four bytes, not two `Option<TrustedDirectTarget>` maps. The
+dispatch-strategy cache is deleted outright: a miss now does ONE probe of
+the record and derives rest/arity/arrow-ness from its bits, which is cheaper
+than the second hash probe the cache cost — and cannot go stale, so the
+#6475 late-registration invalidation shrinks to the four-entry
+`DISPATCH_RECENT` eviction. Every `js_register_closure_*` entry point and
+every `lookup_*` / `is_registered_*` reader keeps its signature; rest still
+wins over arity for dispatch and `closure_arity`, and `closure_length` still
+prefers the explicit length, then rest, then arity.
+
+Measured with `PERRY_GC_CENSUS` on a generated 20k-function fixture (5k
+each of default-param arrows, rest functions, async functions and
+generators; 35,051 registered bodies including the runtime's own): the
+closure registry rows go from 2,916,564 bytes across seven populated maps to
+1,638,416 bytes in one (`closure.body_registry`), −44 %, with byte-identical
+program output. Projected onto cc's recorded census counts (59,384 distinct
+bodies, 58.5k of them strict, 27k arrows, 6.8k dispatch-cache entries) the
+same estimator gives 3.28 MB against the 7.24 MB before, −55 %; the
+remaining floor is hashbrown's power-of-two bucket count at that size. The
+census now prints `closure.body_registry` and `closure.trusted_targets` in
+place of the ten per-attribute rows.
+
+Not in this change: the `fn.name_registry` / `fn.source_registry` tables the
+issue mentions (5.4 MB on the same census) keep their own `func_ptr` keying —
+folding them in wants the dense function-id scheme, which this does not
+introduce. `scripts/gc_runtime_root_holders.json` records the two new
+statics under the same `not_a_gc_pointer` verdict the deleted maps carried
+(code pointers and plain integers only) and drops the eight stale entries.

changelog.d/9724-stale-tls-allowlist-entry.md

@@ -0,0 +1,6 @@
+**Drop the stale `readline_helpers.rs` entry from the cold-`thread_local!`
+allowlist.** #9697 removed that file's only raw `thread_local!` block but left
+its justification behind, and `scripts/check_thread_locals.py` fails a recorded
+entry that no longer matches the tree — "a stale entry is one nobody has to
+justify". The check runs in `tls-budget.yml`, a satellite workflow outside the
+64-gate `lint` set, so it was not caught at merge time.

crates/perry-codegen/src/codegen/string_pool.rs

@@ -169,7 +169,7 @@ pub(super) fn emit_string_pool(
     user_fn_wrapper_rest_and_arguments: &std::collections::HashSet<String>,
     // ABI param count for every top-level user-function wrapper
     // (`__perry_wrap_<original_name>`) — used to register the wrapper's
-    // declared arity in the runtime's `CLOSURE_ARITY_REGISTRY` so dynamic
+    // declared arity in the runtime's closure body registry so dynamic
     // dispatch can pad missing trailing args before invoking the wrapper.
     // Entries for wrappers also present in `user_fn_wrapper_rest` are skipped
     // (those go through the rest registry which already controls dispatch).
@@ -769,7 +769,7 @@ pub(super) fn emit_string_pool(
     // length, which mis-split dynamic-parent (capless-sig-with-snapshot) ctors.
     let mut ctor_triples: Vec<(u32, String, u32, u32)> = Vec::new();
     // #wall3: class ctors with a rest param (`constructor(...args)`) need their
-    // standalone `_constructor` func_ptr registered in CLOSURE_REST_REGISTRY so
+    // standalone `_constructor` func_ptr registered as rest-bearing in the closure body registry so
     // a member-new (`new ns.Sub(opts)` → js_new_function_construct →
     // js_native_call_value) BUNDLES trailing args into the rest array. Without
     // this the rest param binds to the first arg as a scalar (a=opts, not

crates/perry-hir/src/lower_decl/block.rs

@@ -178,6 +178,32 @@ pub(crate) fn pre_register_forward_captured_lets(
                     ast::VarDeclKind::Let | ast::VarDeclKind::Const
                 ) {
                     for decl in &var_decl.decls {
+                        // A closure in a declarator's OWN initializer can
+                        // reference the binding that declarator introduces:
+                        //   `const off = ev.on(() => off())`
+                        //   `const { unmount: O } = await render({ onDone: () => O() })`
+                        // (claude-code's `install` subcommand, #9718). The
+                        // closure body does not run until after initialization,
+                        // so this is legal — node only has a TDZ window here.
+                        // Record this declarator's closure refs BEFORE deciding
+                        // whether its own bindings are forward-captured; doing
+                        // it only afterwards (which is all the LATER-declarator
+                        // case below needs) left the self-referential shape
+                        // unregistered, so the reference fell through to
+                        // `js_global_get_or_throw_unresolved` and threw
+                        // `ReferenceError: <name> is not defined`.
+                        //
+                        // Recording early is a superset of recording late: later
+                        // declarators of the same declaration still see these
+                        // refs, which is what the intra-declaration case wants:
+                        //   `let z = (w) => { … O … }, O = setTimeout(z, K);`
+                        // (the minified `new Promise` executor shape — without
+                        // it the `resolve` never fires and the awaiting caller
+                        // hangs). Cross-statement forward-refs are handled by
+                        // the trailing `cic_stmt`.
+                        if let Some(init) = &decl.init {
+                            cic_expr(init, false, &mut seen_closure_refs);
+                        }
                         let mut binding_idents: Vec<(String, u32)> = Vec::new();
                         collect_pat_forward_idents(&decl.name, &mut binding_idents);
                         for (name, span_lo) in binding_idents {
@@ -213,27 +239,6 @@ pub(crate) fn pre_register_forward_captured_lets(
                                 }
                             }
                         }
-                        // A closure in an EARLIER declarator of THIS same
-                        // `let`/`const` can forward-reference a name bound by a LATER
-                        // declarator in the SAME declaration:
-                        //   `let z = (w) => { … O … Y … A … },
-                        //        Y = () => z(false),
-                        //        A = () => clearTimeout(O),
-                        //        O = setTimeout(z, K);`
-                        // (the minified `new Promise` executor shape). Record this
-                        // declarator's closure refs NOW so the later declarators are
-                        // seen as forward-captured too — `seen_closure_refs` is
-                        // otherwise only updated by the trailing `cic_stmt` AFTER the
-                        // whole declaration, so intra-declaration forward-refs were
-                        // missed: the later names never got pre-registered, so the
-                        // ref fell through to `js_global_get_or_throw_unresolved` and
-                        // the closure captured a global instead of the local box —
-                        // e.g. a `new Promise` `resolve` that never fires, hanging
-                        // the awaiting caller. Cross-statement forward-refs were
-                        // already handled by the trailing `cic_stmt`.
-                        if let Some(init) = &decl.init {
-                            cic_expr(init, false, &mut seen_closure_refs);
-                        }
                     }
                 } else {
                     // `var` bindings are already predefined + boxed by

crates/perry-runtime/src/async_hooks.rs

@@ -1602,7 +1602,7 @@ extern "C" fn async_resource_bind_trampoline(closure: *const ClosureHeader, rest
 
 fn register_bind_trampoline_once() {
     thread_local! {
-        // CLOSURE_REST_REGISTRY is thread-local, so each thread that
+        // The closure body registry is thread-local, so each thread that
         // synthesizes a bind() trampoline must register the func_ptr once.
         static REGISTERED: Cell<bool> = const { Cell::new(false) };
     }