fix(hir): #1001 — restore built-in static identity in member position (regression from #973) - #1007
Merged
Merged
Conversation
… (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
force-pushed
the
fix/number-static-method-identity
branch
from
May 18, 2026 09:42
bc8c15e to
deb3c4e
Compare
This was referenced May 18, 2026
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1001.
Problem
Number.parseFloat === parseFloatandNumber.parseInt === parseIntevaluated tofalse(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 testtest-files/test_gap_number_math.tsagainstnode --experimental-strip-types.Root cause (bisected on Windows)
Introduced by
5ddccbbc—feat(runtime): .constructor property on Date/Array/Object instances (#973).Number.parseFloat === parseFloat7586a920(#968)43cd9ffe(#970)5ddccbbc(#973)94d419f1(#975) … current17e99ea7#973 added an HIR lowering arm (
is_builtin_global_value_nameincrates/perry-hir/src/lower.rs) that rewrites bare built-in identifiers used as values toPropertyGet { GlobalGet(0), name }, so identity checks likeinst.constructor === Dateresolve both sides to the samepopulate_global_this_builtinsclosure. That intent is correct. The bug: the rewrite also fired when the built-in ident was the object of a member access, soNumber.parseFloatbecameglobalThis.Number.parseFloatrather than the intrinsic static — no longer the same value as the intrinsic globalparseFloat.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 intrinsicExpr::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 toLocalGetand never reaches this reroute.Validation
Number.parseFloat === parseFloat/parseInt→truetest_gap_number_mathbyte-identical to the known-good v0.5.969 / Node outputtest_constructor_property.ts(.constructor === Date/Array/Object) still fully passesOut of scope
const f = Number.parseFloat; f(x)throwsTypeError: value is not a function— a separate, pre-existing limitation (fails identically on pre-#973 v0.5.969), not addressed here.