Skip to content

Commit fe3d705

Browse files
Ralph Kuepperclaude
andcommitted
fix(ui-slider): align FFI sig across all 7 backends with codegen (v0.5.326)
Codegen emits 3-arg `Slider(min, max, onChange)` per the TS surface, but every backend declared 4 args (`min, max, initial, on_change`) and read uninitialized register state for the missing `initial`. On macOS/iOS/Android/Windows the resulting NaN was silently clamped to min by AppKit/UIKit/Material/Win32. On GTK4 it flowed into `Adjustment::new(NaN, ...)` and triggered: GLib-GObject-CRITICAL: value "nan" of type 'gdouble' is invalid or out of range for property 'value' of type 'gdouble' Surfaced by docs/examples/ui/styling/visual_test.ts section 10 on Linux. IR dump confirms the call site emits 3 args; backends declared 4. Fix: drop `initial` from the FFI shim on the 5 native backends and the 2 JS runtime functions. Internal `widgets/slider.rs::create` keeps its 4-arg form so any future caller can pass an explicit initial. Default-to-min matches the existing implicit cross-platform behavior and the most common UX pattern. Verified: pre-fix visual_test printed the GLib-CRITICAL once before exit; post-fix it exits silent. Linux ui/ doc-tests: 44/45 pass (1 skip = pre-existing image_symbol banner exclusion). Version slot: bumped above origin's v0.5.325 (#219 doc-tests banner fix) per the merge precedent for parallel-track collisions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent f6e5fda commit fe3d705

10 files changed

Lines changed: 58 additions & 46 deletions

File tree

CLAUDE.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co
88

99
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.
1010

11-
**Current Version:** 0.5.325
11+
**Current Version:** 0.5.326
1212

1313
## TypeScript Parity Status
1414

@@ -149,6 +149,7 @@ First-resolved directory cached in `compile_package_dirs`; subsequent imports re
149149

150150
Keep entries to 1-2 lines max. Full details in CHANGELOG.md.
151151

152+
- **v0.5.326** — Fix latent calling-convention mismatch on `perry_ui_slider_create` across all 7 backends. Surfaced as `GLib-GObject-CRITICAL: value "nan" of type 'gdouble' is invalid or out of range for property 'value'` on Linux GTK4 when running `docs/examples/ui/styling/visual_test.ts` (section 10). Root cause: codegen at `crates/perry-codegen/src/lower_call.rs:5094` declares Slider with 3 args (`UiArgKind::F64, F64, Closure`) per the TS surface `Slider(min, max, onChange)` in `types/perry/ui/index.d.ts:224`, but every native backend's FFI shim declared 4 (`min, max, initial, on_change`) and called `widgets::slider::create(min, max, initial, on_change)`. Verified by `PERRY_NO_CACHE=1 PERRY_SAVE_LL=...` dump: emitted IR is `declare i64 @perry_ui_slider_create(double, double, double)` + `call i64 @perry_ui_slider_create(double 0.0, double 100.0, double %r480_callback)`. The 4th `initial` parameter at the FFI boundary therefore reads whatever the SysV ABI's xmm3 holds at the call site (uninitialized → frequently NaN). On macOS / iOS / Android / Windows that NaN flowed into `slider.setDoubleValue(NaN)` → silently clamped to min by AppKit/UIKit/Material/Win32 — no symptom. On GTK4 it flowed into `Adjustment::new(value=NaN, lower=0, upper=100, ...)` → GTK validated the range and emitted the loud warning. Fix: 5 native FFI shims (`crates/perry-ui-{gtk4,macos,ios,android,windows}/src/lib.rs`) drop the `initial` parameter and pass `min` to the internal `widgets::slider::create(min, max, min, on_change)`. Internal `widgets/slider.rs::create` keeps its 4-arg form for any future caller that wants explicit-initial semantics. Web (`crates/perry-codegen-js/src/web_runtime.js:275`) and wasm (`crates/perry-codegen-wasm/src/wasm_runtime.js:2579`) `perry_ui_slider_create` JS functions also drop their `initial` param — pre-fix Web set `el.value = undefined` (browser silently coerced to empty string then probably to min), wasm set `el.value = initial || 0` (worked coincidentally for `[0..N]` ranges, broke silently for `[10..100]`). Default-to-`min` matches the most common pattern (slider starts at the low end of its range) and the cross-backend behavior the existing native impls were already implicitly providing. Verified end-to-end on the visual_test repro: pre-fix `PERRY_UI_TEST_MODE=1 ./visual_test` printed the GLib-GObject-CRITICAL once before exit-0; post-fix the binary exits silent. Full Linux ui/ doc-tests: 44/45 pass / 0 fail / 1 skip (the lone skip is the pre-existing `image_symbol.ts` Linux banner exclusion). No regressions on the 12 other styling examples. Cross-backend FFI signatures now agree with codegen — closes the latent UB across all 7 platforms in one change. Version slot: bumped above origin's v0.5.325 (#219 doc-tests banner fix, parallel-track on origin) per the merge precedent for collisions.
152153
- **v0.5.325** — Closes #219: `docs/examples/platforms/wasm_snippets.ts` was failing every host doc-tests gate (macOS, GTK4, Windows) because its `// platforms: macos, linux, windows` banner enabled the host run phase, but the file's `declare function bloom_init_window/bloom_draw_rect` FFIs only resolve as WASM imports under `--target wasm` / `--target web` — on a native compile the host linker can't find them and bombs with `undefined reference to bloom_init_window`. Two-part fix: (1) `crates/perry-doc-tests/src/main.rs::read_banner` now tracks a `platforms_seen` flag — an explicit empty `// platforms:` directive (no values) suppresses the default-fill of `[macos, linux, windows]`, letting an example opt out of the host run phase entirely. Pre-fix the parser couldn't distinguish "directive absent" from "directive present but empty". (2) `docs/examples/platforms/wasm_snippets.ts` banner switched from `// platforms: macos, linux, windows` to `// platforms:` + `// targets: wasm, web` — the cross-compile phase still drives both targets (catching API drift in `declare function`, `fetch()`'s options shape, and `parallelMap`), but the host phase skips. Verified end-to-end: `./target/release/doc-tests --filter wasm_snippets --verbose` now reports `SKIP platform 'macos' not listed in banner` + `XCOMPILE_PASS target='wasm'` (504 ms) + `XCOMPILE_PASS target='web'` (101 ms); `strings target/perry-doc-tests/platforms_wasm_snippets__wasm.wasm` confirms `bloom_init_window` and `bloom_draw_rect` are present as WASM imports (per the issue's spot-check criterion). Full sweep `./target/release/doc-tests --skip-xcompile` now 78/80 pass with the lone fail being the pre-existing `ui/gallery.ts` retina screenshot baseline mismatch — unchanged from prior baselines, no regression from the parser change.
153154
- **v0.5.324** — Issue #185 follow-up: `box-shadow` on macOS (sections 4 of `docs/examples/ui/styling/visual_test.ts`) was invisible despite the FFI args being correct in the IR dump. Root cause: `crates/perry-ui-macos/src/widgets/mod.rs::set_shadow` set `shadowColor` / `shadowOpacity` / `shadowRadius` / `shadowOffset` on the view's CALayer but never set `setMasksToBounds: false` — and CALayer shadows are clipped by `masksToBounds`. The iOS / tvOS / visionOS mirror impls all set this; the macOS one was missing it. One-line fix: `let _: () = objc2::msg_send![layer, setMasksToBounds: false];` after the four shadow setters. Also adds (1) `docs/examples/ui/styling/visual_test.ts` — single-window comprehensive 13-section visual styling test exercising every styling prop family (colors hex/named/object/alpha/runtime, borders, padding, **shadow** (the case that surfaced the bug), gradient, opacity, typography, buttons, inputs, controls, image symbols, hidden/opacity-0 states, runtime-resolved colors via Phase C step 7); (2) `docs/examples/ui/styling/visual_test.spec.md` — LLM-debuggable per-cell expected-values manifest with 13 numbered sections matching the .ts and a "Visible signature" string per cell so a screenshot can be compared cell-by-cell against the spec; (3) `scripts/run_visual_test_check.sh` — drift check that asserts the .ts's `labeled("N. ...", ...)` calls and the spec's `### N.` headers stay 1:1 in lockstep, fails CI when either file adds/removes a row without updating the other; (4) wired into `.github/workflows/test.yml` after the existing UI styling matrix step. Verified end-to-end via 3 rounds of `shadow_repro.ts` (high-contrast yellow shadow against gray cells) — round 1 was inconclusive low-contrast, round 2 confirmed shadows missing, round 3 (post-fix) confirmed shadows render correctly. Cross-platform compile sanity check: web/wasm + iOS-simulator both produce valid binaries from the same `visual_test.ts` source; tvOS/watchOS sim builds need nightly Rust + `-Zbuild-std` (tier-3 targets), deferred. iOS simulator launch crash observed by user — investigation deferred so Windows/Linux validation can run; the macOS fix here is independently shippable and only touches the macOS UI crate.
154155
- **v0.5.323** — Closes #212: class declared inside a function whose method body references an enclosing-fn local now compiles + runs correctly (pre-fix: `Error compiling module ... lowering body of method 'C::log': ArrayPush(0): local not in scope`). The codegen emitted each class method as a stand-alone Function with no capture wiring, so `class C { log(s) { captured.push(s); } }` inside `function test() { const captured = []; ... }` failed at the LocalGet site for `captured`. Fix is a HIR-level rewrite in `lower_class_decl` (`crates/perry-hir/src/lower_decl.rs`): (1) walk every instance member (methods, getters, setters, constructor) for outer-scope LocalIds via the new `collect_method_captures` helper — same `collect_local_refs_stmt` walk the existing closure capture analysis uses, filtered against a snapshot of `ctx.locals` taken at class-decl-end so inner-closure params (which `collect_local_refs_*` descends through indiscriminately) don't get falsely captured. (2) Add a hidden `__perry_cap_<outer_id>` instance field per captured outer id; field name is keyed off the OUTER id so every method/ctor agrees on which field reads which capture, independent of per-method fresh ids. (3) For each method/getter/setter, allocate a FRESH method-local `LocalId` per captured outer id, run a global LocalId remap on the body (the new `analysis::remap_local_ids_in_stmts_with_field_propagation`, which mirrors `perry_transform::inline::substitute_locals`'s variant coverage so specialized HIR shapes like `Expr::ArrayJoin { array }`, `Expr::ArrayMap`, etc. don't silently fall through and skip the rewrite — the soft-fallback `double_literal(0.0)` for unrecognized LocalIds was producing array handle 0 at runtime), then prepend `Stmt::Let { id: fresh_id, init: PropertyGet(This, "__perry_cap_<outer_id>") }`. The body's existing `LocalGet(outer_id)` now resolves to a method-local slot at codegen. PER-METHOD fresh ids are essential — a `Stmt::Let { id: outer_id }` in a method that has a closure mutating the captured value would mark `outer_id` as boxed *globally* via the module-wide `module_boxed_vars` union, which then makes the outer fn's plain (non-boxed) read of `outer_id` segfault on a `js_box_get` of a non-box pointer. The rebind let preserves the outer's `Type` (looked up in `ctx.locals`) so typed-array fast paths like `out.length` / `out[i]` keep firing instead of falling off into generic by-name dispatch. (4) Constructor: append a fresh-id param per captured outer id, prepend `this.__perry_cap_<outer_id> = LocalGet(fresh_param_id)` after any `super(...)` call (so derived classes initialize `this` first), and apply the same body-remap. (5) Register the class in a new `LoweringContext::class_captures` registry; the `Expr::New { class_name }` lowering site (`crates/perry-hir/src/lower.rs`) now appends `LocalGet(outer_id)` per captured outer id at every construction site (the outer scope's actual id, since we're lowering inside it). Static methods aren't included because they have no `this` to read captures from — if a static method body references an outer local, the original codegen error fires (out of scope for #212). Mutation note: every top-level `LocalSet(outer_id, v)` and `Expr::Update { id: outer_id, .. }` in a method body is wrapped in a `Sequence` that also writes through to `this.__perry_cap_<id>` via the new `analysis::remap_local_ids_in_stmts_with_field_propagation`, so subsequent method calls re-reading the field see the latest value (the canonical `set value(v) { stored = v; } get value() { return stored; }` pattern works — getter after setter sees the just-written value). Mutations still don't propagate back to the OUTER scope's slot — `stored` in the enclosing function still reads its own original snapshot — that's the residual JS divergence. The propagation only fires at top-level expression positions (statement, return value, condition); deeply-nested captured writes like `(stored = v).toString()` only update the local copy. Reference-type mutation (array.push, obj.x = ...) works regardless because the method-local copy and the outer binding hold the same reference. Side effect: removed the v0.5.317 silent-drop guard for `[Symbol.dispose]` / `[Symbol.asyncDispose]` methods at `lower_decl.rs:744` — the dispose family was being dropped when nested in a function and capturing outer locals (the `disposed: string[]` pattern in `test_gap_async_advanced.ts`), which is now redundant: the same hidden-field rewrite that lets `log() { captured.push(...) }` work also lets `[Symbol.dispose]() { disposed.push(this.label); }` work. Inheritance: `class Derived extends Base` where Base has captures works too — at child class lowering, the parent's `__perry_cap_<id>` field declarations are detected via `lookup_class_field_names` and skipped in the child's `fields` (otherwise the keys array would carry two same-named entries at different offsets, the parent's method body would read its index while the child's ctor wrote to the child's index — the inherited-class-shared-capture bug). The child's synthesized ctor still takes the inherited capture as a param + emits `this.__perry_cap_<id> = LocalGet(param)`; the runtime's by-name PropertySet writes to the (single) parent-declared field. For disjoint-capture inheritance (Base captures one local, Derived captures a different one), the child's lowering unions the parent's `class_captures` registry into its own captures_vec — without this, the child's synthesized ctor wouldn't take the parent's capture as a param and the parent's field would never initialize. New regression test `test-files/test_issue_212_class_method_capture.ts` (10 cases — issue's exact repro, multi-capture, user-written ctor with capture-using body, multiple instances per outer-fn invocation, nested closure inside capturing method, dispose-hook pattern from #154, shared-capture inheritance, disjoint-capture inheritance, captured-primitive reassignment via setter/getter, async method with capture — all byte-for-byte against `node --experimental-strip-types`). Existing `test_issue_154_using_dispose.ts` also still passes byte-for-byte; gap tests still 26/28 (no regressions); doc-tests 79/80 (the lone fail is the pre-existing retina screenshot baseline mismatch in `ui/gallery.ts`). `analysis::remap_local_ids_in_stmts` is a new public HIR helper documented to require a new arm whenever a HIR variant carrying a LocalId-bearing sub-expr is added — same class of bug as the one fixed here. Version slot: bumped above v0.5.322 (GTK4 FFIs, #216/#217/#218) per the merge precedent for collisions — origin's pull-and-cherry-pick on top of my v0.5.322 commit ate it before push, so this re-applies as v0.5.323.

Cargo.lock

Lines changed: 27 additions & 27 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ opt-level = "s" # Optimize for size in stdlib
109109
opt-level = 3
110110

111111
[workspace.package]
112-
version = "0.5.325"
112+
version = "0.5.326"
113113
edition = "2021"
114114
license = "MIT"
115115
repository = "https://github.com/PerryTS/perry"

crates/perry-codegen-js/src/web_runtime.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -272,12 +272,12 @@ function perry_ui_toggle_create(label, callback) {
272272
return wrapWidget(allocHandle(wrapper));
273273
}
274274

275-
function perry_ui_slider_create(min, max, initial, callback) {
275+
function perry_ui_slider_create(min, max, callback) {
276276
const el = document.createElement("input");
277277
el.type = "range";
278278
el.min = min;
279279
el.max = max;
280-
el.value = initial;
280+
el.value = min; // codegen emits 3-arg Slider(min, max, onChange); default initial=min
281281
el.step = "any";
282282
if (typeof callback === "function") {
283283
el.addEventListener("input", () => callback(parseFloat(el.value)));

crates/perry-codegen-wasm/src/wasm_runtime.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2576,9 +2576,10 @@ function perry_ui_toggle_create(label, callback) {
25762576
});
25772577
return uiAlloc(wrap);
25782578
}
2579-
function perry_ui_slider_create(min, max, initial, callback) {
2579+
function perry_ui_slider_create(min, max, callback) {
2580+
// Codegen emits 3-arg Slider(min, max, onChange); default initial=min
25802581
const el = document.createElement("input"); el.type = "range";
2581-
el.min = min || 0; el.max = max || 100; el.value = initial || 0; el.step = "any";
2582+
el.min = min || 0; el.max = max || 100; el.value = el.min; el.step = "any";
25822583
el._perryCallback = callback;
25832584
el.addEventListener("input", () => {
25842585
if (el._perryCallback !== undefined) callWasmClosure(el._perryCallback, parseFloat(el.value));

0 commit comments

Comments
 (0)