From f9cec7e6aabedae6a1a2ae84c1bb75b81f6ca8c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 14 Jul 2026 19:15:55 +0200 Subject: [PATCH] fix(hir): don't store the capture BOX in an object literal's field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #5951 boxes a mutable local that a class captures AND mutates into a one-element array cell, and rewrites every value read of it into `o[0]`. A real class construction auto-appends CAPTURE arguments that must pass the raw array handle, so the `Expr::New` arm of that rewrite deliberately skips a bare `LocalGet(boxed)` argument. But an OBJECT LITERAL also lowers to `Expr::New` — against a synthetic `__AnonShape_*` whose constructor arguments are the PROPERTY VALUES, not capture handles. The skip therefore stored the CELL in the field: function factory() { let o; class Holder { constructor() { this.snap = o; } update(x) { o = x; } } o = { locales: ["en", "de"] }; new Holder(); return function () { const lit = { routing: o, a: 1, b: 2, c: 3, d: 4 }; o.locales.length; // 2 — this read IS rewritten to o[0] lit.routing; // node: o perry: [o] <-- the box lit.routing.locales; // node: [...] perry: undefined }; } Every other read of `o` works, so the failure surfaces far from its cause: a guard like `o.locales.length > 1` passes on the line *before* the literal that then hands its consumer a `routing` whose `.locales` is `undefined`. A synthetic shape has no captures, so every one of its args is a value and must be rewritten. Real class constructions keep the existing behavior — their capture arguments still pass the array handle by reference, so writes through the class stay visible to the declaring function (covered by the test). This is next-intl's alternate-links config, reached on every page render of a localized Next.js app: it built `{routing: o, …}` from a module-scope config that a bundled class captures and mutates, and the reader then did `routing.locales.map(…)` → `TypeError: Cannot read properties of undefined (reading 'map')`, i.e. a 500 on every page. Found while compiling a real Next.js 16 + Auth.js + next-intl app. Covered by `test_gap_anon_shape_boxed_capture` — field identity, `Array.isArray`, reading the field back through a destructured parameter, the cell still being SHARED with the class after a write, and the class-capture-by-reference case. Byte-identical to node. --- .../src/lower/shared_mutable_capture.rs | 19 ++++- .../test_gap_anon_shape_boxed_capture.ts | 77 +++++++++++++++++++ 2 files changed, 94 insertions(+), 2 deletions(-) create mode 100644 test-files/test_gap_anon_shape_boxed_capture.ts diff --git a/crates/perry-hir/src/lower/shared_mutable_capture.rs b/crates/perry-hir/src/lower/shared_mutable_capture.rs index 2dc6b8472e..c253341259 100644 --- a/crates/perry-hir/src/lower/shared_mutable_capture.rs +++ b/crates/perry-hir/src/lower/shared_mutable_capture.rs @@ -873,9 +873,24 @@ fn rewrite_expr(expr: &mut Expr, shared: &HashSet, index_uses: &HashSet // Capture sites snapshot the WHOLE handle (the array). Leave the bare // `LocalGet(id)` capture args alone; still rewrite non-capture children. Expr::RegisterClassCaptures { .. } => return, - Expr::New { args, .. } => { + Expr::New { + class_name, args, .. + } => { + // An OBJECT LITERAL also lowers to `New` — against a synthetic + // `__AnonShape_*` whose ctor args are the PROPERTY VALUES, not the + // auto-appended capture handles a real class construction carries. + // Skipping a bare `LocalGet(boxed)` there stored the one-element array + // BOX in the field instead of the value it holds, so `{routing: o}` + // produced `routing === [o]`: every other read of `o` is rewritten to + // `o[0]` and works, while the object's field holds the cell — the guard + // `o.locales.length > 1` passes one line before the literal that then + // hands the callee a `routing` whose `.locales` is `undefined`. + // + // A synthetic shape has no captures, so every one of its args is a value + // and must be rewritten. + let is_anon_shape = class_name.starts_with("__AnonShape_"); for a in args.iter_mut() { - if matches!(a, Expr::LocalGet(id) if index_uses.contains(id)) { + if !is_anon_shape && matches!(a, Expr::LocalGet(id) if index_uses.contains(id)) { continue; // auto-appended capture argument — keep the array handle } rewrite_expr(a, shared, index_uses); diff --git a/test-files/test_gap_anon_shape_boxed_capture.ts b/test-files/test_gap_anon_shape_boxed_capture.ts new file mode 100644 index 0000000000..193499b854 --- /dev/null +++ b/test-files/test_gap_anon_shape_boxed_capture.ts @@ -0,0 +1,77 @@ +// #5951 boxes a mutable local that a class captures AND mutates into a +// one-element array cell, rewriting every value read `o` into `o[0]`. An object +// literal also lowers to `Expr::New` — against a synthetic `__AnonShape_*` whose +// constructor args are the PROPERTY VALUES, not the capture handles a real class +// construction carries. Passing the bare box there stored the CELL in the field: +// `{ routing: o }` yielded `routing === [o]`, so the reader saw +// `routing.locales === undefined` while every other read of `o` was fine. + +function factory() { + let o: any; + + // a class that captures AND mutates `o` -> `o` becomes a shared cell + class Holder { + snapshot: any; + constructor() { + this.snapshot = o; + } + update(x: any) { + o = x; + } + } + + o = { locales: ["en", "de", "es"], defaultLocale: "en" }; + const h = new Holder(); + + return function (req: string) { + // a 5-property object literal -> lowered to `New { __AnonShape_… }` + const lit: any = { + routing: o, + internalTemplateName: undefined, + localizedPathnames: undefined, + request: req, + resolvedLocale: "en", + }; + + console.log("direct read :", o.locales.length); + console.log("field isArray :", Array.isArray(lit.routing)); + console.log("identity kept :", lit.routing === o); + console.log("locales :", lit.routing.locales.join(",")); + console.log("other fields :", lit.request, lit.resolvedLocale); + + // reading the field back through a destructured parameter (how it is consumed) + const read = function ({ routing: a, request: r }: any) { + return a.locales.join("|") + " / " + r; + }; + console.log("via destructure:", read(lit)); + + // the cell must still be SHARED with the class (the reason the box exists) + h.update({ locales: ["fr"], defaultLocale: "fr" }); + console.log("after update :", o.locales.join(","), Array.isArray(o)); + const lit2: any = { routing: o, a: 1, b: 2, c: 3, d: 4 }; + console.log("relit isArray :", Array.isArray(lit2.routing), lit2.routing.locales.join(",")); + return "ok"; + }; +} + +console.log("result :", factory()("REQ")); + +// a real class construction must still receive its capture handle by reference: +// writes through the class stay visible to the declaring function. +function sharedCell() { + let count = 0; + class Counter { + bump() { + count++; + } + read() { + return count; + } + } + const c = new Counter(); + c.bump(); + c.bump(); + count += 10; + return c.read() + "," + count; +} +console.log("shared cell :", sharedCell());