-
-
Notifications
You must be signed in to change notification settings - Fork 161
fix(hir): keep the reified receiver for a computed dynamic key on a builtin namespace #10629
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| Fixed `Math[k]`, `JSON[k]`, `Object[k]`, `Number[k]`, `Reflect[k]`, `Date[k]`, `String[k]`, and | ||
| other built-in namespace/constructor member reads with a *variable* (non-literal) computed key | ||
| reading a property of the number `0` instead of the real object — `Math[key](x)` threw | ||
| `(number).x is not a function`, and `typeof Math[key]` was `undefined` for every key. #973's | ||
| value-form reroute of bare built-in identifiers was correctly undone in member-object position for | ||
| a statically-known member name (`Math.max(...)`), so the intrinsic call/constant-fold paths could | ||
| keep their pre-#973 `GlobalGet(0)` receiver — but the undo also fired for a computed non-literal | ||
| key, where nothing can resolve to an intrinsic at lowering time, collapsing the receiver to the | ||
| bare `GlobalGet(0)` sentinel. Closed #6677 fixed only the string-literal direct-call form | ||
| (`Math["max"](...)`); this fixes the variable-key forms (call and value-read position, keys from a | ||
| variable, a template literal, or a function return) it left broken. Static (literal) key paths are | ||
| unaffected by construction. `#10483` |
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
155 changes: 155 additions & 0 deletions
155
test-files/test_gap_10483_computed_key_namespace_member.ts
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,155 @@ | ||
| // #10483: `Math[k]` / `JSON[k]` / `Object[k]` / ... with a *variable* (non-literal) | ||
| // computed key must resolve against the real namespace/constructor object, not the | ||
| // number 0. #973 rerouted bare builtin idents used as VALUES to | ||
| // `PropertyGet{GlobalGet(0), name}`; member_tail.rs undoes that reroute in | ||
| // member-OBJECT position so the intrinsic call / constant-fold paths for a | ||
| // STATICALLY-KNOWN member name (`Math.max(...)`) keep their pre-#973 `GlobalGet(0)` | ||
| // receiver. That undo is wrong for a computed non-literal key — nothing can resolve | ||
| // to an intrinsic at lowering time, so the receiver collapsed to the bare `GlobalGet(0)` | ||
| // sentinel and the read landed on the number 0. Closed #6677 fixed only the | ||
| // string-literal direct-call form (`Math["max"](...)`); this covers the variable-key | ||
| // forms it left broken, in both call position and value-read position, with keys from | ||
| // a plain variable, a template literal, and a function return. Static (literal) key | ||
| // paths are re-asserted unchanged at the end — that's the entire point of the | ||
| // reroute-undo this fix narrows (test_gap_number_math regressed when #973 first | ||
| // landed). | ||
|
|
||
| // --------------------------------------------------------------------------- | ||
| // call position, key from a plain variable | ||
| // --------------------------------------------------------------------------- | ||
| function callMath(key: string, ...args: number[]): number { | ||
| // @ts-ignore -- deliberately untyped computed access, the #10483 repro shape | ||
| return Math[key](...args); | ||
| } | ||
| console.log("call/variable:"); | ||
| console.log(callMath("max", 1, 5, 3)); | ||
| console.log(callMath("min", 1, 5, 3)); | ||
| console.log(callMath("round", 2.6)); | ||
|
|
||
| function callJson(key: string, value: unknown): string { | ||
| // @ts-ignore | ||
| return JSON[key](value); | ||
| } | ||
| console.log(callJson("stringify", { a: 1, b: [1, 2, 3] })); | ||
|
|
||
| function callReflect(key: string, target: object, prop: string): boolean { | ||
| // @ts-ignore | ||
| return Reflect[key](target, prop); | ||
| } | ||
| console.log(callReflect("has", { x: 1 }, "x")); | ||
| console.log(callReflect("has", { x: 1 }, "y")); | ||
|
|
||
| function callNumber(key: string, value: number): boolean { | ||
| // @ts-ignore | ||
| return Number[key](value); | ||
| } | ||
| console.log(callNumber("isInteger", 5)); | ||
| console.log(callNumber("isInteger", 5.5)); | ||
|
|
||
| function callArray(key: string, value: unknown): boolean { | ||
| // @ts-ignore | ||
| return Array[key](value); | ||
| } | ||
| console.log(callArray("isArray", [1, 2])); | ||
| console.log(callArray("isArray", {})); | ||
|
|
||
| function callString(key: string, ...codes: number[]): string { | ||
| // @ts-ignore | ||
| return String[key](...codes); | ||
| } | ||
| console.log(callString("fromCharCode", 72, 105)); | ||
|
|
||
| function callDate(key: string): boolean { | ||
| // @ts-ignore | ||
| return typeof Date[key]() === "number"; | ||
| } | ||
| console.log(callDate("now")); | ||
|
|
||
| // --------------------------------------------------------------------------- | ||
| // value-read position (not called), key from a plain variable | ||
| // --------------------------------------------------------------------------- | ||
| console.log("value-read/variable:"); | ||
| function readTypeof(obj: unknown, key: string): string { | ||
| // @ts-ignore | ||
| return typeof obj[key]; | ||
| } | ||
| console.log(readTypeof(Math, "max")); | ||
| console.log(readTypeof(JSON, "stringify")); | ||
| console.log(readTypeof(Reflect, "ownKeys")); | ||
| console.log(readTypeof(Number, "isInteger")); | ||
| console.log(readTypeof(Date, "now")); | ||
| console.log(readTypeof(Array, "isArray")); | ||
| console.log(readTypeof(String, "fromCharCode")); | ||
|
|
||
| function grabMax(key: string): (...xs: number[]) => number { | ||
| // @ts-ignore | ||
| return Math[key]; | ||
| } | ||
| const maxFn = grabMax("max"); | ||
| console.log(typeof maxFn, maxFn(7, 42, 3)); | ||
|
|
||
| // --------------------------------------------------------------------------- | ||
| // key from a template literal | ||
| // --------------------------------------------------------------------------- | ||
| console.log("template-literal key:"); | ||
| function templateCall(part: string): number { | ||
| // @ts-ignore | ||
| return Math[`${part}`](4, 9); | ||
| } | ||
| console.log(templateCall("max")); | ||
| console.log(templateCall("min")); | ||
|
|
||
| function templateRead(part: string): string { | ||
| // @ts-ignore | ||
| return typeof JSON[`${part}`]; | ||
| } | ||
| console.log(templateRead("parse")); | ||
|
|
||
| // --------------------------------------------------------------------------- | ||
| // key from a function return | ||
| // --------------------------------------------------------------------------- | ||
| console.log("function-return key:"); | ||
| function pickMaxKey(): string { | ||
| return "max"; | ||
| } | ||
| // @ts-ignore | ||
| console.log(Math[pickMaxKey()](4, 9)); | ||
|
|
||
| function pickIsIntegerKey(): string { | ||
| return "isInteger"; | ||
| } | ||
| // @ts-ignore | ||
| console.log(Number[pickIsIntegerKey()](10)); | ||
|
|
||
| // --------------------------------------------------------------------------- | ||
| // static (literal) key controls — must stay byte-identical to before #10483 | ||
| // --------------------------------------------------------------------------- | ||
| console.log("static controls:"); | ||
| console.log(Math.max(1, 5, 3)); | ||
| console.log(Math["max"](1, 5, 3)); | ||
| console.log(Math.min(1, 5, 3)); | ||
| console.log(JSON.stringify({ z: 1 })); | ||
| console.log(JSON["stringify"]({ z: 1 })); | ||
| console.log(Number.parseInt("42", 10)); | ||
| console.log(Number.isInteger(5)); | ||
| console.log(Array.isArray([1])); | ||
| console.log(Reflect.has({ x: 1 }, "x")); | ||
|
|
||
| // --------------------------------------------------------------------------- | ||
| // `console`'s pre-existing dynamic dispatch (#js_console_method_by_value, | ||
| // the Next.js `prefixedLog` fix) must keep working — #10483's new guard | ||
| // deliberately excludes "console" so the call-position undo it depends on | ||
| // still fires. | ||
| // --------------------------------------------------------------------------- | ||
| console.log("console dynamic dispatch:"); | ||
| function consoleCall(method: string, msg: string): void { | ||
| // @ts-ignore | ||
| console[method](msg); | ||
| } | ||
| consoleCall("log", "console[method](...) still dispatches"); | ||
|
|
||
| function consoleValueRead(method: string): string { | ||
| // @ts-ignore | ||
| return typeof console[method]; | ||
| } | ||
| console.log(consoleValueRead("log")); | ||
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: PerryTS/perry
Length of output: 13560
Add direct dynamic value reads for each builtin receiver.
readTypeofevaluatesobj[key]withobjas a local parameter. Its calls do not exercise builtin receiver lowering. The fixture already directly reads computed members onMathandJSON, sograbMaxis not the only such test. It lacks direct value-read coverage forObject,Reflect,Number,Date,Array, andString.Proposed test direction
🤖 Prompt for AI Agents