Skip to content

fix(runtime): refresh rooted arrays during iteration - #8670

Closed
proggeramlug wants to merge 1 commit into
mainfrom
fix/5898-array-remaining
Closed

fix(runtime): refresh rooted arrays during iteration#8670
proggeramlug wants to merge 1 commit into
mainfrom
fix/5898-array-remaining

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes the Array.prototype.find and findIndex mutation-during-iteration failures from the #5898 Test262 worklist. Callback-driven array relocation now refreshes the rooted receiver, and reads beyond a callback-shrunk logical length return absent/undefined instead of stale tail storage.

Changes

  • Resolve array-growth and moving-GC forwarding stubs before each callback-loop access, then self-heal the runtime root.
  • Check the current logical length before reading inline array elements.
  • Add a focused runtime regression test covering forwarding followed by splice shrinkage.

Related issue

Refs #5898 — completes the find/findIndex array-altered-during-loop subcluster.

Test plan

  • cargo build --release -p perry-runtime-static -p perry-stdlib-static
  • cargo test --release -p perry-runtime forwarded_array_observes_shrunk_length_during_callback_iteration -- --test-threads=1
  • Test262 built-ins/Array/prototype/find + findIndex: 36 pass, 0 failures (pinned SHA 4249661388e5d3f92a85186213da140a6481490f)
  • Full Test262 built-ins/Array slice: 2498 pass, 0 diff, 29 existing runtime failures, 0 compile failures; selected failures absent
  • cargo fmt -p perry-runtime -- --check
  • python scripts/check_test_registration.py
  • Full workspace test suite (not run locally)

Screenshots / output

Before: 34/36 pass in the selected Test262 directories; both array-altered-during-loop cases failed with stale Bike instead of undefined.

After: 36/36 pass.

Checklist

  • I have NOT bumped the workspace version or edited CLAUDE.md / CHANGELOG.md (maintainer handles these at merge)
  • My commit follows the loose feat/fix/docs/chore prefix convention used in the log
  • I've read CONTRIBUTING.md and agree to the Code of Conduct

Summary by CodeRabbit

  • Bug Fixes
    • Improved array iteration reliability when arrays move during memory management.
    • Array accesses now respect the current array length, preventing reads beyond shortened arrays.
    • Correctly handles forwarded arrays, missing elements, and out-of-range lookups.

@coderabbitai

coderabbitai Bot commented Aug 23, 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: df7e11dc-0de1-4659-b67d-f71647abe41f

📥 Commits

Reviewing files that changed from the base of the PR and between f4a7559 and 5c7d3f8.

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

Included review availability: Your plan provides up to 8 included reviews per hour; 4 remain after this review.


📝 Walkthrough

Walkthrough

RootedIterArray now resolves forwarded arrays on each access, refreshes roots, derives storage from the live header, and prevents reads beyond the current length. A regression test covers growth, shrinkage, forwarding, valid elements, holes, and out-of-range access.

Changes

Rooted array access

Layer / File(s) Summary
Live header resolution and access validation
crates/perry-runtime/src/array/iter_methods.rs
RootedIterArray follows forwarding pointers, refreshes its root, uses the current header, and checks the current length before access. The regression test covers array growth, shrinkage, forwarding, holes, and out-of-range reads.

Estimated code review effort: 3 (Moderate) | ~15–30 minutes

Merge Risk: ⚪ Minimal · up to 5c7d3

This localized runtime change refreshes array roots during find/findIndex iteration and prevents stale values after callback-driven shrinkage; no actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: thehypnoo

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes refreshing rooted arrays during runtime iteration, which is the primary change.
Description check ✅ Passed The description includes the required summary, changes, issue, test plan, output, and checklist, and it records the unrun full workspace suite.
Docstring Coverage ✅ Passed Docstring check was indeterminate for this PR — some files could not be analyzed in time. Not blocking.
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/5898-array-remaining

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.

proggeramlug added a commit that referenced this pull request Aug 24, 2026
…re safepoints (#8680)

Lands four reviewed PRs as one squash.

- #8670: refresh rooted arrays during iteration.
- #8673: fix a `--report-size` false positive from std-internal crate names.
- #8668: specialize dense Array-subclass indexing.
- #8678 (#8583): count property/index STORES as GC safepoint sites in the
  spill estimate. `PropertySet`/`PropertyUpdate`/`IndexSet` lower to
  collecting runtime calls that rewrite-statepoints-for-gc gives a
  statepoint, but none were counted, so a closed-shape object literal's
  constructor -- one long run of `this.field = v` -- estimated ~0, was
  never spilled to the shadow frame, and RS4GC grew one
  `__AnonShape_*_constructor` from 34,009 to 2,280,128 instructions,
  overrunning the #8586 per-function budget and refusing the whole
  module. Reads are deliberately not counted: they frequently inline to
  a shape-cached load with no call, so counting them would over-spill
  read-heavy hot loops.

Version bump stripped per maintainer policy; the Cargo.lock diff was
verified version-only before stripping.

Co-authored-by: Ralph Küpper <ralph@skelpo.com>
@proggeramlug

Copy link
Copy Markdown
Contributor Author

Landed on main via #8680 (squash 81ddd46b2), together with the rest of this batch.

Validated on the merged 4-PR result rather than per-branch: 9 ratchet gates + cargo fmt --check pass, perry-codegen --lib 1194/0, perry-runtime --lib 2651/0 at RUST_TEST_THREADS=1. The squashed tree was verified byte-identical to the merged tree before push.

Version bump stripped per maintainer policy (the Cargo.lock diff was version-only); your changelog.d/ fragment carried through unchanged. Thanks!

@proggeramlug
proggeramlug deleted the fix/5898-array-remaining branch August 24, 2026 03:30
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