diff --git a/CHANGELOG.md b/CHANGELOG.md index bbe7d4ac0..eec99d6a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ Detailed changelog for Perry. See CLAUDE.md for concise summaries. +## v0.5.922 — fix(transform): #858 — closure-captured numeric (or otherwise literal-trivial) params now reach an object-literal method body intact. **Symptom (reported as #858, downstream as #859).** The 12-line repro `function makeDT(y: number) { return { toDate(): Date { return new Date(Date.UTC(y, 4, 15, 17, 29, 35, 402)); } } } const d = makeDT(2026).toDate(); console.log("INLINE:", d.getTime())` printed `INLINE: -62155492224598` (Node prints `1778866175402`). Only the captured numeric `y` was corrupt — the other `Date.UTC` literal args were intact, and the value `-62155492224598` is exactly `Date.UTC(0, 4, 15, ...)`, confirming `y` reads as `0` inside the method body. The same shape blew up as a SIGBUS in `@perryts/mysql`'s `MyDateTime.toDate()` (#859 — `function makeMyDateTime(y, mo, d, h, ...) { return { toDate(): Date { return new Date(Date.UTC(y, mo-1, d, h, ...)); } } }`). **Root cause.** `crates/perry-transform/src/inline.rs`'s call-site inliners (`try_inline_simple_call` Pattern 1 / Pattern 2 / single-Return method / void-method, and `try_inline_call`'s fn + method branches) collectively short-circuited "literal-trivial" args (Integer / Number / Bool / String / Null / Undefined / WtfString / GlobalGet, per `is_trivial_expr`) into `param_map.insert(param.id, arg.clone())`, then ran `substitute_locals` over the cloned function body. `substitute_locals`'s `Expr::Closure` arm faithfully recurses into the closure body and substitutes captured-param `LocalGet(id)` with the literal — *and* removes that id from the closure's `captures` list (the `retain_mut` branch that drops orphan captures when the replacement isn't itself a `LocalGet`). At the call site (init), the inlined `Expr::Closure` therefore ended up with `body: [...Integer(2026)...]` and `captures: []`. But closure bodies are compiled exactly once by `compile_closure`, keyed by `func_id`, and `collect_closures_in_stmts` registers the **first** occurrence it sees per `func_id` — that's the original (uninlined) Closure inside `makeDT`'s body, which still has `body: [...LocalGet(3)...]` and `captures: [3]`. The compiled body therefore expects capture slot 0 to hold `y`. At the call site, `lower_expr`'s `Expr::Closure` arm + `compute_auto_captures` consult the inlined expression's body/captures and see no captured local — so the closure object is allocated with zero capture slots. Reading slot 0 in the compiled body returns whatever uninitialized bits live past the allocation header — usually 0.0, which `Date.UTC` happily accepts as year zero. The mismatch is invisible to type checking and to codegen warnings because both sides ARE internally consistent — just with each other disagreeing on the func_id ↔ capture-shape contract. **Fix.** Before substituting, pre-walk the callee's body for any `Expr::Closure`'s `captures` / `mutable_captures` lists (new helper `collect_closure_captured_local_ids` in `inline.rs`). For each call-site arg whose `is_trivial_expr` would normally allow in-place substitution, force-materialize a fresh `Let` instead when the corresponding param appears in that set AND the arg isn't already a plain `LocalGet` (a `LocalGet → LocalGet` substitution is benign — the closure body still references *a* local, just renumbered). Applied symmetrically to all six call-site code paths in `inline.rs` that drive `substitute_locals` / `substitute_locals_in_stmts` over closure-bearing inlined bodies: `try_inline_simple_call` fn-call Pattern 1, fn-call Pattern 2 (Let-then-Return), single-Return method-call, void-method-call, and `try_inline_call`'s fn-call + method-call paths. The closure body now keeps its `LocalGet(fresh_id)` reference, its `captures: [fresh_id]` stays non-empty, and `lower_expr`'s closure-creation site + `compile_closure`'s body emission agree on slot index 0. The fresh `Let` lives in setup_stmts hoisted before the call site, so the literal value is bound to a real local that the closure-creation site can read and forward into the capture array. **Why this also closes (or unblocks) #859.** `@perryts/mysql`'s `makeMyDateTime(y, mo, d, h, mi, s, ms)` is the exact same shape — a top-level helper that returns `{ toDate(): Date { return new Date(Date.UTC(y, mo-1, d, h, mi, s, ms)) } }` — called with literal args (and from a hot ORM hot path). Pre-fix, each numeric param ended up routed through the same substitute-into-closure-body bug; post-fix they all read correctly. The downstream SIGBUS in shop-admin is consistent with the all-zero `Date.UTC` argument vector producing a NaN / out-of-range Date that's then handed to a subsequent native-method dispatch expecting a valid date pointer. Not separately verified end-to-end in shop-admin (the test rig isn't part of this worktree), so the issue is described as "likely closes #859" rather than confirmed-closed — file a follow-up if the original SIGBUS still reproduces post-merge. **Validation.** New regression test `test-files/test_issue_858_closure_numeric_capture.ts` covers seven shapes: numeric/string/multi-primitive captures inside method shorthand, arrow-returning shape, arrow nested inside an object literal value, `Array.prototype.map` callback capture, and the multi-stmt Pattern 2 path. All seven byte-identical to `node --experimental-strip-types`. Full parity suite (`./run_parity_tests.sh`) — 345/372 pass, all 27 parity failures + all 18 compile failures pre-listed in `test-parity/known_failures.json` (zero new failures). `cargo test --release --workspace` (excluding cross-host UI crates per CLAUDE.md) green. **Files touched.** `crates/perry-transform/src/inline.rs` (new helper + six call-site updates), `test-files/test_issue_858_closure_numeric_capture.ts` (new regression test). Closes #858. Likely closes #859 (downstream — shop-admin SIGBUS site has identical shape). Refs #793. + ## v0.5.921 — fix(codegen + runtime): #489 — `.then` / `.catch` / `.finally` on a Promise returned from a class field/method now actually resumes the await chain. **Symptom (#489 followup).** After v0.5.920 closed the link error, the perry-compiled drizzle + `@perryts/mysql` program reached MySQL and sent the INSERT — but `await db.insert(users).values(...)` never settled. The proxy callback returned `{rows: [{insertId, affectedRows}]}`, the SQL landed at the wire, but execution exited cleanly with code 0 instead of continuing to the line after the await. Two distinct gaps fed the same symptom: (1) at compile time, perry's `is_promise_expr` didn't recognize `obj.field(args)` / `obj.method(args)` as a Promise even when the field is typed `(…) => Promise` or the method is `async`, so `.then(cb)` chained on it fell out of the `js_promise_then` fast-path; (2) at runtime, `js_native_call_method` (the dynamic-dispatch fallback that ate the missed fast-path) had no arm for `then` / `catch` / `finally` on a Promise receiver, so the call returned undefined and the await chain's resolver was never wired up to the underlying promise's settlement queue. **Fix part 1 — type-aware Promise recognition (`crates/perry-codegen/src/type_analysis.rs::is_promise_expr`).** New `Expr::Call { callee: PropertyGet { … } }` arms: (a) consult `static_type_of(callee)` — when it resolves to `HirType::Function(ft)` with `ft.is_async` true or `ft.return_type` being `Promise<…>` / `Generic { base: "Promise" }`, the call result is a Promise (covers class-field arrows typed as `(…) => Promise`); (b) when the callee is a `PropertyGet` whose receiver resolves to a known class, walk the class's `methods` Vec (and parent chain) — if the method is declared `async` or returns `Promise`, the call is a Promise. Parallel arm added to the `Expr::LocalGet` callee branch: a local typed `HirType::Function(ft)` with `is_async`/return-Promise also yields true. Together these cover the drizzle shapes — `this.client(...)` on a `RemoteCallback`-typed field, `this.pre.execute()` on an async method, `this.session.all(q)` on a method whose return is `Promise`. **Fix part 2 — runtime intrinsic for `.then` / `.catch` / `.finally` on a Promise (`crates/perry-runtime/src/object.rs::js_native_call_method`).** Prepended a check: when `method_name` is `"then"` / `"catch"` / `"finally"` and `js_value_is_promise(object)` is non-zero, unbox the promise handle from the NaN-boxed receiver and call `js_promise_then` / `js_promise_catch` / `js_promise_finally` directly, NaN-boxing the returned promise pointer. The closure-arg extractor handles **both** ABIs perry uses for callbacks crossing this boundary: NaN-boxed `POINTER_TAG | (ptr & 0x0000_FFFF_FFFF_FFFF)` from `js_closure_alloc_singleton` callsites, **and** raw `*ClosureHeader` bit-cast to `f64` from `js_assimilate_thenable` (see `promise.rs:2438-2442`: when the await wrapper calls a user-defined `then(resolve, reject)` method via the vtable, it passes `resolve_f64 = f64::from_bits(resolve_closure as u64)` — a bare pointer in a double slot — and when that user method propagates the params through to an inner `e.then(onF, onR)`, perry's dispatch tower stores them straight into the args buffer for `js_native_call_method` without re-tagging). Falsy / out-of-range / TAG_UNDEFINED slots become null `ClosurePtr` so `js_promise_then` treats the handler as missing per spec. **Validation.** New `/tmp/probe489/entry_subset.ts` — a perry-compiled drizzle + `@perryts/mysql` + `drizzle-orm/mysql-proxy` program — runs INSERT, UPDATE, DELETE against real MySQL 9.6 on `127.0.0.1:3306` and produces output **byte-for-byte identical** to the `tsx` baseline; MySQL row state after the perry run matches the baseline run (alice → alice2 rename committed, bob deleted). The original 50-line `entry.ts` advances past the `.then` chain — INSERT/UPDATE/DELETE all complete — but `db.select().from(users)` is **still blocked** by drizzle's `applyMixins(MySqlSelectBase, [QueryPromise])` runtime prototype-copy pattern (perry's static class table doesn't see methods added via `Object.defineProperty(baseClass.prototype, ...)` at module init time). That's a separate compat gap — filed as a #489 followup. **Other test suites.** `cargo test --release -p perry` (223/223 ok), `cargo test --release -p perry-runtime` (250/250 ok). Pre-existing drizzle-sqlite + hono-basic fixture link failures on main are unchanged by this patch (separate top-level-const re-export bug). **Remaining for #489 acceptance.** (a) `applyMixins` runtime-mixin pattern blocks the `db.select()` arm; needs prototype-copy modeling or a perry-side workaround that recognizes the call chain. (b) `crypto.publicEncrypt` (#463) still needed for `caching_sha2_password` first-time auth on non-TLS connections (workaround: warm server auth cache via `mysql` CLI + `PERRY_ALLOW_UNIMPLEMENTED=1` at compile time). Both filed as separate follow-ups under #489. ## v0.5.920 — fix(codegen): #489 — transitive parent-class closure picks the canonical defining path instead of the first BTreeMap match by name. **Symptom.** Compiling a 50-line drizzle + `@perryts/mysql` program (issue #489 acceptance) against MySQL fails at link time with `Undefined symbols: _perry_method_node_modules_drizzle_orm_index_js__QueryPromise__then, referenced from: _perry_method_node_modules_drizzle_orm_mysql_proxy_session_js__MySqlRemoteSession__all`. drizzle's `mysql-proxy/session.js` calls `this.client(...).then(({ rows }) => rows)` — synchronous `.then` chaining on the Promise returned by the proxy callback (#488/pg-proxy uses `async all() { await … }`, sidestepping this). `MySqlPreparedQuery` (imported by session.js) reaches `QueryPromise` through the transitive parent-class closure. **Root cause.** `crates/perry/src/commands/compile.rs` (line 4451 pre-fix) walked parent-class refs by scanning `exported_classes` with a name-only `find`. The BTreeMap is keyed `(path, name)`, and the `Export::ExportAll` / `Export::ReExport` propagation loop above stamps each re-exporter's path under the same name — so a class re-exported via `export * from "./query-promise.js"` in `index.js` lands as `(drizzle-orm/index.js, "QueryPromise")` *and* `(drizzle-orm/query-promise.js, "QueryPromise")`. BTreeMap iteration is alphabetical, `index_js` sorts before `query_promise_js`, the closure picks the barrel path. The downstream codegen then emits `perry_method___QueryPromise__then` extern declarations + dispatch references in session.js's object file; the actual symbol is defined under `perry_method___QueryPromise__then` and the link fails. Same shape as #83 / #678 / #785 (cross-module method dispatch picking the wrong owning module), but for the `export *` star-re-export path through the transitive parent closure instead of the consumer's own import. The namespace-import enumeration path at the same file (line 4046-4072) already avoids this by iterating `ctx.native_modules` directly — the comment there explicitly warns "`exported_classes` gets alias entries stamped under every re-exporter's path … iterating it would hand us the class keyed by `index.ts` when it was actually compiled under `pool.ts`". The parent-closure path had not been hardened the same way. **Fix.** New side map `class_canonical_path: HashMap` populated only from each module's own `hir_module.classes` Vec (the *defining* file, never a re-export). In the transitive parent-closure search, prefer the BTreeMap entry whose `(path, name)` key matches the canonical path for `class.id`; fall back to the old first-match behavior when the class id has no canonical record (defensive — for classes that exist only via re-exports). The fix is a 20-line localized change in `compile.rs` that doesn't touch codegen or the existing namespace-import logic. **Validation.** Drizzle + `@perryts/mysql` + `drizzle-orm/mysql-proxy` 50-line program now compiles clean (binary size 6.4 MB) and reaches MySQL: TCP connect ✓, handshake ✓, schema DDL ✓, prepared INSERT lands at the wire with the correct SQL + params (`insert into users_489 (id, name) values (default, ?)` with `["alice"]`). Pre-existing drizzle-sqlite fixture `_perry_wrap_perry_fn_…_utils_js__textDecoder` link failure on main is unchanged by this patch (separate top-level-const re-export bug, not the parent-closure path). `cargo build --release -p perry` clean. **Remaining blockers for #489 acceptance.** (a) Insert callback resolves cleanly but the `await db.insert(...).values(...)` promise never settles in the perry binary — the proxy callback's `{rows: [{insertId, affectedRows}]}` return value isn't flowing back through `queryWithCache → execute → QueryPromise.then` and the program exits with code 0 before the next line runs. Separate runtime / Promise-plumbing bug, not the codegen issue this patch closes. (b) `crypto.publicEncrypt` (#463) is needed for `caching_sha2_password` first-time auth on non-TLS connections; the test workaround is to warm the server's auth cache via the `mysql` CLI before running perry and compile with `PERRY_ALLOW_UNIMPLEMENTED=1`. Both filed as follow-ups under #489. diff --git a/CLAUDE.md b/CLAUDE.md index 36767fcd6..2af1869fd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -8,7 +8,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co Perry is a native TypeScript compiler written in Rust that compiles TypeScript source code directly to native executables. It uses SWC for TypeScript parsing and LLVM for code generation. -**Current Version:** 0.5.921 +**Current Version:** 0.5.922 ## TypeScript Parity Status diff --git a/Cargo.lock b/Cargo.lock index 1a63bfdfc..1cbee9f12 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4790,7 +4790,7 @@ checksum = "9b4f627cb1b25917193a259e49bdad08f671f8d9708acfd5fe0a8c1455d87220" [[package]] name = "perry" -version = "0.5.921" +version = "0.5.922" dependencies = [ "anyhow", "base64", @@ -4845,14 +4845,14 @@ dependencies = [ [[package]] name = "perry-api-manifest" -version = "0.5.921" +version = "0.5.922" dependencies = [ "serde", ] [[package]] name = "perry-codegen" -version = "0.5.921" +version = "0.5.922" dependencies = [ "anyhow", "log", @@ -4865,7 +4865,7 @@ dependencies = [ [[package]] name = "perry-codegen-arkts" -version = "0.5.921" +version = "0.5.922" dependencies = [ "anyhow", "perry-hir", @@ -4874,7 +4874,7 @@ dependencies = [ [[package]] name = "perry-codegen-glance" -version = "0.5.921" +version = "0.5.922" dependencies = [ "anyhow", "perry-hir", @@ -4882,7 +4882,7 @@ dependencies = [ [[package]] name = "perry-codegen-js" -version = "0.5.921" +version = "0.5.922" dependencies = [ "anyhow", "perry-dispatch", @@ -4892,7 +4892,7 @@ dependencies = [ [[package]] name = "perry-codegen-swiftui" -version = "0.5.921" +version = "0.5.922" dependencies = [ "anyhow", "perry-hir", @@ -4901,7 +4901,7 @@ dependencies = [ [[package]] name = "perry-codegen-wasm" -version = "0.5.921" +version = "0.5.922" dependencies = [ "anyhow", "base64", @@ -4914,7 +4914,7 @@ dependencies = [ [[package]] name = "perry-codegen-wear-tiles" -version = "0.5.921" +version = "0.5.922" dependencies = [ "anyhow", "perry-hir", @@ -4922,7 +4922,7 @@ dependencies = [ [[package]] name = "perry-diagnostics" -version = "0.5.921" +version = "0.5.922" dependencies = [ "serde", "serde_json", @@ -4930,7 +4930,7 @@ dependencies = [ [[package]] name = "perry-dispatch" -version = "0.5.921" +version = "0.5.922" [[package]] name = "perry-doc-fixture-my-bindings" @@ -4941,7 +4941,7 @@ dependencies = [ [[package]] name = "perry-doc-tests" -version = "0.5.921" +version = "0.5.922" dependencies = [ "anyhow", "clap", @@ -4956,7 +4956,7 @@ dependencies = [ [[package]] name = "perry-ext-argon2" -version = "0.5.921" +version = "0.5.922" dependencies = [ "argon2", "perry-ffi", @@ -4964,7 +4964,7 @@ dependencies = [ [[package]] name = "perry-ext-axios" -version = "0.5.921" +version = "0.5.922" dependencies = [ "perry-ffi", "reqwest", @@ -4973,7 +4973,7 @@ dependencies = [ [[package]] name = "perry-ext-bcrypt" -version = "0.5.921" +version = "0.5.922" dependencies = [ "bcrypt", "perry-ffi", @@ -4981,7 +4981,7 @@ dependencies = [ [[package]] name = "perry-ext-better-sqlite3" -version = "0.5.921" +version = "0.5.922" dependencies = [ "perry-ffi", "rusqlite", @@ -4989,7 +4989,7 @@ dependencies = [ [[package]] name = "perry-ext-cheerio" -version = "0.5.921" +version = "0.5.922" dependencies = [ "perry-ffi", "scraper", @@ -4997,14 +4997,14 @@ dependencies = [ [[package]] name = "perry-ext-commander" -version = "0.5.921" +version = "0.5.922" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-cron" -version = "0.5.921" +version = "0.5.922" dependencies = [ "chrono", "cron", @@ -5013,7 +5013,7 @@ dependencies = [ [[package]] name = "perry-ext-dayjs" -version = "0.5.921" +version = "0.5.922" dependencies = [ "chrono", "perry-ffi", @@ -5021,7 +5021,7 @@ dependencies = [ [[package]] name = "perry-ext-decimal" -version = "0.5.921" +version = "0.5.922" dependencies = [ "perry-ffi", "rust_decimal", @@ -5029,7 +5029,7 @@ dependencies = [ [[package]] name = "perry-ext-dotenv" -version = "0.5.921" +version = "0.5.922" dependencies = [ "perry-ffi", "serde_json", @@ -5037,7 +5037,7 @@ dependencies = [ [[package]] name = "perry-ext-ethers" -version = "0.5.921" +version = "0.5.922" dependencies = [ "perry-ffi", "rand 0.8.6", @@ -5045,21 +5045,21 @@ dependencies = [ [[package]] name = "perry-ext-events" -version = "0.5.921" +version = "0.5.922" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-exponential-backoff" -version = "0.5.921" +version = "0.5.922" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-fastify" -version = "0.5.921" +version = "0.5.922" dependencies = [ "bytes", "http-body-util", @@ -5073,7 +5073,7 @@ dependencies = [ [[package]] name = "perry-ext-fetch" -version = "0.5.921" +version = "0.5.922" dependencies = [ "lazy_static", "perry-ffi", @@ -5084,7 +5084,7 @@ dependencies = [ [[package]] name = "perry-ext-http" -version = "0.5.921" +version = "0.5.922" dependencies = [ "lazy_static", "perry-ext-http-server", @@ -5096,7 +5096,7 @@ dependencies = [ [[package]] name = "perry-ext-http-server" -version = "0.5.921" +version = "0.5.922" dependencies = [ "bytes", "http-body-util", @@ -5115,7 +5115,7 @@ dependencies = [ [[package]] name = "perry-ext-ioredis" -version = "0.5.921" +version = "0.5.922" dependencies = [ "lazy_static", "perry-ffi", @@ -5125,7 +5125,7 @@ dependencies = [ [[package]] name = "perry-ext-jsonwebtoken" -version = "0.5.921" +version = "0.5.922" dependencies = [ "base64", "jsonwebtoken", @@ -5136,7 +5136,7 @@ dependencies = [ [[package]] name = "perry-ext-lru-cache" -version = "0.5.921" +version = "0.5.922" dependencies = [ "lru", "perry-ffi", @@ -5144,7 +5144,7 @@ dependencies = [ [[package]] name = "perry-ext-moment" -version = "0.5.921" +version = "0.5.922" dependencies = [ "chrono", "perry-ffi", @@ -5152,7 +5152,7 @@ dependencies = [ [[package]] name = "perry-ext-mongodb" -version = "0.5.921" +version = "0.5.922" dependencies = [ "bson", "futures-util", @@ -5164,7 +5164,7 @@ dependencies = [ [[package]] name = "perry-ext-mysql2" -version = "0.5.921" +version = "0.5.922" dependencies = [ "chrono", "perry-ffi", @@ -5174,7 +5174,7 @@ dependencies = [ [[package]] name = "perry-ext-nanoid" -version = "0.5.921" +version = "0.5.922" dependencies = [ "nanoid", "perry-ffi", @@ -5183,7 +5183,7 @@ dependencies = [ [[package]] name = "perry-ext-net" -version = "0.5.921" +version = "0.5.922" dependencies = [ "perry-ffi", "rustls", @@ -5194,7 +5194,7 @@ dependencies = [ [[package]] name = "perry-ext-nodemailer" -version = "0.5.921" +version = "0.5.922" dependencies = [ "lettre", "perry-ffi", @@ -5204,7 +5204,7 @@ dependencies = [ [[package]] name = "perry-ext-pg" -version = "0.5.921" +version = "0.5.922" dependencies = [ "perry-ffi", "sqlx", @@ -5213,7 +5213,7 @@ dependencies = [ [[package]] name = "perry-ext-ratelimit" -version = "0.5.921" +version = "0.5.922" dependencies = [ "governor", "perry-ffi", @@ -5221,7 +5221,7 @@ dependencies = [ [[package]] name = "perry-ext-sharp" -version = "0.5.921" +version = "0.5.922" dependencies = [ "base64", "image", @@ -5230,14 +5230,14 @@ dependencies = [ [[package]] name = "perry-ext-slugify" -version = "0.5.921" +version = "0.5.922" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-streams" -version = "0.5.921" +version = "0.5.922" dependencies = [ "lazy_static", "perry-ffi", @@ -5245,7 +5245,7 @@ dependencies = [ [[package]] name = "perry-ext-uuid" -version = "0.5.921" +version = "0.5.922" dependencies = [ "perry-ffi", "uuid", @@ -5253,7 +5253,7 @@ dependencies = [ [[package]] name = "perry-ext-validator" -version = "0.5.921" +version = "0.5.922" dependencies = [ "perry-ffi", "regex", @@ -5263,7 +5263,7 @@ dependencies = [ [[package]] name = "perry-ext-ws" -version = "0.5.921" +version = "0.5.922" dependencies = [ "futures-util", "lazy_static", @@ -5274,7 +5274,7 @@ dependencies = [ [[package]] name = "perry-ext-zlib" -version = "0.5.921" +version = "0.5.922" dependencies = [ "flate2", "perry-ffi", @@ -5282,7 +5282,7 @@ dependencies = [ [[package]] name = "perry-ffi" -version = "0.5.921" +version = "0.5.922" dependencies = [ "dashmap 6.1.0", "once_cell", @@ -5291,7 +5291,7 @@ dependencies = [ [[package]] name = "perry-hir" -version = "0.5.921" +version = "0.5.922" dependencies = [ "anyhow", "perry-api-manifest", @@ -5305,7 +5305,7 @@ dependencies = [ [[package]] name = "perry-jsruntime" -version = "0.5.921" +version = "0.5.922" dependencies = [ "anyhow", "deno_core", @@ -5325,7 +5325,7 @@ dependencies = [ [[package]] name = "perry-parser" -version = "0.5.921" +version = "0.5.922" dependencies = [ "anyhow", "perry-diagnostics", @@ -5337,7 +5337,7 @@ dependencies = [ [[package]] name = "perry-runtime" -version = "0.5.921" +version = "0.5.922" dependencies = [ "anyhow", "base64", @@ -5361,7 +5361,7 @@ dependencies = [ [[package]] name = "perry-stdlib" -version = "0.5.921" +version = "0.5.922" dependencies = [ "aes", "aes-gcm", @@ -5429,7 +5429,7 @@ dependencies = [ [[package]] name = "perry-transform" -version = "0.5.921" +version = "0.5.922" dependencies = [ "anyhow", "perry-hir", @@ -5439,7 +5439,7 @@ dependencies = [ [[package]] name = "perry-types" -version = "0.5.921" +version = "0.5.922" dependencies = [ "anyhow", "thiserror 1.0.69", @@ -5447,11 +5447,11 @@ dependencies = [ [[package]] name = "perry-ui" -version = "0.5.921" +version = "0.5.922" [[package]] name = "perry-ui-android" -version = "0.5.921" +version = "0.5.922" dependencies = [ "itoa", "jni", @@ -5466,7 +5466,7 @@ dependencies = [ [[package]] name = "perry-ui-geisterhand" -version = "0.5.921" +version = "0.5.922" dependencies = [ "rand 0.8.6", "serde", @@ -5476,7 +5476,7 @@ dependencies = [ [[package]] name = "perry-ui-gtk4" -version = "0.5.921" +version = "0.5.922" dependencies = [ "cairo-rs", "dirs 5.0.1", @@ -5495,7 +5495,7 @@ dependencies = [ [[package]] name = "perry-ui-ios" -version = "0.5.921" +version = "0.5.922" dependencies = [ "block2", "libc", @@ -5510,7 +5510,7 @@ dependencies = [ [[package]] name = "perry-ui-macos" -version = "0.5.921" +version = "0.5.922" dependencies = [ "block2", "libc", @@ -5528,11 +5528,11 @@ version = "0.1.0" [[package]] name = "perry-ui-testkit" -version = "0.5.921" +version = "0.5.922" [[package]] name = "perry-ui-tvos" -version = "0.5.921" +version = "0.5.922" dependencies = [ "block2", "libc", @@ -5547,7 +5547,7 @@ dependencies = [ [[package]] name = "perry-ui-visionos" -version = "0.5.921" +version = "0.5.922" dependencies = [ "block2", "libc", @@ -5562,7 +5562,7 @@ dependencies = [ [[package]] name = "perry-ui-watchos" -version = "0.5.921" +version = "0.5.922" dependencies = [ "block2", "libc", @@ -5575,7 +5575,7 @@ dependencies = [ [[package]] name = "perry-ui-windows" -version = "0.5.921" +version = "0.5.922" dependencies = [ "libc", "perry-runtime", @@ -5589,7 +5589,7 @@ dependencies = [ [[package]] name = "perry-updater" -version = "0.5.921" +version = "0.5.922" dependencies = [ "base64", "ed25519-dalek", @@ -5603,7 +5603,7 @@ dependencies = [ [[package]] name = "perry-wasm-host" -version = "0.5.921" +version = "0.5.922" dependencies = [ "wasmi", ] diff --git a/Cargo.toml b/Cargo.toml index 174769679..359faffa6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -190,7 +190,7 @@ opt-level = "s" # Optimize for size in stdlib opt-level = 3 [workspace.package] -version = "0.5.921" +version = "0.5.922" edition = "2021" license = "MIT" repository = "https://github.com/PerryTS/perry" diff --git a/crates/perry-transform/src/inline.rs b/crates/perry-transform/src/inline.rs index b89491583..bfe91801c 100644 --- a/crates/perry-transform/src/inline.rs +++ b/crates/perry-transform/src/inline.rs @@ -2207,6 +2207,147 @@ fn body_contains_closure_capturing( stmts.iter().any(|s| check_stmt(s, captured_ids)) } +/// Collect every LocalId that appears in some nested closure's `captures` or +/// `mutable_captures` list anywhere inside `stmts`. Used by the call-site +/// inliners to decide which params *must* be materialized as a Let rather +/// than substituted in-place with the call argument. +/// +/// Why this matters (issue #858): closure bodies have a stable `func_id`, +/// which codegen compiles ONCE from whichever `Expr::Closure` occurrence +/// `collect_closures_in_*` saw first (typically the original definition +/// inside the enclosing function). The body of that compiled function reads +/// captured locals from indexed slots in the per-closure capture array. If +/// the inliner substitutes a literal (`Integer(2026)`) for a captured +/// `LocalGet` inside the closure's body at a call site, the call-site +/// `Expr::Closure` ends up with an empty `captures` list — so +/// `compute_auto_captures` at the closure-creation site emits zero capture +/// slots. But the compiled body (from the original occurrence) still reads +/// slot 0, getting an uninitialized value. The visible symptom is that a +/// closure-captured numeric param reads as `0` inside an object-literal +/// method shorthand (`function makeDT(y){ return { toDate(){ ... y ... } } }`). +/// +/// Fix: when the inlined function's body contains a closure that captures +/// one of the params, force the inliner to introduce a setup `Let` for that +/// param (instead of substituting the arg literal in place). The closure +/// body then continues to reference the param via `LocalGet(fresh_id)` and +/// `captures: [fresh_id]`, preserving the func_id <-> capture-shape contract. +fn collect_closure_captured_local_ids( + stmts: &[Stmt], + out: &mut std::collections::HashSet, +) { + fn visit_expr(e: &Expr, out: &mut std::collections::HashSet) { + if let Expr::Closure { + captures, + mutable_captures, + body, + .. + } = e + { + for id in captures { + out.insert(*id); + } + for id in mutable_captures { + out.insert(*id); + } + // Nested closures inside the body can also capture outer + // params transitively (e.g. `function f(y){ return { m(){ return + // [].map(_ => y); } } }`). Walk the body so we don't miss them. + collect_closure_captured_local_ids(body, out); + } + // Descend into immediate sub-expressions for non-Closure variants + // (and into Param defaults for Closure). The walker is exhaustive, + // so any new HIR variant carrying an Expr is automatically covered. + perry_hir::walker::walk_expr_children(e, &mut |sub| visit_expr(sub, out)); + } + + fn visit_stmt(s: &Stmt, out: &mut std::collections::HashSet) { + match s { + Stmt::Let { init: Some(e), .. } => visit_expr(e, out), + Stmt::Expr(e) | Stmt::Return(Some(e)) | Stmt::Throw(e) => visit_expr(e, out), + Stmt::Return(None) => {} + Stmt::If { + condition, + then_branch, + else_branch, + } => { + visit_expr(condition, out); + for s in then_branch { + visit_stmt(s, out); + } + if let Some(eb) = else_branch { + for s in eb { + visit_stmt(s, out); + } + } + } + Stmt::While { condition, body } | Stmt::DoWhile { body, condition } => { + visit_expr(condition, out); + for s in body { + visit_stmt(s, out); + } + } + Stmt::For { + init, + condition, + update, + body, + } => { + if let Some(i) = init { + visit_stmt(i, out); + } + if let Some(c) = condition { + visit_expr(c, out); + } + if let Some(u) = update { + visit_expr(u, out); + } + for s in body { + visit_stmt(s, out); + } + } + Stmt::Switch { + discriminant, + cases, + } => { + visit_expr(discriminant, out); + for case in cases { + if let Some(t) = &case.test { + visit_expr(t, out); + } + for s in &case.body { + visit_stmt(s, out); + } + } + } + Stmt::Try { + body, + catch, + finally, + } => { + for s in body { + visit_stmt(s, out); + } + if let Some(c) = catch { + for s in &c.body { + visit_stmt(s, out); + } + } + if let Some(f) = finally { + for s in f { + visit_stmt(s, out); + } + } + } + Stmt::Labeled { body, .. } => visit_stmt(body, out), + _ => {} + } + } + + for s in stmts { + visit_stmt(s, out); + } +} + /// Check if a function is "pure" for init-inlining purposes: its body only /// references its own parameters and locally-declared variables. No GlobalGet, /// GlobalSet, ExternFuncRef, or NativeMethodCall. This makes it safe to inline @@ -3587,13 +3728,49 @@ fn try_inline_simple_call( // Pattern 1: single Return(expr) if func.body.len() == 1 { if let Stmt::Return(Some(return_expr)) = &func.body[0] { + // Issue #858: params that are captured by some + // closure inside the body MUST be materialized as a + // fresh `Let` rather than substituted in place with + // the (possibly-literal) call argument. Otherwise + // the literal silently rewrites the closure body's + // `LocalGet(param) -> Integer(N)`, the captures list + // empties out at the call site, and the compiled + // closure body (which still reads slot 0 from the + // original `func_id`-keyed occurrence) reads + // garbage / 0. See `collect_closure_captured_local_ids`. + let mut closure_capt: std::collections::HashSet = + std::collections::HashSet::new(); + collect_closure_captured_local_ids(&func.body, &mut closure_capt); + + let mut setup_stmts: Vec = Vec::new(); let mut param_map: HashMap = HashMap::new(); for (param, arg) in func.params.iter().zip(args.iter()) { - param_map.insert(param.id, arg.clone()); + // If the param is closure-captured AND the arg + // isn't already a plain LocalGet (which the + // closure body can capture as-is), materialize + // via a fresh Let so the closure body's + // `LocalGet(fresh)` and `captures: [fresh]` + // stay in agreement with codegen. + let needs_let = closure_capt.contains(¶m.id) + && !matches!(arg, Expr::LocalGet(_)); + if needs_let { + let fresh = *next_local_id; + *next_local_id += 1; + setup_stmts.push(Stmt::Let { + id: fresh, + name: param.name.clone(), + ty: param.ty.clone(), + mutable: false, + init: Some(arg.clone()), + }); + param_map.insert(param.id, Expr::LocalGet(fresh)); + } else { + param_map.insert(param.id, arg.clone()); + } } let mut result = return_expr.clone(); substitute_locals(&mut result, ¶m_map, next_local_id); - return Some((vec![], result)); + return Some((setup_stmts, result)); } } @@ -3614,10 +3791,24 @@ fn try_inline_simple_call( ) }); if all_lets { + // Issue #858: params closure-captured anywhere in + // the body MUST be materialized as a Let even if + // the arg is a literal (Integer/Number/...). + // Substituting a literal in place inside a + // closure body silently empties the closure's + // captures list, breaking the func_id <-> + // capture-shape contract codegen relies on. + let mut closure_capt: std::collections::HashSet = + std::collections::HashSet::new(); + collect_closure_captured_local_ids(&func.body, &mut closure_capt); + // Build param substitution map let mut param_map: HashMap = HashMap::new(); for (param, arg) in func.params.iter().zip(args.iter()) { - if is_trivial_expr(arg) { + let trivial_in_closure = is_trivial_expr(arg) + && !matches!(arg, Expr::LocalGet(_)) + && closure_capt.contains(¶m.id); + if is_trivial_expr(arg) && !trivial_in_closure { param_map.insert(param.id, arg.clone()); } else { let fresh = *next_local_id; @@ -3641,8 +3832,13 @@ fn try_inline_simple_call( let mut setup: Vec = Vec::new(); // First, add Lets for non-trivial param args + // (and for trivial-but-closure-captured args — + // those got promoted to a fresh LocalGet above). for (param, arg) in func.params.iter().zip(args.iter()) { - if !is_trivial_expr(arg) { + let trivial_in_closure = is_trivial_expr(arg) + && !matches!(arg, Expr::LocalGet(_)) + && closure_capt.contains(¶m.id); + if !is_trivial_expr(arg) || trivial_in_closure { if let Some(Expr::LocalGet(fresh_id)) = param_map.get(¶m.id) { setup.push(Stmt::Let { @@ -3734,6 +3930,17 @@ fn try_inline_simple_call( // Check for single return statement if method_candidate.func.body.len() == 1 { if let Stmt::Return(Some(return_expr)) = &method_candidate.func.body[0] { + // Issue #858: see fn-call branch above. Same + // closure-captured-param materialization rule + // applies to method-call inlining. + let mut closure_capt: std::collections::HashSet = + std::collections::HashSet::new(); + collect_closure_captured_local_ids( + &method_candidate.func.body, + &mut closure_capt, + ); + + let mut setup_stmts: Vec = Vec::new(); let mut param_map: HashMap = HashMap::new(); // Map 'this' parameter to the receiver object (only @@ -3751,7 +3958,22 @@ fn try_inline_simple_call( // Note: Method params don't include 'this' - they use Expr::This instead for (param, arg) in method_candidate.func.params.iter().zip(args.iter()) { - param_map.insert(param.id, arg.clone()); + let needs_let = closure_capt.contains(¶m.id) + && !matches!(arg, Expr::LocalGet(_)); + if needs_let { + let fresh = *next_local_id; + *next_local_id += 1; + setup_stmts.push(Stmt::Let { + id: fresh, + name: param.name.clone(), + ty: param.ty.clone(), + mutable: false, + init: Some(arg.clone()), + }); + param_map.insert(param.id, Expr::LocalGet(fresh)); + } else { + param_map.insert(param.id, arg.clone()); + } } let mut result = return_expr.clone(); @@ -3763,7 +3985,13 @@ fn try_inline_simple_call( substitute_this(&mut result, obj_id); } - return Some((vec![], result)); + // Caller only consumes `(setup, result)` — + // returning a non-empty setup is fine; the + // setup_stmts are hoisted before the call site. + if setup_stmts.is_empty() { + return Some((vec![], result)); + } + return Some((setup_stmts, result)); } } @@ -3772,24 +4000,48 @@ fn try_inline_simple_call( let mut is_void_method = true; let mut inlined_stmts = Vec::new(); + // Issue #858: same closure-captured-param rule. We + // build any required setup Lets once for the whole + // void-method body (all Expr stmts share the same + // param substitution). + let mut closure_capt: std::collections::HashSet = + std::collections::HashSet::new(); + collect_closure_captured_local_ids( + &method_candidate.func.body, + &mut closure_capt, + ); + let mut setup_for_params: Vec = Vec::new(); + let mut shared_param_map: HashMap = HashMap::new(); + if let (Some(this_id), Some(obj_id)) = + (method_candidate.this_param_id, obj_id_opt) + { + shared_param_map.insert(this_id, Expr::LocalGet(obj_id)); + } + for (param, arg) in method_candidate.func.params.iter().zip(args.iter()) { + let needs_let = closure_capt.contains(¶m.id) + && !matches!(arg, Expr::LocalGet(_)); + if needs_let { + let fresh = *next_local_id; + *next_local_id += 1; + setup_for_params.push(Stmt::Let { + id: fresh, + name: param.name.clone(), + ty: param.ty.clone(), + mutable: false, + init: Some(arg.clone()), + }); + shared_param_map.insert(param.id, Expr::LocalGet(fresh)); + } else { + shared_param_map.insert(param.id, arg.clone()); + } + } + for stmt in &method_candidate.func.body { match stmt { Stmt::Return(None) => {} Stmt::Expr(e) => { - let mut param_map: HashMap = HashMap::new(); - if let (Some(this_id), Some(obj_id)) = - (method_candidate.this_param_id, obj_id_opt) - { - param_map.insert(this_id, Expr::LocalGet(obj_id)); - } - // Note: Method params don't include 'this' - they use Expr::This instead - for (param, arg) in - method_candidate.func.params.iter().zip(args.iter()) - { - param_map.insert(param.id, arg.clone()); - } let mut expr = e.clone(); - substitute_locals(&mut expr, ¶m_map, next_local_id); + substitute_locals(&mut expr, &shared_param_map, next_local_id); if let Some(obj_id) = obj_id_opt { substitute_this(&mut expr, obj_id); } @@ -3803,7 +4055,9 @@ fn try_inline_simple_call( } if is_void_method && !inlined_stmts.is_empty() { - return Some((inlined_stmts, Expr::Undefined)); + let mut all = setup_for_params; + all.extend(inlined_stmts); + return Some((all, Expr::Undefined)); } } } @@ -3830,8 +4084,22 @@ fn try_inline_call( let mut setup_stmts: Vec = Vec::new(); let mut param_map: HashMap = HashMap::new(); + // Issue #858: see fn-call branch in try_inline_simple_call. + // Params closure-captured by the inlined body must be + // materialized as a Let even when the arg is a literal, + // otherwise the in-place substitution silently rewrites the + // closure body's `LocalGet` -> literal at the call site only, + // while codegen still compiles the original `func_id` body + // expecting capture slot 0 to hold the param. + let mut closure_capt: std::collections::HashSet = + std::collections::HashSet::new(); + collect_closure_captured_local_ids(&func.body, &mut closure_capt); + for (param, arg) in func.params.iter().zip(args.iter()) { - if is_trivial_expr(arg) { + let trivial_in_closure = is_trivial_expr(arg) + && !matches!(arg, Expr::LocalGet(_)) + && closure_capt.contains(¶m.id); + if is_trivial_expr(arg) && !trivial_in_closure { param_map.insert(param.id, arg.clone()); } else { let local_id = *next_local_id; @@ -3946,10 +4214,23 @@ fn try_inline_call( param_map.insert(this_id, Expr::LocalGet(obj_id)); } + // Issue #858: closure-captured params from the inlined + // method body must be materialized as Lets even for + // literal args. See try_inline_simple_call. + let mut closure_capt: std::collections::HashSet = + std::collections::HashSet::new(); + collect_closure_captured_local_ids( + &method_candidate.func.body, + &mut closure_capt, + ); + // Map parameters to arguments // Note: Method params don't include 'this' - they use Expr::This instead for (param, arg) in method_candidate.func.params.iter().zip(args.iter()) { - if is_trivial_expr(arg) { + let trivial_in_closure = is_trivial_expr(arg) + && !matches!(arg, Expr::LocalGet(_)) + && closure_capt.contains(¶m.id); + if is_trivial_expr(arg) && !trivial_in_closure { param_map.insert(param.id, arg.clone()); } else { let local_id = *next_local_id; diff --git a/test-files/test_issue_858_closure_numeric_capture.ts b/test-files/test_issue_858_closure_numeric_capture.ts new file mode 100644 index 000000000..720b9dc68 --- /dev/null +++ b/test-files/test_issue_858_closure_numeric_capture.ts @@ -0,0 +1,95 @@ +// Regression test for issue #858 (downstream #859): +// closure-captured numeric params reach an object-literal method body. +// +// Pre-fix, the inliner's `try_inline_simple_call` Pattern 1 substituted a +// literal call-site arg (`makeDT(2026)` -> `y === Integer(2026)`) directly +// into the nested closure's body — rewriting `LocalGet(y) -> Integer(2026)` +// and emptying the closure's `captures` list at that call site. Codegen +// then emitted the closure with zero capture slots, while the compiled +// closure body (keyed by `func_id`, collected from the original +// uninlined occurrence) still read capture slot 0 expecting `y`. Reading +// the uninitialized slot returned 0, so `Date.UTC(y, 4, 15, ...)` +// computed `Date.UTC(0, 4, 15, ...)` — year 0, getTime() ≈ -6.2e13. +// +// The fix is in `crates/perry-transform/src/inline.rs`: +// pre-walk the inlined body for closure captures, and for any param +// captured by a nested closure, materialize the arg as a fresh `Let` +// (so the closure body keeps a `LocalGet(fresh)` reference and +// `captures: [fresh]` stays non-empty) instead of substituting the +// literal in place. Symmetric fix applied to all four inliner paths: +// fn-call simple, method-call simple, void-method simple, and the +// multi-stmt `try_inline_call` fn / method branches. + +// 1. The exact repro from the issue: numeric param captured by a method +// shorthand inside the returned object literal. +function show(label: string, d: Date) { + console.log(label, "getTime=", d.getTime(), " iso=", d.toISOString()); +} +function makeDT(y: number) { + return { + toDate(): Date { + return new Date(Date.UTC(y, 4, 15, 17, 29, 35, 402)); + }, + }; +} +const d1 = makeDT(2026).toDate(); +console.log("INLINE:", d1.getTime()); +show("HELPER:", d1); + +// 2. String capture — the dangerous trivial exprs are +// Integer/Number/Bool/String/Null/Undefined; string literal coverage. +function makeGreeter(name: string) { + return { + say(): string { + return "hello, " + name; + }, + }; +} +console.log("STR:", makeGreeter("ralph").say()); + +// 3. Multiple primitives captured at once. +function makeAdder(a: number, b: number, label: string) { + return { + sum(): string { + return label + ":" + (a + b); + }, + }; +} +console.log("MULTI:", makeAdder(2, 3, "two+three").sum()); + +// 4. Arrow-form closure (not method shorthand) — same inliner path. +function makeArrow(y: number) { + return () => y * 7; +} +console.log("ARROW:", makeArrow(6)()); + +// 5. Closure capture nested inside an object literal *value* expression. +function makeBox(n: number) { + return { + wrap: (label: string) => label + ":" + (n * 2), + }; +} +console.log("ARROW-IN-OBJ:", makeBox(21).wrap("doubled")); + +// 6. Captures used in array .map — passes through the same inliner. +function makeMapper(factor: number) { + return { + run(xs: number[]): number[] { + return xs.map((x) => x * factor); + }, + }; +} +console.log("MAP:", makeMapper(10).run([1, 2, 3]).join(",")); + +// 7. Multi-statement body (Pattern 2 of try_inline_simple_call): +// a const Let then a Return-with-closure. Pre-fix this had the same +// problem when the closure captured a param. +function makeMulti(y: number) { + const k = "yval"; + return { + get(): string { + return k + "=" + y; + }, + }; +} +console.log("MULTI-STMT:", makeMulti(123).get());