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).
bc8c15e to
deb3c4e
Compare
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.