land #8761: honor exotic indices in Array shift

[proggeramlug]

Lands #8761 (fix(runtime): honor exotic indices in Array shift), original author's commit preserved.

I held this PR on regressions it did not cause

I blocked #8761 on four gap-suite parity regressions and asked the author not to re-baseline. That was the right instinct about baselines, but the wrong attribution — the PR caused none of them.

#8761's CI ran 2026-08-24 17:03–17:25 UTC against main tip 20a388974, and actually reported eight regressions across five shards, not the four I quoted. Two unrelated sibling PRs on the same main window — #8771 and #8776 — report the identical failure set. The cluster is class-instance prototype-chain breakage that main carried at that moment, and it was fixed within hours by #8766 (17:41), #8778 (18:26) and #8780/#8781 (19:27) — #8781's own body names String(new C()), i.e. test_gap_string_coercion_tostring, one of the eight.

So the "shared helper" I suspected was main, not this PR. CLAUDE.md's own guidance is to attribute a red via a sibling PR on the same base; I did not do that, and the PR sat blocked as a result.

Evidence

Full 580-test gap sweep on both arms — clean main @ e043aa294 vs main + PR — identical toolchain, identical -p set, same target dir, sequential, byte-compared against Node 26.5.1 (matching .node-version):

	pass	compile_fail	parity_fail
clean main	546	12	22
main + PR	546	12	22

Divergences across all 580 tests: zero. All four originally-named tests pass on both arms, as do the four additional ones from the other shards. Arm A was verified genuinely PR-free (nm finds 0 shift_array_spec_path symbols in its libperry_runtime.a); arm B's .a mtimes were confirmed to move after each edit, so neither arm was measuring a stale archive.

The actual blocker

A lint failure: "This PR changes crates/ but adds no changelog.d/ fragment." This branch adds changelog.d/8761-array-shift-exotic-indices.md. No baseline was touched, no test deleted, no #[ignore], no version bump.

Validation

• all 30 lint-job gates pass
• perry-runtime 2690, perry-codegen 1252, perry-stdlib 120 — all 0 failed

Known coverage gap

15 ext-routed gap tests (http/http2/net/ws/zlib/events) were not compared against Node in either arm — they need perry-ext-* wrappers built in the same cargo invocation. This matters for this PR specifically, because js_array_shift_f64 is called from node_stream* and events_on.rs. Their status is identical in both arms (compile_fail, same missing-wrapper reason), so nothing is attributable to the PR, but CI is the authority for those paths.

Summary by CodeRabbit

• Bug Fixes

  • Fixed Array.prototype.shift behavior for sparse arrays, inherited indexed properties, accessors, custom descriptors, and holes.
  • Ensured getter side effects and property operations occur in the specified order.
  • Correctly converts internal array holes to undefined when shifting.

• Tests

  • Added coverage for exotic index scenarios, inherited properties, frozen arrays, and non-writable lengths.

• Documentation

  • Documented the corrected Array.prototype.shift behavior.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 6df93f17-c107-43de-9bc7-e888f46cf87c

📥 Commits

Reviewing files that changed from the base of the PR and between cd5ab38 and a25c511.

📒 Files selected for processing (3)

• changelog.d/8761-array-shift-exotic-indices.md
• crates/perry-runtime/src/array/push_pop.rs
• crates/perry/tests/issue_5898_array_shift_exotic.rs

---

📝 Walkthrough

Walkthrough

Array.prototype.shift now uses a specification-compliant path for arrays with observable indexed operations. The change handles inherited properties, holes, accessors, deletion failures, and length mutations. Regression coverage verifies these cases.

Changes

Array shift behavior

Layer / File(s)	Summary
Runtime shift paths crates/perry-runtime/src/array/push_pop.rs	js_array_shift_f64 selects a specification path for exotic arrays. The path performs live HasProperty, Get, Set, and Delete operations, preserves values across accessors, and validates the final length update. Dense arrays retain the optimized path.
Regression coverage and change record crates/perry/tests/issue_5898_array_shift_exotic.rs, changelog.d/8761-array-shift-exotic-indices.md	Tests cover inherited indices, holes, accessor side effects, frozen arrays, and non-writable lengths. The changelog documents the updated behavior.

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

Sequence Diagram(s)

sequenceDiagram
  participant ShiftCall
  participant js_array_shift_f64
  participant shift_array_spec_path
  participant IndexedProperties
  ShiftCall->>js_array_shift_f64: invoke shift
  js_array_shift_f64->>shift_array_spec_path: select exotic path
  shift_array_spec_path->>IndexedProperties: HasProperty, Get, Set, Delete
  IndexedProperties-->>shift_array_spec_path: property results and side effects
  shift_array_spec_path-->>ShiftCall: shifted value or TypeError

Loading

✨ 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 land-8761-array-shift

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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