Skip to content

test(gc): accept enriched root-spill diagnostic - #8756

Closed
proggeramlug wants to merge 1 commit into
mainfrom
codex/release-spill-diagnostic
Closed

test(gc): accept enriched root-spill diagnostic#8756
proggeramlug wants to merge 1 commit into
mainfrom
codex/release-spill-diagnostic

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Summary

  • match the stable semantic portion of the root-spill diagnostic
  • tolerate the newer (incl. call-result temporaries) detail inserted between the root count and shadow-frame wording

Release blocker

The current main cargo suite reaches the expected root-spill behavior but fails because this assertion still expects the old contiguous wording.

Verification

  • cargo fmt --all -- --check
  • git diff --check
  • clean-runner evidence: CI run 32739246257 compiled and ran the fixture, emitted five current-form root-spill diagnostics, then failed only this assertion

Summary by CodeRabbit

  • Tests
    • Updated spill-detection test expectations to reflect revised compiler diagnostic wording.
    • Diagnostics now refer to roots being kept in a shadow frame instead of statepoints.

@coderabbitai

coderabbitai Bot commented Aug 24, 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: eaa6247b-ca4d-4ebe-8456-786fb7e3fc03

📥 Commits

Reviewing files that changed from the base of the PR and between ed14ce5 and cba1665.

📒 Files selected for processing (1)
  • crates/perry/tests/gc_root_spill_mixed_frames_8583.rs

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


📝 Walkthrough

Walkthrough

The test assertion in gc_root_spill_mixed_frames_8583.rs now matches the compiler diagnostic that reports roots kept in a shadow frame instead of statepoints.

Changes

GC spill diagnostic

Layer / File(s) Summary
Update diagnostic assertion
crates/perry/tests/gc_root_spill_mixed_frames_8583.rs
The test checks the revised compiler diagnostic wording for roots kept in a shadow frame.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to cba16

This localized test-only change updates the expected diagnostic wording to accept the newer root-spill detail; no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains the change and verification, but it omits the required Changes, Related issue, Test plan, and Checklist sections. Add the required template sections, use "n/a" for Related issue if applicable, and record the relevant test-plan and checklist items.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies a GC test update that accepts the enriched root-spill diagnostic.
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 codex/release-spill-diagnostic

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 24, 2026 15:45
@proggeramlug

Copy link
Copy Markdown
Contributor Author

Superseded by #8754: the one-line assertion fix is now commit 85f149e on the combined release-blocker candidate, so cargo/gap/GC/parity can validate one exact tree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant