Skip to content

Commit ce550d9

Browse files
committed
fix(codegen): #142 Math.tan/asin/acos/atan/atan2 silent identity (v0.5.177)
Merged via conflict-resolved cherry-pick of PR #148 (original branch cut at v0.5.164; main had advanced to v0.5.176 so the CLAUDE.md / Cargo.toml / Cargo.lock bumps had to be taken from main and reapplied at v0.5.177). The five codegen arms at crates/perry-codegen/src/expr.rs:5417-5424 fell through to lower_expr(ctx, o), returning the argument unchanged. Runtime functions (js_math_tan/asin/acos/atan/atan2 in perry-runtime/src/math.rs) and extern declarations (runtime_decls.rs:1482-1493) were already present — only the codegen wiring was missing. Replaced the fall-through with calls to the existing runtime symbols, matching the shape of the sinh/cosh/tanh arms three lines above. atan2 evaluates y first then x (JS arg order) and passes both to js_math_atan2. Verified against node --experimental-strip-types: Math.tan(1) = 1.557… (was 1), Math.atan2(0,-1) = π (was -1), Math.asin(1) = π/2, Math.acos(1) = 0, Math.atan(1) = π/4 — all within last-digit libm rounding. Original author: Ralph Küpper <ralph.kuepper@skelpo.com> (PR #148).
1 parent 4428e6f commit ce550d9

4 files changed

Lines changed: 52 additions & 36 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.176
11+
**Current Version:** 0.5.177
1212

1313
## TypeScript Parity Status
1414

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

148148
Keep entries to 1-2 lines max. Full details in CHANGELOG.md.
149149

150+
- **v0.5.177** — Fix #142 via PR #148: `Math.tan`/`asin`/`acos`/`atan`/`atan2` were lowering to silent identity functions. The five arms at `crates/perry-codegen/src/expr.rs:5417-5424` had an old "no runtime wrappers yet, no LLVM intrinsics for these" comment and fell through to `lower_expr(ctx, o)` — returning the argument unchanged with no diagnostic. The runtime functions (`js_math_tan/asin/acos/atan/atan2` in `perry-runtime/src/math.rs:46-62`, each a thin `f64::tan()` etc.) and the extern declarations (`runtime_decls.rs:1488-1499`) had actually been in place for a while; the codegen wiring was just missing. Replaced the fall-through with five `ctx.block().call(DOUBLE, "js_math_*", ...)` arms matching the shape of the already-working `sinh/cosh/tanh` arms three lines above. `atan2(y, x)` evaluates y first then x (JS left-to-right argument order) and passes both to `js_math_atan2`. Verified against Node `--experimental-strip-types`: `Math.tan(1) = 1.557…` (was `1`), `Math.atan2(0,-1) = π` (was `-1`), `Math.asin(1) = π/2`, `Math.acos(1) = 0`, `Math.atan(1) = π/4` — all within last-digit libm rounding. Merged via conflict-resolved cherry-pick since the PR branch was cut at v0.5.164 and `main` had advanced to v0.5.176.
150151
- **v0.5.176** — Fix #158: `Map`/`Set` now treat `-0` and `+0` as the same key (SameValueZero, 23.1.3.9 / 23.2.3.1). Added a `normalize_zero(v: f64) -> f64` helper in `crates/perry-runtime/src/map.rs` and `set.rs` that rewrites `-0` bits (`0x8000_0000_0000_0000`) to `+0` via an `v == 0.0` IEEE check — safe against NaN-boxed tagged values because any tag in the upper 16 bits makes the f64 a NaN and `NaN == 0.0` is false. Wired into `js_map_set` / `_get` / `_has` / `_delete` and `js_set_add` / `_has` / `_delete` so both insert and lookup paths normalize. Previously `numMap.set(0,"a"); numMap.set(-0,"b")` yielded size=2 with `get(0)` returning "a"; now yields size=1 with `get(0)`="b" matching Node's `--experimental-strip-types`.
151152
- **v0.5.175** — Close two review-flagged bypasses introduced by Phase 3 / perry/thread compile-time work. (1) `new Error(msg, { cause })` shorthand + paren-wrapped options. The AST-level extraction added to `crates/perry-hir/src/lower.rs:10363-10386` to survive Phase 3 (anon-shape synthesis converts `{cause: e}` into `Expr::New { __AnonShape_N }` before the HIR Object-match would see it) only handled `Prop::KeyValue` at the outer Object — so the canonical ES2022 form `catch (cause) { throw new Error('msg', { cause }) }` (shorthand) and `new Error('msg', ({ cause: e }))` (paren-wrapped) both silently lost `.cause` and emitted a plain `ErrorNew(Some(msg))`. Fix: peel `Expr::Paren` before matching and add a `Prop::Shorthand(ident)` arm that resolves the ident via `ctx.lookup_func` / `lookup_local` / `lookup_class` the same way `lower.rs`'s own Object-literal lowering does. Verified: both repros now print the expected cause message and match node byte-for-byte. (2) `perry/thread` named-function bypass. The v0.5.174 mutable-capture check only pattern-matched `Expr::Closure`, so `function worker(n){counter++;} parallelMap(xs, worker)` (semantically identical to the rejected inline form) fell straight through and compiled silently — defeating the compile-time safety claim. FnCtx doesn't carry the full HIR function table (only `func_names: FuncId → String`), so we can't cheaply resolve the callee body at codegen time. Conservative fix: when the callback is `Expr::FuncRef` / `Expr::LocalGet` / `Expr::ExternFuncRef`, bail with a diagnostic that names the method + points at the inline-closure workaround (`parallelMap(xs, (x) => myFn(x))`) + links `docs/src/threading/overview.md#no-shared-mutable-state`. Pure function workers that don't actually need the analysis still have to wrap but the wrap is trivial; unsafe named workers that silently lost writes are now loud compile errors. Verified against the ultrareview's exact repro (`let counter=0; function worker(n){counter++;...} parallelMap([1..4], worker)` → compile bail). Proper long-term fix (walk FuncRef body via a HIR-time pass with the full function table in scope) tracked for follow-up — the conservative bail is sound in the meantime. No codegen emitted change for the closure-ok path; gap tests steady at 22/28; HIR tests 40/40.
152153
- **v0.5.174** — Fix #146: perry/thread primitives now actually work, and mutable outer captures are rejected at compile time. Before this bump, `parallelMap` / `parallelFilter` / `spawn` imported from `perry/thread` flowed into HIR as `NativeMethodCall { module: "perry/thread", method, object: None }`, but `perry-codegen/src/lower_call.rs`'s `NATIVE_MODULE_TABLE` had zero rows for that module — so receiver-less dispatch missed, fell through to the TAG_UNDEFINED early-out, and every call silently returned `undefined`. The runtime side (`js_thread_parallel_map` / `js_thread_parallel_filter` / `js_thread_spawn` in `perry-runtime/src/thread.rs`) had been in place for a while with no callers. Also fixed: the "compile-time safety" claim in `perry-runtime/src/thread.rs:99-100` / `docs/src/threading/overview.md` / `docs/src/introduction.md` was not backed by any pass — codegen had `let _ = mutable_captures;` that silenced a warning and discarded the field. Wired up three `NativeModSig` rows (`perry/thread` → `parallelMap`/`parallelFilter`/`spawn`, all `args: &[NA_F64, NA_F64]` or `[NA_F64]`, `ret: NR_F64`) plus three extern decls in `runtime_decls.rs`. For the compile-time safety half, added a mutable-capture check inline in `lower_native_method_call`'s receiver-less dispatch that walks the closure body for `LocalSet` / `Update` writing to any LocalId not introduced inside the body (params or `let`s). Can't use the closure's own `mutable_captures` field alone: HIR lowering drops module-level LocalIds from `captures` via `filter_module_level_captures` (see `lower.rs:457`, v0.5.91-era fix for `const f = () => f(...)` sibling-closure races), so `let counter = 0; parallelMap(data, () => counter++)` ends up with `captures: [], mutable_captures: []` at the HIR even though the body writes to `counter`. New helpers `collect_closure_introduced_ids` + `find_outer_writes_{stmt,expr}` walk the body themselves, stop at nested closure boundaries (those get their own check if threaded), and bail with a message naming the method + LocalId + pointing at `docs/src/threading/overview.md#no-shared-mutable-state`. Verified end-to-end: `parallelMap([1,2,3,4], x => x*10)` now returns `[10,20,30,40]` (was `undefined`); `parallelFilter` / `spawn` same shape; `let c = 0; parallelMap(d, () => c++)` now errors at compile time with "closure passed to `parallelMap` writes to outer variable (LocalId 0)"; `const rate = 1.08; parallelMap(d, x => x*rate)` still compiles (const value captures are safe — deep-copied snapshot). Tests: new `docs/examples/runtime/thread_primitives.ts` + `_expected/runtime/thread_primitives.stdout` covers the runtime half end-to-end via the existing doc-tests stdout-diff harness. New `scripts/run_thread_tests.sh` covers the compile-error half: 3 mutation cases that must fail compilation with the expected substring + 1 const-capture case that must succeed, wired into `.github/workflows/test.yml` right after the `test-files/*.ts` compile-smoke loop. CI catches regressions in both halves: drop a NATIVE_MODULE_TABLE row → `thread_primitives.stdout` diff fails; drop the mutable-capture check → `run_thread_tests.sh` fails all three expected-error cases.

Cargo.lock

Lines changed: 26 additions & 26 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
@@ -104,7 +104,7 @@ opt-level = "s" # Optimize for size in stdlib
104104
opt-level = 3
105105

106106
[workspace.package]
107-
version = "0.5.176"
107+
version = "0.5.177"
108108
edition = "2021"
109109
license = "MIT"
110110
repository = "https://github.com/PerryTS/perry"

crates/perry-codegen/src/expr.rs

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5412,15 +5412,30 @@ pub(crate) fn lower_expr(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
54125412
let v = lower_expr(ctx, o)?;
54135413
Ok(ctx.block().call(DOUBLE, "js_math_tanh", &[(DOUBLE, &v)]))
54145414
}
5415-
// tan/asin/acos/atan: still stubs returning input (runtime has
5416-
// no wrappers yet, no LLVM intrinsics for these).
5417-
Expr::MathTan(o)
5418-
| Expr::MathAsin(o)
5419-
| Expr::MathAcos(o)
5420-
| Expr::MathAtan(o) => lower_expr(ctx, o),
5415+
Expr::MathTan(o) => {
5416+
let v = lower_expr(ctx, o)?;
5417+
Ok(ctx.block().call(DOUBLE, "js_math_tan", &[(DOUBLE, &v)]))
5418+
}
5419+
Expr::MathAsin(o) => {
5420+
let v = lower_expr(ctx, o)?;
5421+
Ok(ctx.block().call(DOUBLE, "js_math_asin", &[(DOUBLE, &v)]))
5422+
}
5423+
Expr::MathAcos(o) => {
5424+
let v = lower_expr(ctx, o)?;
5425+
Ok(ctx.block().call(DOUBLE, "js_math_acos", &[(DOUBLE, &v)]))
5426+
}
5427+
Expr::MathAtan(o) => {
5428+
let v = lower_expr(ctx, o)?;
5429+
Ok(ctx.block().call(DOUBLE, "js_math_atan", &[(DOUBLE, &v)]))
5430+
}
54215431
Expr::MathAtan2(y, x) => {
5422-
let _ = lower_expr(ctx, y)?;
5423-
lower_expr(ctx, x)
5432+
let y_v = lower_expr(ctx, y)?;
5433+
let x_v = lower_expr(ctx, x)?;
5434+
Ok(ctx.block().call(
5435+
DOUBLE,
5436+
"js_math_atan2",
5437+
&[(DOUBLE, &y_v), (DOUBLE, &x_v)],
5438+
))
54245439
}
54255440

54265441
// -------- String.fromCharCode(code) --------

0 commit comments

Comments
 (0)