test(codegen): pin the GC strategy onto the define line, in BOTH root lowerings

[proggeramlug]

Follow-up to #7998 (both refs #7982). Small, and it is a story about tests rather than about code.

#7998 made LlFunction::define_header the single renderer of the define … { line, after the in-process native path's private copy silently lost gc "statepoint-example". Without that strategy RS4GC never runs on the module: it verifies, links and executes correctly on any program that does not collect, while having no precise roots at all.

The test that shipped with #7998 does not pin that property, and my first two attempts to fix it failed the same way the original bug did.

1. The to_ir == define_header agreement test cannot see a dropped strategy — with one shared renderer, both sides change identically. Sabotage passed.
2. A dedicated strategy test that branched on native_stack_roots_enabled() never ran its ON arm under cargo test: no module has called set_native_roots_for_target, so the predicate is false in the test process. Sabotage passed again.
3. Only pinning both lowerings with NativeRootsPin::{native,shadow} — and asserting stack_map_slot_count in each arm first, so neither can pass vacuously — goes red on the sabotage.

That is this change's own bug class occurring twice inside its own test, which is why it is written up rather than quietly fixed.

Native-roots must take the stack-map path and name the strategy; shadow-stack must not take it and must not name it. Both arms carry a liveness assertion.

The tests live in function.rs, which compiles without the llvm-inprocess feature, so they run in per-PR cargo-test rather than only in the feature job.

Sabotage-verified with the fix committed first: dropping the strategy from the one renderer fails this test with the message naming #7332's shape, and the other two stay green. Restored and rebuilt (not merely git checkout-ed) to re-confirm. 935 perry-codegen lib tests green under --features llvm-inprocess.

Summary by CodeRabbit

• Bug Fixes

  • Added regression coverage for garbage-collection strategy selection across native-stack and shadow-stack modes.
  • Improved validation that stack-map behavior and strategy assignments remain correct for functions that require them.

• Documentation

  • Documented the GC-strategy pinning follow-up, prior regression, test limitations, and verification results.

[coderabbitai]

📝 Walkthrough

Walkthrough

The change adds regression coverage for GC strategy emission under native-root and shadow-stack lowering. It verifies stack-map slot accounting and documents prior vacuous test approaches and the final test setup.

Changes

GC strategy pinning

Layer / File(s)	Summary
GC strategy regression test and test rationale crates/perry-codegen/src/function.rs, changelog.d/8000-gc-strategy-define-header-pin.md, gc-handoff/KNOBS-NOTES.md	The regression test selects native-root lowering modes, checks GC strategy presence, verifies stack-map slot counts, and checks strategy absence for shadow-stack lowering. The changelog and notes document the test setup and earlier ineffective approaches.

Estimated code review effort: 2 (Simple) | ~10 minutes

Mergeability Score: ⚪ Minimal · up to 4e3db

This localized test-focused change is merge-ready after normal checks; the only follow-up is to trim the changelog to describe shipped behavior rather than development history, with no actionable merge-blocking risk remaining.

Possibly related PRs

• PerryTS/perry#7314: Adds related GC-strategy pinning coverage in perry-codegen.
• PerryTS/perry#7370: Introduces the native/shadow lowering mechanism used by this regression test.
• PerryTS/perry#7509: Adds related NativeRootsPin lowering tests with explicit strategy selection.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the main change: testing GC strategy placement for both root lowerings.
Description check	✅ Passed	The description clearly explains the bug, test approach, coverage for both lowerings, sabotage verification, and test results.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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/7982-gc-strategy-pin

---

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

[coderabbitai]

Actionable comments posted: 1

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

Inline comments:
In `@changelog.d/8000-gc-strategy-define-header-pin.md`:
- Around line 9-30: The changelog fragment should contain one concise
release-note entry describing only the shipped contract: native-roots functions
reserving stack-map slots emit gc "statepoint-example", shadow-stack functions
do not, and regression tests cover both paths. Remove the failed-test history,
sabotage details, implementation attempts, and internal test-run information
from the fragment.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 50e4f153-b55d-43c7-9ba2-6fd88b6a4ce1

📥 Commits

Reviewing files that changed from the base of the PR and between 564c563 and 4e3db54.

📒 Files selected for processing (3)

• changelog.d/8000-gc-strategy-define-header-pin.md
• crates/perry-codegen/src/function.rs
• gc-handoff/KNOBS-NOTES.md

[coderabbitai]

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Keep the changelog fragment focused on shipped behavior.

The fragment mixes the final behavior with failed-test history, sabotage steps, and internal test-run details. Keep one concise entry that states the shipped contract: native-roots functions that reserve stack-map slots emit gc "statepoint-example"; shadow-stack functions do not; regression coverage covers both.

Based on learnings: “For changelog fragments under changelog.d/, describe the final shipped behavior as one coherent release-note entry. Do not include separate development-slice narratives.”

🤖 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 `@changelog.d/8000-gc-strategy-define-header-pin.md` around lines 9 - 30, The
changelog fragment should contain one concise release-note entry describing only
the shipped contract: native-roots functions reserving stack-map slots emit gc
"statepoint-example", shadow-stack functions do not, and regression tests cover
both paths. Remove the failed-test history, sabotage details, implementation
attempts, and internal test-run information from the fragment.

Source: Learnings