Skip to content

fix(hir): #1001 — restore built-in static identity in member position (regression from #973) - #1007

Merged
proggeramlug merged 1 commit into
mainfrom
fix/number-static-method-identity
May 18, 2026
Merged

fix(hir): #1001 — restore built-in static identity in member position (regression from #973)#1007
proggeramlug merged 1 commit into
mainfrom
fix/number-static-method-identity

Conversation

@proggeramlug

Copy link
Copy Markdown
Contributor

Fixes #1001.

Problem

Number.parseFloat === parseFloat and Number.parseInt === parseInt evaluated to false (Node/spec: true). More broadly, any built-in wrapper-constructor static accessed as a member value (Number.*, Object.*, Array.*, String.*, …) lost identity with its intrinsic global. This regressed the pre-existing core gap test test-files/test_gap_number_math.ts against node --experimental-strip-types.

Root cause (bisected on Windows)

Introduced by 5ddccbbcfeat(runtime): .constructor property on Date/Array/Object instances (#973).

Commit Number.parseFloat === parseFloat
7586a920 (#968) ✅ true
43cd9ffe (#970) ✅ true
5ddccbbc (#973) false
94d419f1 (#975) … current 17e99ea7 ❌ false

#973 added an HIR lowering arm (is_builtin_global_value_name in crates/perry-hir/src/lower.rs) that rewrites bare built-in identifiers used as values to PropertyGet { GlobalGet(0), name }, so identity checks like inst.constructor === Date resolve both sides to the same populate_global_this_builtins closure. That intent is correct. The bug: the rewrite also fired when the built-in ident was the object of a member access, so Number.parseFloat became globalThis.Number.parseFloat rather than the intrinsic static — no longer the same value as the intrinsic global parseFloat.

Fix

crates/perry-hir/src/lower/expr_member.rs: after lowering the member object, detect the case where #973's reroute fired on a member-object built-in ident and revert that object to the intrinsic Expr::GlobalGet(0) (pre-#973 behavior). Strictly limited to member-object position, so #973's bare-ident-as-value feature is untouched. Local shadowing is inherently safe — a shadowing local lowers to LocalGet and never reaches this reroute.

Validation

  • Number.parseFloat === parseFloat / parseInttrue
  • test_gap_number_math byte-identical to the known-good v0.5.969 / Node output
  • feat(runtime): .constructor on Date/Array/Object instances #973's own test_constructor_property.ts (.constructor === Date/Array/Object) still fully passes
  • Builtin-heavy compile+run sweep green (Object.keys, Array.isArray, String.fromCharCode, Number.* statics as values and calls)

Out of scope

const f = Number.parseFloat; f(x) throws TypeError: value is not a function — a separate, pre-existing limitation (fails identically on pre-#973 v0.5.969), not addressed here.

… (regression from #973)

`Number.parseFloat === parseFloat` (and every built-in
wrapper-constructor static accessed as a member value) regressed to
`false`; Node/spec is `true`. Also regressed the core gap test
test_gap_number_math vs node --experimental-strip-types.

Bisected to 5ddccbb (feat(runtime): .constructor property on
Date/Array/Object instances, #973): its HIR arm rewrites bare built-in
idents used as VALUES to PropertyGet{GlobalGet(0),name} for
`inst.constructor === Date` identity. That fired in member-OBJECT
position too, so `Number.parseFloat` resolved via globalThis.Number
rather than the intrinsic static.

Fix in expr_member.rs: after lowering the member object, revert #973's
reroute to the intrinsic GlobalGet(0) when it fired on a member-object
built-in ident. Bare-ident-as-value (#973's feature) untouched; local
shadowing inherently safe (shadowing local lowers to LocalGet).
@proggeramlug
proggeramlug force-pushed the fix/number-static-method-identity branch from bc8c15e to deb3c4e Compare May 18, 2026 09:42
@proggeramlug
proggeramlug merged commit f78361f into main May 18, 2026
@proggeramlug
proggeramlug deleted the fix/number-static-method-identity branch May 18, 2026 09:42
proggeramlug added a commit that referenced this pull request May 19, 2026
…HIR collapse (#1064)

`const [a, b] = await Promise.all([fn1(), fn2()])` returned undefined
under Perry HEAD=13dc587a (v0.5.1004, pre-#1007). Bisected: the bug
disappears at f78361f (#1007 — restore built-in static identity in
member position), which collapses `Promise.all` from
`PropertyGet { PropertyGet { GlobalGet(0), "Promise" }, "all" }` down
to `PropertyGet { GlobalGet(0), "all" }`. The codegen Promise-static
dispatch at that commit only matched the bare `GlobalGet(_)` receiver,
so the deeper shape fell through to `js_native_call_method("all", …)`
against the Promise constructor — returning 0.0 (no such method on a
function value) and breaking the destructure.

Current main is doubly-protected: (1) the #1007 HIR collapse keeps
the shallow shape, and (2) PR #1030 introduced `is_global_constructor_expr`,
which accepts both shapes at the codegen dispatch. No code change is
required — the bug is fixed in current main as a side effect of
#1007 + #1030. This test pins the byte-for-byte node match so a
future regression of either piece (the HIR collapse or the helper)
fails immediately. Closes #1013.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Number.parseFloat === parseFloat is false (regression #973): built-in static identity broken in member position

1 participant