fix: #5735 — declaration completion-value (5) + typed-array symbol-key writes#5762
Merged
Conversation
…d-array symbol-key writes
Two coherent test262 clusters from the 2026-06-27 sweep.
## Cluster 2 — declaration completion-value (5 cases, fully fixed)
The completion value of a statement list whose last evaluated statement is a
`function`/`async function`/`generator`/`var` *declaration* must be empty, so it
falls through to the prior statement (`eval("1; function f(){}")` is `1`;
`eval("function f(){}")` is `undefined`; `eval("9; var x = 10")` is `9`).
In global-script mode the eval-fold hoist published these declarations to the
global environment with statements the completion tracker mis-counted:
- A top-level `function` is published via `Object.defineProperty(globalThis, …)`,
which *returns* `globalThis`. The tracker rewrote that to `__perry_cv = define…`,
so a declaration-only body yielded the global object. Fix: `void`-wrap the
defineProperty calls (the publish block is prepended, so an empty completion is
enough).
- A top-level `var x = init` was rewritten in place to `void (x = init)`, still an
expression statement, so the tracker wrote `__perry_cv = undefined` and
*clobbered* a preceding value (`9; var x = 10` wrongly became `undefined`). Fix:
emit a completion-inert lexical declaration `{ let __perry_eval_void = (x = init); }`
— a declaration's completion is empty, so the prior value shows through.
Fixes test262 `language/statements/{function,async-function,generators,variable}/cptn-*`
and `language/statementList/eval-fn-block` (all pass).
## Cluster 1 — typed-array symbol-key writes
A Symbol write on a *statically-typed* multi-byte typed array
(`const t = new Float64Array(2); t[sym] = v`) lowered through the width-tracked
native store, which `fptosi`-coerced the NaN-boxed Symbol to index 0 and
*clobbered element 0* instead of storing a symbol property. Fix: route
non-numeric keys on the width-tracked store path to the symbol-aware runtime
dispatcher (mirroring the symmetric IndexGet guard), and make
`js_typed_array_index_{get,set}_dynamic` triage Symbol keys into the symbol side
table.
Note: the two `OwnPropertyKeys/integer-indexes-and-string-and-symbol-keys` cases
remain — they exercise a multi-byte view over a buffer from a discarded temporary
(`new TA(new TA([…]).buffer)`) whose receiver is mis-inferred onto a typed-feedback
array path; that is a separate type-inference issue tracked for follow-up.
Tests: extends issue_5579 completion test with the declaration cases; adds
issue_5735_typed_array_symbol_keys for the symbol-write fix. perry-hir,
perry-codegen, perry-runtime suites green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughTyped-array Symbol-key access now uses the symbol side table in runtime get/set paths and in codegen for non-numeric IndexSet lowering. Global eval hoisting now emits completion-inert publish statements, wraps defineProperty calls in void, and adds regression coverage for declaration-ending eval bodies. ChangesTypedArray Symbol Key Handling
Global Eval Hoist Completion Semantics
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/perry-hir/src/lower/global_eval_hoist.rs`:
- Around line 95-109: The synthetic inert binding in synth_inert_assign_stmt
currently uses a fixed name that user code can reference, causing name
collisions and possible TDZ issues during initializer resolution. Update this
helper to generate a collision-free hidden identifier, preferably by reusing
fresh_hidden() or an equivalent temp-name generator, and wire that fresh name
into the parsed inert block before attaching the initializer from
synth_assign_stmt.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 65ad7b41-38df-4d7b-ae73-c85e69a5503f
📒 Files selected for processing (5)
crates/perry-codegen/src/expr/index_set.rscrates/perry-hir/src/lower/global_eval_hoist.rscrates/perry-runtime/src/typedarray_props.rscrates/perry/tests/issue_5579_indirect_eval_global_completion.rscrates/perry/tests/issue_5735_typed_array_symbol_keys.rs
Addresses CodeRabbit review: the completion-inert `var x = init` publish wrapped
the assignment in `{ let __perry_eval_void = (x = init); }` with a *fixed* sink
name. The lexical sink is in TDZ while its own initializer (`x = init`)
evaluates, so eval source whose `init` referenced that exact name
(`eval("var x = __perry_eval_void")`) would throw a spurious ReferenceError and
diverge from the pre-rewrite resolution. Generate a fresh per-publish hidden name
via `fresh_hidden()` instead, matching the existing renamed-binding convention.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/perry-hir/src/lower/global_eval_hoist.rs`:
- Around line 760-764: The inert sink in `global_eval_hoist.rs` is only made
fresh with `fresh_hidden()`, but it can still collide with identifiers from the
eval source and shadow them during initialization. Update the sink selection
used before `synth_inert_assign_stmt` so it chooses a name that is
collision-free against all source identifiers, not just previously generated
publishes, and keep the hidden sink logic within the publish path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e2bb6ccf-bf4b-4c58-a4e2-fe5054880495
📒 Files selected for processing (1)
crates/perry-hir/src/lower/global_eval_hoist.rs
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
1 participant
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 the two net-new test262 clusters from issue #5735 (2026-06-27 sweep).
Cluster 2 — declaration completion-value (5 cases ✅ all fixed)
The completion value of a statement list whose last evaluated statement is a
function/async function/generator/vardeclaration must be empty, falling through to the prior statement:function f(){}undefined1; function f(){}1var x = 10;undefined9; var x = 10;9function fn(){}{}undefinedIn global-script mode the eval-fold hoist published these declarations to the global environment with statements the completion tracker mis-counted:
functionis published viaObject.defineProperty(globalThis, …), which returnsglobalThis. The tracker rewrote that into__perry_cv = Object.defineProperty(…), so a declaration-only body yielded the global object. →void-wrap thedefinePropertycalls (the publish block is prepended, so an empty completion suffices).var x = initwas rewritten in place tovoid (x = init)— still an expression statement, so the tracker wrote__perry_cv = undefinedand clobbered a preceding value (9; var x = 10wrongly becameundefined). → emit a completion-inert lexical declaration{ let __perry_eval_void = (x = init); }; a declaration's completion is empty, so the prior value shows through.Closes test262
language/statements/{function,async-function,generators,variable}/cptn-*andlanguage/statementList/eval-fn-block.Cluster 1 — typed-array symbol-key writes
A Symbol write on a statically-typed multi-byte typed array (
const t = new Float64Array(2); t[sym] = v) lowered through the width-tracked native store, whichfptosi-coerced the NaN-boxed Symbol to index0and clobbered element 0 instead of storing a symbol property — corrupting the array's data and dropping the symbol.IndexGetguard already inindex_get.rs).js_typed_array_index_{get,set}_dynamictriage Symbol keys into the symbol side table.Remaining (separate follow-up): the two
built-ins/TypedArrayConstructors/internals/OwnPropertyKeys/.../integer-indexes-and-string-and-symbol-keyscases still fail. They exercise a multi-byte view over a buffer from a discarded temporary (new TA(new TA([…]).buffer)), whose receiver is mis-inferred onto a typed-feedback array store path — a deeper type-inference issue, not fixable safely in this change.Testing
crates/perry/tests/issue_5579_indirect_eval_global_completion.rs— extended with the declaration / var-init completion cases.crates/perry/tests/issue_5735_typed_array_symbol_keys.rs— new; pins the symbol write lands in the side table, leaves element data intact, and survivesreverse()/Reflect.ownKeysordering.perry-hir,perry-codegen,perry-runtimesuites green; test262 subset for the affected subtrees re-measured (cluster 2 fully closed,reverse/HasProperty/not-enumerableremain 100%).🤖 Generated with Claude Code
Summary by CodeRabbit
Symbolreads/writes via the typed array’s symbol-key storage (avoids clobbering element indices).eval/global hoisting completion behavior so completion is properly empty or preserved for declaration-ending eval bodies and related global binding publishes.Symbol-keyed typed array properties.evalcompletion semantics in global scripts.