Skip to content

fix(repsel): contain guarded for-of element facts - #7899

Merged
proggeramlug merged 2 commits into
mainfrom
fix/7777-for-of-element-shape
Aug 12, 2026
Merged

fix(repsel): contain guarded for-of element facts#7899
proggeramlug merged 2 commits into
mainfrom
fix/7777-for-of-element-shape

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Restore the Ptr<Shape> element facts lost when #7761 wrapped proven-array for…of loops in an iterator-patch guard, without allowing a patched iterator to export stale shape facts across the branch join.

Changes

  • Recognize the compiler-only top-level ArrayIterationPatched guard and retain facts in its mutually exclusive index arm.
  • Require the guard to be the last use of the tracked array and every producer/reader object; nested guards and post-join uses remain conservatively denied.
  • Add positive coverage for the real guarded lowering plus negative coverage for post-join mutation, lazy-arm member reuse, and loop backedges.

Related issue

Fixes #7777

Test plan

  • cargo test -p perry-codegen --lib ptr_shape_elements::tests — 35 passed
  • cargo test -p perry-codegen — full crate and doc tests passed
  • Full 29-workload repsel census — Census OK; fixture recovered from 0/0 to exactly 3 selected / 3 consumed
  • Exact branch compiler/runtime/stdlib on test_gap_repsel_ptr_shape_elements.ts — byte-exact with Node, both exit 0
  • Positive and post-join-boundary sabotage runs both failed with exit 101
  • cargo fmt --all -- --check
  • git diff --check
  • bash scripts/check_file_size.sh
  • User-facing regression coverage added in the affected crate
  • Relevant full crate suite (cargo test -p perry-codegen)

Screenshots / output

Before:

fixture_ptr_shape_elements  ptr-shape 0 (floor 3)  ptr-shape-consumed 0 (floor 3)

After:

fixture_ptr_shape_elements  ptr-shape 3  ptr-shape-consumed 3
Census OK.

Checklist

  • I have NOT bumped the workspace version or edited CLAUDE.md / CHANGELOG.md
  • My commits follow the repository commit convention
  • I have read CONTRIBUTING.md and agree to the Code of Conduct

Summary by CodeRabbit

  • Bug Fixes

    • Fixed guarded for…of loops so array element information is preserved when the indexed path is the final use.
    • Prevented incorrect information from carrying across lazy iterator escapes, branch joins, reused values, and nested loops.
    • Improved handling of array aliases and nested closures.
  • Tests

    • Added coverage for successful terminal iteration, branch boundaries, invalidated values, and nested guarded loops.
    • Added end-to-end compatibility coverage for Node.js behavior.

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The change updates array element-shape analysis for compiler-generated guarded for…of loops. Terminal indexed paths preserve valid facts, while lazy iterator escapes, later uses, alias reuse, and nested guards block propagation. Tests cover these cases.

Changes

Guarded for-of element facts

Layer / File(s) Summary
Guarded iteration analysis contract
crates/perry-codegen/src/collectors/ptr_shape_elements.rs
Documents the ArrayIterationPatched escape rules, terminal-use requirement, and nested-guard restrictions.
Guarded ArrayWalk traversal
crates/perry-codegen/src/collectors/ptr_shape_elements.rs
Traverses guarded branches, tracks aliases, checks later references, and rejects unsafe element-shape fact propagation.
Fixtures and regression coverage
crates/perry-codegen/src/collectors/ptr_shape_elements_tests.rs, changelog.d/7899-guarded-for-of-element-facts.md
Adds guarded for…of fixtures and tests for fact retention, branch joins, group-member reuse, and nested loops. Documents the fix and coverage.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ArrayWalk
  participant ArrayIterationPatched
  participant LazyGetIterator
  participant IndexedFastArm
  participant ElementShapeFacts
  ArrayWalk->>ArrayIterationPatched: recognize guarded for-of structure
  ArrayIterationPatched->>LazyGetIterator: inspect iterator escape branch
  ArrayIterationPatched->>IndexedFastArm: inspect indexed branch and aliases
  ArrayWalk->>ElementShapeFacts: retain terminal indexed facts
  LazyGetIterator->>ElementShapeFacts: block facts when later uses or group reuse exist
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the guarded for-of element-fact fix.
Description check ✅ Passed The description includes the required summary, changes, issue, test plan, output, and checklist sections.
Linked Issues check ✅ Passed The changes restore guarded for-of element facts and preserve conservative behavior required by issue #7777.
Out of Scope Changes check ✅ Passed The changelog, implementation, and regression tests directly support the linked issue objectives.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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/7777-for-of-element-shape

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
proggeramlug marked this pull request as ready for review August 12, 2026 03:05

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

🧹 Nitpick comments (1)
crates/perry-codegen/src/collectors/ptr_shape_elements_tests.rs (1)

960-986: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a case that covers the group-member alias fixpoint.

The fixpoint at lines 609-619 of crates/perry-codegen/src/collectors/ptr_shape_elements.rs extends group_members across alias_edges. No test here exercises it: every group member is referenced by its own id. Delete that loop and this file still passes. Add a case that binds const row_alias = row; before the guard and then references row_alias after the join, and assert the facts are empty.

🧪 Proposed additional test
/// `ptr_shape` promotes an immutable alias with its root, so an alias of a
/// group member reused after the join observes the same reshaped object.
///
/// Sabotage: delete the `alias_edges` fixpoint that extends `group_members`
/// and the producer keeps its fact across the iterator escape.
#[test]
fn guarded_for_of_boundary_follows_group_member_aliases() {
    let c = class_c();
    let cs = [c];
    let classes = classes_of(&cs);
    let stmts = vec![
        let_arr(1, "rows"),
        let_c(2, "row"),
        push(1, Expr::LocalGet(2)),
        Stmt::Let {
            id: 3,
            name: "row_alias".to_string(),
            ty: Type::Named("C".to_string()),
            mutable: false,
            init: Some(Expr::LocalGet(2)),
        },
        guarded_for_of(
            1,
            20,
            30,
            31,
            Vec::new(),
            vec![let_elem(32, "guarded", 30, 31), read_x(32)],
        ),
        read_x(3),
    ];
    assert!(elements(&stmts, &classes).is_empty());
    assert!(promote(&stmts, &classes).is_empty());
}
🤖 Prompt for 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.

In `@crates/perry-codegen/src/collectors/ptr_shape_elements_tests.rs` around lines
960 - 986, Add a regression test in ptr_shape_elements_tests.rs covering the
alias_edges group_members fixpoint: bind an immutable row_alias to the group
member row before guarded_for_of, reference row_alias after the join, and assert
both elements(...) and promote(...) return empty. Follow the existing
guarded_for_of test structure and use the visible symbols let_arr, let_c,
guarded_for_of, read_x, and Stmt::Let.
🤖 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.

Nitpick comments:
In `@crates/perry-codegen/src/collectors/ptr_shape_elements_tests.rs`:
- Around line 960-986: Add a regression test in ptr_shape_elements_tests.rs
covering the alias_edges group_members fixpoint: bind an immutable row_alias to
the group member row before guarded_for_of, reference row_alias after the join,
and assert both elements(...) and promote(...) return empty. Follow the existing
guarded_for_of test structure and use the visible symbols let_arr, let_c,
guarded_for_of, read_x, and Stmt::Let.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 0d633c9a-9460-4e9c-b65e-56173aa4b817

📥 Commits

Reviewing files that changed from the base of the PR and between ae34f5d and 1d5d696.

📒 Files selected for processing (3)
  • changelog.d/7899-guarded-for-of-element-facts.md
  • crates/perry-codegen/src/collectors/ptr_shape_elements.rs
  • crates/perry-codegen/src/collectors/ptr_shape_elements_tests.rs

@proggeramlug
proggeramlug merged commit 6d9f12e into main Aug 12, 2026
14 of 19 checks passed
@proggeramlug
proggeramlug deleted the fix/7777-for-of-element-shape branch August 12, 2026 03:22
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.

repsel-census red on main: for (const r of rows) no longer licenses element-shape facts (fixture_ptr_shape_elements 0/3)

1 participant