fix(runtime): a live-index read inside forEach defers the squeeze instead of shifting the walk

[proggeramlug]

Summary

#9504 made the array-like map[i] / set[i] read a live-index accessor: it squeezes tombstones so raw index == live index and never hands out a hole. forEach walks the raw entries with a counter that the delete-path squeeze defers around (the walk registers itself) — but the accessor's own squeeze had no such guard. A callback that deletes already-visited entries and then reads map[j] compacted the buffer under the walk's counter, shifting the survivors below it:

const s = new Set<number>(); for (let i = 0; i < 20; i++) s.add(i);
const seen: number[] = [];
s.forEach((v) => { seen.push(v); if (v === 5) { s.delete(1); s.delete(2); void (s as any)[0]; } });
// main: visits 18 of 20 — 6 and 7 are never visited (Map: same)
// without the read: 20 of 20

Found by the automated review on #9513 and reproduced on merged main. The for…of fast path is unaffected — its cursor rebases through the compaction log (#9513), so a squeeze under it was already exact.

Fix

While a forEach walk is active, the live-index accessor defers the squeeze exactly as the delete path does and resolves the live index by stepping over the tombstones (O(idx), on a path that is rare by construction: an array-like read on a collection during its own walk). The outermost walk's completion performs the deferred squeeze as before. Outside a walk the accessor squeezes as #9504 specified, and #9504's a_tombstoned_collection_never_hands_a_hole_to_an_indexed_read stays green.

Tests / validation

• a_live_index_read_inside_foreach_defers_the_squeeze_and_skips_nothing for Map and Set: every entry visited exactly once, live values read mid-walk are correct, the layout is left alone during the walk and squeezed on completion, and the accessor still squeezes outside a walk.
• test-files/test_gap_foreach_live_index_read_no_skip.ts: single and nested walks, node-differential (the reproducer goes 18/20 → 20/20).
• RUST_TEST_THREADS=1 cargo test --release -p perry-runtime --lib: 3018 passed, 0 failed. fix(runtime): the hole-leak family — Set/Map raw reads, pop, typeof/String, param guards, console.table; util.format %s/%o policy (#9462, #9463) #9504's and fix(runtime,hir,codegen): Map/Set mutation during for…of is O(1) per step and never skips an entry #9513's fixtures still match node. cargo fmt --all --check clean; no clippy warning in the touched files.

Summary by CodeRabbit

• Bug Fixes

  • Fixed Map and Set forEach iteration so deleting previously visited entries no longer causes later entries to be skipped.
  • Corrected indexed reads performed during active forEach callbacks, including nested walks.
  • Collection compaction is now deferred until iteration completes, preserving accurate traversal results.

• Tests

  • Added regression coverage for deletion, indexed reads, visit order, final sizes, and nested iterations.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: f0cfc943-bf16-45cb-bf2a-6da11e17cdf3

📥 Commits

Reviewing files that changed from the base of the PR and between 7bb834e and 06e443d.

📒 Files selected for processing (6)

• changelog.d/9561-foreach-live-index-read.md
• crates/perry-runtime/src/map.rs
• crates/perry-runtime/src/map_tombstone_tests.rs
• crates/perry-runtime/src/set.rs
• crates/perry-runtime/src/set_tombstone_tests.rs
• test-files/test_gap_foreach_live_index_read_no_skip.ts

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

---

📝 Walkthrough

Walkthrough

Map and Set live-index accessors now defer tombstone compaction during active forEach walks. They translate live indices over tombstones and compact after the outermost walk. Runtime and integration tests cover deletion, nested walks, visit order, and post-walk compaction.

Changes

forEach live-index traversal

Layer / File(s)	Summary
Defer compaction and resolve live indices crates/perry-runtime/src/map.rs, crates/perry-runtime/src/set.rs	Map and Set accessors preserve tombstones during active forEach walks and translate live indices to raw positions. Outside a walk, accessors retain immediate compaction.
Validate Map and Set traversal behavior crates/perry-runtime/src/map_tombstone_tests.rs, crates/perry-runtime/src/set_tombstone_tests.rs	Runtime tests verify correct indexed reads, complete visitation, deferred compaction, and compaction after the walk.
Validate runtime and nested traversal cases test-files/test_gap_foreach_live_index_read_no_skip.ts, changelog.d/9561-foreach-live-index-read.md	Integration tests cover Map and Set deletion during forEach, nested Map walks, visit order, and final sizes. The changelog records the fix and regression coverage.

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

Merge Risk: ⚪ Minimal · up to 06e44

The change defers live-index compaction during active forEach walks and resolves indexes without skipping entries; no actionable merge-blocking risk remains after the stated validation.

Sequence Diagram(s)

sequenceDiagram
  participant MapSetForEach
  participant ForEachCallback
  participant LiveIndexAccessor
  MapSetForEach->>ForEachCallback: invoke callback
  ForEachCallback->>LiveIndexAccessor: delete earlier entries and read live index
  LiveIndexAccessor->>LiveIndexAccessor: step over tombstones
  LiveIndexAccessor-->>ForEachCallback: return requested live entry
  MapSetForEach->>MapSetForEach: compact after outermost walk

Loading

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 78.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 5 files. (1 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Title check	✅ Passed	The title clearly identifies the runtime fix: live-index reads inside forEach defer tombstone squeezing. It is specific and related to the main change.
Description check	✅ Passed	The description is detailed and on-topic. It explains the bug, fix, scope, regression coverage, and validation results. It does not use every template heading, such as Related issue, Screenshots / out…

Full details: Docstring Coverage

Explanation

Docstring coverage is 78.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 5 files. (1 skipped: 1 unsupported.)

Full details: Description check

Explanation

The description is detailed and on-topic. It explains the bug, fix, scope, regression coverage, and validation results. It does not use every template heading, such as Related issue, Screenshots / output, and Checklist, but the missing sections are non-critical for this change.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.}

[proggeramlug]

Landed via merge train #9563 (rebase-merge, authorship preserved).