Repository navigation
jit(fbw): fix broken main — drop #763's residual last_caught_exception_value propagation (removed in #756) - #765
Conversation
build_multi_frame_miframe copies last_caught_exception_value between the reconstructed InlineFrame/MIFrame chain, but #756 removed that field from MIFrame, BlackholeInterpreter, InlineFrame, and WalkSession — traceback recording is keyed on the raising instruction, not object identity. #763 was based before #756 and merged after it, so the references no longer resolve and pyre-jit-trace fails to compile (E0609 at residual_call.rs:240/241/273). Remove the vestigial per-frame propagation. The multi-frame build path is PYRE_FBW_MULTIFRAME gated (default off); no behavior change. check.py: dynasm 303/303, cranelift 303/303, wasm 300/300. Assisted-by: Claude
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughMulti-frame blackhole construction in the residual-call path no longer copies or overrides ChangesResidual-call exception state
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
🤖 Codex parity reviewStatic analysis of this diff vs the local RPython/PyPy sources (commit 5fb7a11). Files in the reviewed diff1. Regressions to PyPy parity introduced by this patchNone. 2. Other mismatches introduced by this patchNone. 3. Pre-existing mismatches (already present before this patch)None. 4. Structural adaptationsNone. |
Problem
origin/maindoes not compile.pyre-jit-tracefails with 4×E0609:Because PR CI builds
merge(PR, main), this reddens every open PR, not just the one that introduced it.Root cause
Stale-branch merge:
dc3559c94aa, "instruction-keyed, per-frame, and loop-close exception traceback recording") deliberately removedlast_caught_exception_valuefromMIFrame,BlackholeInterpreter,InlineFrame, andWalkSession. Its commit body: "no traceback decision keys on object identity." Traceback recording is now keyed on the raising instruction.80d590b0f6d, "multi-frame blackhole-resume build path") was developed on a pre-jit: instruction-keyed, per-frame, and loop-close exception traceback recording #756 base and still reads/writes that field inbuild_multi_frame_miframe. It merged 4 commits after jit: instruction-keyed, per-frame, and loop-close exception traceback recording #756 (#756 → #753 → #748 → #735 → #763), so the field references no longer resolve.Fix
Drop #763's vestigial per-frame
last_caught_exception_valuepropagation to match #756's instruction-keyed model — not restore the field (that would undo #756's design). The removed lines only copied the field between reconstructed frames; the multi-frame build path isPYRE_FBW_MULTIFRAME-gated (default off), so there is no behavior change.Verification
python3 ./pyre/check.pyon the fix:— commented by Claude
Summary by CodeRabbit