Skip to content

fix(array): exotic index propagation for Object.prototype + join/slice#5764

Merged
proggeramlug merged 2 commits into
mainfrom
fix/array-exotic-index-5589
Jun 28, 2026
Merged

fix(array): exotic index propagation for Object.prototype + join/slice#5764
proggeramlug merged 2 commits into
mainfrom
fix/array-exotic-index-5589

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Three focused runtime fixes that extend Perry's "exotic array" slow-path to cover Object.prototype numeric indices and two previously unguarded methods (join, slice), reducing test262 built-ins/Array failures tracked in #5589.

Changes

  • crates/perry-runtime/src/array/indexing.rsarray_iteration_is_exotic: add OBJECT_PROTO_HAS_INDEX check alongside the existing ARRAY_PROTO_HAS_INDEX one. Every iteration method (reduce, reduceRight, forEach, map, filter, find, findIndex, every, some, indexOf, lastIndexOf, flat, flatMap, sort) already delegates to this predicate; without the extra check, a numeric property on Object.prototype was invisible to all of them.

  • crates/perry-runtime/src/array/iter_methods.rsjs_array_join: add the exotic fast/slow-path split that every other iteration method already uses. Previously join (and thus Array.prototype.toString) always read directly from the dense element store, so inherited numeric properties on Array.prototype or Object.prototype were silently skipped.

  • crates/perry-runtime/src/array/splice_slice.rsjs_array_slice: same exotic-path split for both the plain-array and species-container branches. Per ECMA-262 §23.1.3.25 step 8b, slice must use [[HasProperty]]/[[Get]] per element, so an inherited index should appear in the result.

Related issue

Refs #5589

Test plan

Before/after comparison against the test262 built-ins/Array subset is not directly runnable (network policy blocks the tc39/test262 clone in this environment), but each fix was verified with a hand-written Perry script covering the three cases:

// Fix 1: Object.prototype numeric index visible to join/reduce
(Object.prototype as any)[1] = 999;
const a1: any[] = [0, , 2];
console.assert(a1.join(",") === "0,999,2");
console.assert(a1.reduce((acc: number, v: number) => acc + v, 0) === 1001);
delete (Object.prototype as any)[1];

// Fix 2: Array.prototype numeric index visible to join
(Array.prototype as any)[1] = 777;
const a2: any[] = [0, , 2];
console.assert(a2.join(",") === "0,777,2");
delete (Array.prototype as any)[1];

// Fix 3: slice picks up inherited index
(Array.prototype as any)[2] = "x";
const a3: any[] = [0, 1]; a3.length = 3;
console.assert(a3.slice(0, 3)[2] === "x");
delete (Array.prototype as any)[2];

Output: all tests passed (Perry exit 0).

  • cargo build --release clean (perry, perry-runtime, perry-stdlib — all warnings, zero errors)
  • cargo fmt --all -- --check passes
  • bash scripts/check_file_size.sh passes

Screenshots / output

$ cargo build --release -p perry -p perry-runtime -p perry-stdlib
   Finished `release` profile [optimized] target(s) in 4m 45s
$ cargo fmt --all -- --check
(no output — clean)
$ bash scripts/check_file_size.sh
OK: no Rust source files exceed 2000 lines.
$ ./target/release/perry test_array_exotic.ts -o test_array_exotic && ./test_array_exotic
all tests passed

Checklist

  • I have NOT bumped the workspace version or edited CLAUDE.md / CHANGELOG.md (maintainer handles these at merge)
  • My commits follow the loose feat: / fix: / docs: / chore: prefix convention used in the log

Generated by Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Fixed array iteration, join, and slice to correctly account for indexed properties inherited from Object.prototype, avoiding incorrect dense-element reads.
    • Corrected join behavior for special/exotic array forms by skipping missing indices and preserving correct element presence.
    • Improved slice so absent entries are kept as holes and missing indices aren’t turned into result properties, including for custom/spec species behavior.

#5589)

Three runtime fixes for test262 built-ins/Array failures:

1. `array_iteration_is_exotic` (indexing.rs): add `OBJECT_PROTO_HAS_INDEX`
   check alongside the existing `ARRAY_PROTO_HAS_INDEX` one. Without it, any
   method that delegates to the exotic slow-path (reduce, reduceRight, forEach,
   map, filter, find, findIndex, every, some, indexOf, lastIndexOf, flat,
   flatMap, sort) would not notice a numeric property added to Object.prototype,
   causing a missed inherited read and a wrong result.

2. `js_array_join` (iter_methods.rs): add the exotic fast/slow-path split that
   every other iteration method already uses. Previously join always read
   directly from the dense element store, so a numeric property on
   Array.prototype or Object.prototype was invisible to join/toString.

3. `js_array_slice` (splice_slice.rs): same fix — when the source array is
   exotic, use array_spec_has_index/array_spec_get per element so inherited
   indices appear in the slice result (ECMA-262 §23.1.3.25 step 8b).

All three cases are covered by a passing Perry verification test.
@coderabbitai

coderabbitai Bot commented Jun 28, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 0fe972aa-8c55-4372-8cde-78b062df7ae9

📥 Commits

Reviewing files that changed from the base of the PR and between 9113d3c and 2278aa2.

📒 Files selected for processing (1)
  • crates/perry-runtime/src/array/splice_slice.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/perry-runtime/src/array/splice_slice.rs

📝 Walkthrough

Walkthrough

array_iteration_is_exotic now treats OBJECT_PROTO_HAS_INDEX as exotic. js_array_join and js_array_slice use that flag to choose spec-style index reads for exotic arrays, while non-exotic paths keep raw slot access and hole handling.

Changes

Exotic Array Handling in join/slice

Layer / File(s) Summary
Exotic detection flag
crates/perry-runtime/src/array/indexing.rs
array_iteration_is_exotic now returns true when OBJECT_PROTO_HAS_INDEX is set, alongside the existing ARRAY_PROTO_HAS_INDEX check.
js_array_join exotic branching
crates/perry-runtime/src/array/iter_methods.rs
Computes exotic before the loop; exotic path uses array_spec_has_index/array_spec_get and skips absent indices, while non-exotic path reads raw slots and skips TAG_HOLE.
js_array_slice exotic branching
crates/perry-runtime/src/array/splice_slice.rs
Plain-result copying switches to spec reads for exotic sources and writes TAG_HOLE for absent indices; custom-species insertion skips absent exotic indices and ignores TAG_HOLE for non-exotic sources.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

Possibly related PRs

  • PerryTS/perry#5673: Touches the same exotic/spec index-read path used by array_spec_has_index and array_spec_get.

Poem

🐇 Hop, hop—through arrays I glide,
Exotic paths now read both sides.
If prototype indexes peek through the floor,
Spec reads step in and skip the holey store.
Join and slice in rabbit sync,
Tag holes vanish in a blink.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately captures the main array exotic-index fix for join/slice.
Description check ✅ Passed The description follows the template with Summary, Changes, Related issue, Test plan, Screenshots/output, and Checklist sections.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/array-exotic-index-5589

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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-runtime/src/array/splice_slice.rs`:
- Around line 287-295: The splice_slice slice path is incorrectly materializing
absent dense slots as TAG_HOLE into custom-species results. Update the logic
around `src_exotic`, `array_spec_has_index`, `array_spec_get`, and
`species_result_set` in `splice_slice` so that non-exotic sources still skip
missing indices before writing into the result. Ensure absent slots are treated
as holes and never passed to `species_result_set` for custom species, matching
slice behavior for exotic sources.
🪄 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: 24b06148-63b5-48cb-9c9f-9abd1cedd645

📥 Commits

Reviewing files that changed from the base of the PR and between 69eaa23 and 9113d3c.

📒 Files selected for processing (3)
  • crates/perry-runtime/src/array/indexing.rs
  • crates/perry-runtime/src/array/iter_methods.rs
  • crates/perry-runtime/src/array/splice_slice.rs

Comment thread crates/perry-runtime/src/array/splice_slice.rs
In js_array_slice's custom-species branch, the non-exotic else path was
forwarding raw TAG_HOLE values to species_result_set, creating a property
on the result for absent dense slots. The spec (§23.1.3.25 step 8b) only
calls CreateDataPropertyOrThrow when HasProperty(O, from) is true, so holes
must be skipped regardless of whether the source is exotic.
@proggeramlug proggeramlug merged commit 14d0b60 into main Jun 28, 2026
15 checks passed
@proggeramlug proggeramlug deleted the fix/array-exotic-index-5589 branch June 28, 2026 18:08
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.

2 participants