Skip to content

fix(runtime): honor exotic indices in Array reverse (rebase of #8400) - #8437

Merged
proggeramlug merged 1 commit into
mainfrom
fix/8400-array-reverse-exotic-rebased
Aug 19, 2026
Merged

fix(runtime): honor exotic indices in Array reverse (rebase of #8400)#8437
proggeramlug merged 1 commit into
mainfrom
fix/8400-array-reverse-exotic-rebased

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Rebase of #8400 onto current main. Original work by @proggeramlug; this exists only
because that branch conflicts with main and is on a fork I cannot push to.

Completes the remaining Array.prototype.reverse cluster from #5898: arrays with holes,
indexed descriptors, or indexed prototype properties now follow the specified live
HasProperty / Get / Set / Delete order, with the receiver and both observed values
rooted across accessors.

The conflict, and how it was resolved

crates/perry-runtime/src/array/mod.rs conflicts on a re-export list. Resolved as a
union, not by taking either side:

Taking the PR's side wholesale would have silently dropped the two #8414 symbols. All three
are present here.

Validation

  • perry-runtime --lib — 2599 passed
  • perry --bin perry — 1005 passed
  • issue_5898_array_reverse_exotic — 1 passed
  • scripts/run_lint_gates.sh — all 50 gates
  • 19/19 sweep corpus byte-exact against the Node oracle

Behaviour on the test262 shape matches Node exactly:

before: 1=onproto has1=true
after:  0=3 1=onproto 2=1 own1=false

Holes, dense arrays and accessor-backed indices also match.

Supersedes #8400.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed Array.prototype.reverse behavior for inherited, accessor-backed, sparse, deleted, and other unusual array indices.
    • Corrected array length truncation so non-configurable elements are preserved and reported correctly.
    • Prevented deleted sparse elements from reappearing after arrays grow.
    • Improved handling of getters and side effects during array reversal.
  • Tests

    • Added regression coverage for exotic, sparse, inherited, and dynamically modified arrays.

Rebase of #8400 onto current main. The re-export list in array/mod.rs conflicted
with #8414's additions; resolved as a union so array_object_flags_resolved and
canonicalize_array_numeric_store_value_from_flags are preserved alongside
array_named_property_delete_by_name.

Original work by @proggeramlug in #8400.
@proggeramlug
proggeramlug merged commit 526e0b5 into main Aug 19, 2026
15 of 17 checks passed
@proggeramlug
proggeramlug deleted the fix/8400-array-reverse-exotic-rebased branch August 19, 2026 21:54
@coderabbitai

coderabbitai Bot commented Aug 19, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c36eb00d-6896-40aa-a258-0276ad41f93a

📥 Commits

Reviewing files that changed from the base of the PR and between 9051d3f and 2cc38d4.

📒 Files selected for processing (6)
  • changelog.d/8400-array-reverse-exotic-indices.md
  • crates/perry-runtime/src/array/concat_reverse.rs
  • crates/perry-runtime/src/array/header.rs
  • crates/perry-runtime/src/array/mod.rs
  • crates/perry-runtime/src/array/push_pop.rs
  • crates/perry/tests/issue_5898_array_reverse_exotic.rs

📝 Walkthrough

Walkthrough

Array.prototype.reverse now uses specification-ordered property operations for exotic arrays and keeps the dense-array fast path. Array truncation and deletion now clear sparse and descriptor-backed indices correctly. Regression tests cover inherited indices, accessors, sparse arrays, and GC-related storage cleanup.

Changes

Array reverse and indexed mutation

Layer / File(s) Summary
Indexed deletion and truncation
crates/perry-runtime/src/array/header.rs, crates/perry-runtime/src/array/mod.rs, crates/perry-runtime/src/array/push_pop.rs
Named-property deletion is available by string name. Truncation now deletes indices in descending order and preserves length when deletion fails. Deletion clears both dense and sparse representations.
Spec-driven exotic reversal
crates/perry-runtime/src/array/concat_reverse.rs
js_array_reverse routes arrays with holes, inherited indices, or accessors through live presence checks, reads, writes, and deletes. Dense arrays retain direct slot swapping.
Regression coverage and changelog
crates/perry/tests/issue_5898_array_reverse_exotic.rs, changelog.d/8400-array-reverse-exotic-indices.md
The integration test covers inherited indices, getter side effects, sparse truncation, GC rekeying, and deletion cleanup. The changelog documents the changes.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Array.prototype.reverse
  participant reverse_array_spec_path
  participant Array indexed properties
  Array.prototype.reverse->>reverse_array_spec_path: reverse exotic array
  reverse_array_spec_path->>Array indexed properties: check presence and read values
  reverse_array_spec_path->>Array indexed properties: set or delete paired indices
Loading

Possibly related PRs

  • PerryTS/perry#8400: Covers the same exotic-index reverse and array mutation fixes.
  • PerryTS/perry#8366: Applies similar spec-compliant exotic-array mutation handling to pop.
  • PerryTS/perry#8397: Relates to GC rekeying of array side-table ownership covered by the regression test.

Suggested reviewers: jdalton, thehypnoo

✨ 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/8400-array-reverse-exotic-rebased

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.

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.

1 participant