test(codegen): make the stale codegen-test contracts able to fail again (#7505, #7504, #7503, #6988) - #7675
Conversation
📝 WalkthroughWalkthroughThis change adds lowering-independent IR analyzers, moves temporary-root coverage into the codegen crate, scopes Buffer and root-slot assertions to relevant functions and allocas, updates regression tests for both lowerings, and adds CI source-to-suite mapping validation. ChangesTemporary-root contracts
Shadow-slot accounting
Native Buffer proof analysis
CI source-suite mapping
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Suggested labels: 🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (6)
crates/perry-codegen/src/testing/temp_slots.rs (4)
391-413: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe explicit load branch is redundant.
Lines 391-399 already mark every
, ptr <reg>operand as touched, and a load line such as%d = load i64, ptr %scontains, ptr. The block at Lines 400-413 therefore re-inserts slots that are already intouched. Remove it, or add a comment that states which shape it covers that the generic scan misses.🤖 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/testing/temp_slots.rs` around lines 391 - 413, Remove the explicit load-prefix handling block from the touched-slot collection in the surrounding temp-slot analysis, since the preceding “, ptr ” scan already captures those operands. Preserve the existing generic scan and touched insertion behavior.
338-365: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
derives_from_slot_loadrepeats the single-operand walk fromderives_from.Lines 348-363 duplicate the walk in
derives_fromat Lines 205-221, with the same limitation: only the first%token in each defining line is followed. Extract one shared walk helper that yields predecessors, then let bothderives_fromandderives_from_slot_loadsupply the stop predicate.🤖 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/testing/temp_slots.rs` around lines 338 - 365, The register-walking logic is duplicated between derives_from and derives_from_slot_load and only follows the first predecessor token. Extract a shared helper that walks defining predecessors and accepts a caller-supplied stop predicate, then update both derives_from and derives_from_slot_load to use it while preserving their existing termination behavior.
204-222: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winThe def-use walk follows only one operand per instruction.
derives_frompicks the first%token in the defining line and drops the rest. For a two-register instruction such as%r = or i64 %tag, %value, only%tagis followed. If a rooted value reaches a consumer through the second operand of such an instruction,assert_rooted_acrossreports a missing re-read even though the re-read exists. The failure direction is a false alarm, not a silent pass, so it is not blocking. Consider a breadth-first walk over every%operand with a visited set.🤖 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/testing/temp_slots.rs` around lines 204 - 222, Update derives_from to traverse every register operand referenced by each defining instruction rather than only the first % token. Use a breadth-first or equivalent worklist walk with a visited set, preserving the depth limit and returning true when any reachable register matches ancestor; update callers only as needed to retain the existing rooted-value behavior.
447-452: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winThe alloca filter matches the def text exactly, so an
alignsuffix silently empties the result.
defsstores the whole text right of=. If codegen ever printsalloca i64, align 8, thismatches!fails for every slot.temp_root_slotsthen returns an empty vector, and everyassert_no_temp_rootingcall becomes vacuous — the same failure mode#7503removes. The positive tests would fail loudly, so the risk is bounded today, but the negatives lose their meaning without warning. Match on the type prefix instead.♻️ Proposed fix: match the alloca type by prefix
.filter(|(slot, _)| { - matches!( - defs.get(slot.as_str()), - Some(&"alloca i64") | Some(&"alloca ptr addrspace(1)") - ) + defs.get(slot.as_str()).is_some_and(|def| { + let ty = def + .strip_prefix("alloca ") + .map(|rest| rest.split(',').next().unwrap_or(rest).trim()); + matches!(ty, Some("i64") | Some("ptr addrspace(1)")) + }) })Run the following script to check the emitted alloca spelling:
#!/bin/bash # Description: Find how codegen prints entry allocas, to confirm no align suffix. set -euo pipefail rg -n -C3 --type=rust 'alloca ' crates/perry-codegen/src --glob '!**/testing/**' | head -80 rg -n -C3 --type=rust 'alloca_entry|entry_allocas_push_store' crates/perry-codegen/src | head -60🤖 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/testing/temp_slots.rs` around lines 447 - 452, Update the alloca matching inside temp_root_slots to recognize definitions whose text begins with the supported alloca type prefixes, rather than requiring exact equality. Preserve matching for i64 and ptr addrspace(1), including definitions with suffixes such as alignment metadata, so assert_no_temp_rooting remains effective.crates/perry-codegen/src/temp_root_coverage/mod.rs (2)
48-134: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
entry_optsandmodule_with_initnow exist in two places.
crates/perry-codegen/tests/temp_root_operand_temporaries.rsdefines its ownentry_opts()andmodule_with_init(...)and calls them at Lines 861 and 1147. This file adds a second copy of both. Every newCompileOptionsorModulefield must be added twice. Move the harness intocrate::testingand export it, then let the integration suite import it. The module doc at Lines 15-19 already names the harness cost as the reason the coverage did not move earlier, so one shared copy fits that reasoning.🤖 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/temp_root_coverage/mod.rs` around lines 48 - 134, Remove the duplicate entry_opts and module_with_init helpers from the coverage module and move their shared implementations into crate::testing with public visibility. Update temp_root_operand_temporaries.rs to import and use crate::testing::entry_opts and crate::testing::module_with_init, preserving their current behavior and signatures.
219-229: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winThe write-back check compares registers exactly, unlike every other assertion in this file.
Line 225 requires the slot store value to equal
pushverbatim.slot_trafficrecords the register that the store names.assert_rooted_acrossdeliberately tolerates one boxing step between the producer result and the stored register, and thederives_fromdoc atcrates/perry-codegen/src/testing/temp_slots.rsLines 198-203 states that a raw allocation result is NaN-boxed before it reaches a slot. If the write-back path ever boxes the push result, this assertion fails while the contract still holds. Useslot_holdingfor the write-back check so both clauses use the same matching rule.♻️ Proposed fix: match the write-back through the same derivation rule
assert!( - slot_traffic(&ir)[&slot] - .iter() - .any(|e| matches!(e, SlotEvent::Store { value, .. } if *value == push)), + slot_holding(&ir, &push).as_deref() == Some(slot.as_str()), "{lowering}: the reallocated array {push} must be written BACK into \ {slot}; rooting only the pre-push pointer protects the wrong \ allocation (`#6951`):\n{ir}" );
SlotEventandslot_trafficstay imported for the release-order check below.🤖 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/temp_root_coverage/mod.rs` around lines 219 - 229, Update the write-back assertion in assert_rooted_across to match the stored slot value using the same slot_holding derivation rule as the surrounding rooting checks, allowing the push result to pass through one boxing step. Preserve the existing slot_traffic and SlotEvent usage for the release-order assertion below.
🤖 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 `@crates/perry-codegen/src/temp_root_coverage/operands.rs`:
- Around line 98-110: Add the same non-vacuity reach guard used by
a_proven_index_typed_array_read_is_not_temp_rooted to
a_buffer_index_read_is_not_temp_rooted, verifying the fixture reaches
js_dynamic_string_or_number_add before calling assert_no_temp_rooting. Keep the
existing lowering coverage and negative assertion unchanged.
In `@crates/perry-codegen/src/testing/root_slots.rs`:
- Around line 76-90: Update classify to recognize SlotKind::TempRoot only when
the zero-initializing store for the alloca appears in the function’s entry
block, rather than anywhere in fn_ir. Preserve the panic for bound allocas
without a recognized entry-block initialization, and add a fixture covering an
alloca that is zeroed and bound only in a later block.
- Around line 213-225: Update enclosing_function so its start-marker search
recognizes define at byte index zero as well as occurrences preceded by a
newline, while preserving the existing function-boundary behavior. Add a focused
test covering a single-function IR string that begins with define and verifies
the enclosing function is returned.
In `@crates/perry-codegen/tests/native_proof_support/mod.rs`:
- Around line 429-526: Move the IR-reader self-tests using probe_body,
native_buffer_element_geps, and assert_no_native_buffer_element_access from
crates/perry-codegen/tests/native_proof_support/mod.rs:429-526 into an in-crate
test module executed by cargo-test, retaining integration copies only as
supplemental coverage. Add equivalent in-crate multibyte-read negative coverage
for both lowerings from
crates/perry-codegen/tests/native_proof_buffer_views.rs:676-693, and equivalent
in-crate proven-store and invalidated-store fallback coverage from
crates/perry-codegen/tests/native_proof_regressions/invalidation.rs:33-87.
In `@crates/perry-codegen/tests/scalar_replaced_slot_roots.rs`:
- Line 228: The required contracts lack unit-level codegen acceptance coverage.
Add equivalent tests in a crates/perry-codegen/src test module for
crates/perry-codegen/tests/scalar_replaced_slot_roots.rs:228-228, covering
value-slot attribution, barriers, and entry-hoisted scalar-slot behavior, and
for crates/perry-codegen/tests/shadow_slot_hygiene.rs:927-973, covering
reserved-slot bind-or-clear hygiene; retain both existing integration tests.
- Around line 687-695: Update the bind validation in the test around main_ir and
the selected value_slot so it searches, before the bind, for the exact
initialization pattern store double {undef}, ptr {value_slot}. Ensure the
assertion targets value_slot specifically rather than counting an undefined
store to any other alloca.
In `@crates/perry-codegen/tests/shadow_slot_hygiene.rs`:
- Around line 952-956: Update the touched-slot predicate in the BTreeSet
construction to require an argument delimiter after the index in
js_shadow_slot_set calls, preventing an index such as 10 from matching slot 1;
preserve the existing js_shadow_slot_bind matching behavior.
In `@crates/perry-codegen/tests/temp_root_operand_temporaries.rs`:
- Around line 241-272: Update all three assertions in
crates/perry-codegen/tests/temp_root_operand_temporaries.rs:241-272, 403-410,
and 1186-1192. Use slot_holding to resolve the specific accumulator or
constructor-argument slot, then inspect only that slot’s slot_traffic: require
the concat accumulator’s two Stores and Load, each constructor argument’s Load
after instance_alloc, and the Object.assign accumulator’s two or more Stores. Do
not search across all temp slots.
- Around line 379-395: Update the store lookup associated with arg0 in the
temp-slot ordering assertion to filter by the same derivation rule used by
slot_holding, rather than selecting the first Store event in the pooled slot.
Prefer exposing and reusing a perry_codegen::testing::temp_slots helper that
returns the store line for a value, including boxed-derived forms, then compare
that arg0-specific line with arg1_line.
In `@scripts/ci_e2e_scope.py`:
- Around line 87-90: Move the required acceptance assertions represented by
SOURCE_SUITE_MAP into in-crate unit tests for perry-codegen that run under
cargo-test; keep the mapping in scripts/ci_e2e_scope.py supplemental rather than
treating it as required coverage. Update
changelog.d/7675-stale-codegen-test-contract.md lines 67-76 to remove the claim
that this mapping closes the per-PR coverage gap, while retaining any accurate
supplemental-coverage description.
---
Nitpick comments:
In `@crates/perry-codegen/src/temp_root_coverage/mod.rs`:
- Around line 48-134: Remove the duplicate entry_opts and module_with_init
helpers from the coverage module and move their shared implementations into
crate::testing with public visibility. Update temp_root_operand_temporaries.rs
to import and use crate::testing::entry_opts and
crate::testing::module_with_init, preserving their current behavior and
signatures.
- Around line 219-229: Update the write-back assertion in assert_rooted_across
to match the stored slot value using the same slot_holding derivation rule as
the surrounding rooting checks, allowing the push result to pass through one
boxing step. Preserve the existing slot_traffic and SlotEvent usage for the
release-order assertion below.
In `@crates/perry-codegen/src/testing/temp_slots.rs`:
- Around line 391-413: Remove the explicit load-prefix handling block from the
touched-slot collection in the surrounding temp-slot analysis, since the
preceding “, ptr ” scan already captures those operands. Preserve the existing
generic scan and touched insertion behavior.
- Around line 338-365: The register-walking logic is duplicated between
derives_from and derives_from_slot_load and only follows the first predecessor
token. Extract a shared helper that walks defining predecessors and accepts a
caller-supplied stop predicate, then update both derives_from and
derives_from_slot_load to use it while preserving their existing termination
behavior.
- Around line 204-222: Update derives_from to traverse every register operand
referenced by each defining instruction rather than only the first % token. Use
a breadth-first or equivalent worklist walk with a visited set, preserving the
depth limit and returning true when any reachable register matches ancestor;
update callers only as needed to retain the existing rooted-value behavior.
- Around line 447-452: Update the alloca matching inside temp_root_slots to
recognize definitions whose text begins with the supported alloca type prefixes,
rather than requiring exact equality. Preserve matching for i64 and ptr
addrspace(1), including definitions with suffixes such as alignment metadata, so
assert_no_temp_rooting remains effective.
🪄 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: 9a43c73a-0c6e-46bb-bd98-1bdea3ce8045
📒 Files selected for processing (16)
changelog.d/7675-stale-codegen-test-contract.mdcrates/perry-codegen/src/lib.rscrates/perry-codegen/src/temp_root_coverage/mod.rscrates/perry-codegen/src/temp_root_coverage/operands.rscrates/perry-codegen/src/testing.rscrates/perry-codegen/src/testing/root_slots.rscrates/perry-codegen/src/testing/temp_slots.rscrates/perry-codegen/tests/native_proof_buffer_views.rscrates/perry-codegen/tests/native_proof_regressions.rscrates/perry-codegen/tests/native_proof_regressions/invalidation.rscrates/perry-codegen/tests/native_proof_support/mod.rscrates/perry-codegen/tests/scalar_replaced_slot_roots.rscrates/perry-codegen/tests/shadow_slot_hygiene.rscrates/perry-codegen/tests/temp_root_argument_temporaries.rscrates/perry-codegen/tests/temp_root_operand_temporaries.rsscripts/ci_e2e_scope.py
💤 Files with no reviewable changes (1)
- crates/perry-codegen/tests/temp_root_argument_temporaries.rs
| fn a_buffer_index_read_is_not_temp_rooted() { | ||
| under_both_lowerings(|lowering| { | ||
| let ir = element_read_fixture( | ||
| "buffer_index_operand.ts", | ||
| Expr::BufferIndexGet { | ||
| buffer: Box::new(Expr::LocalGet(0)), | ||
| index: Box::new(Expr::Integer(0)), | ||
| }, | ||
| Vec::new(), | ||
| ); | ||
| assert_no_temp_rooting(&ir, lowering); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
This negative test has no non-vacuity guard.
a_proven_index_typed_array_read_is_not_temp_rooted asserts at Lines 86-90 that the fixture reaches js_dynamic_string_or_number_add before it calls assert_no_temp_rooting. That guard exists because a negative assertion proves nothing if the fixture never reaches the rooted operand-pair lowering. a_buffer_index_read_is_not_temp_rooted runs the same fixture with a different element expression and omits the guard. If Expr::BufferIndexGet on this local folds, or if the add lowers numerically, the test passes for the wrong reason. That is the vacuity class this PR removes for #7503. Add the same guard.
💚 Proposed fix: add the reach guard
Vec::new(),
);
+ assert!(
+ ir.contains("call double `@js_dynamic_string_or_number_add`"),
+ "{lowering}: the fixture must actually reach the rooted \
+ operand-pair lowering, or this proves nothing:\n{ir}"
+ );
assert_no_temp_rooting(&ir, lowering);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn a_buffer_index_read_is_not_temp_rooted() { | |
| under_both_lowerings(|lowering| { | |
| let ir = element_read_fixture( | |
| "buffer_index_operand.ts", | |
| Expr::BufferIndexGet { | |
| buffer: Box::new(Expr::LocalGet(0)), | |
| index: Box::new(Expr::Integer(0)), | |
| }, | |
| Vec::new(), | |
| ); | |
| assert_no_temp_rooting(&ir, lowering); | |
| }); | |
| } | |
| fn a_buffer_index_read_is_not_temp_rooted() { | |
| under_both_lowerings(|lowering| { | |
| let ir = element_read_fixture( | |
| "buffer_index_operand.ts", | |
| Expr::BufferIndexGet { | |
| buffer: Box::new(Expr::LocalGet(0)), | |
| index: Box::new(Expr::Integer(0)), | |
| }, | |
| Vec::new(), | |
| ); | |
| assert!( | |
| ir.contains("call double `@js_dynamic_string_or_number_add`"), | |
| "{lowering}: the fixture must actually reach the rooted \ | |
| operand-pair lowering, or this proves nothing:\n{ir}" | |
| ); | |
| assert_no_temp_rooting(&ir, lowering); | |
| }); | |
| } |
🤖 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/temp_root_coverage/operands.rs` around lines 98 -
110, Add the same non-vacuity reach guard used by
a_proven_index_typed_array_read_is_not_temp_rooted to
a_buffer_index_read_is_not_temp_rooted, verifying the fixture reaches
js_dynamic_string_or_number_add before calling assert_no_temp_rooting. Keep the
existing lowering coverage and negative assertion unchanged.
| fn classify(fn_ir: &str, defs: &BTreeMap<&str, &str>, slot: &str) -> SlotKind { | ||
| match defs.get(slot) { | ||
| Some(&"alloca double") => SlotKind::Value, | ||
| Some(&"alloca i64") if fn_ir.contains(&format!("store i64 0, ptr {slot}\n")) => { | ||
| SlotKind::TempRoot | ||
| } | ||
| other => panic!( | ||
| "root slot {slot} is bound but its alloca ({other:?}) belongs to no \ | ||
| known slot family. Adding one is fine — classify it HERE, in \ | ||
| `testing::root_slots`, so every test that measures root traffic \ | ||
| sees it. Silently folding it into an existing total is how a \ | ||
| whole-module bind count stopped measuring its subject (#7504)." | ||
| ), | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Restrict TempRoot classification to the entry block.
Line 79 searches the full function body. An unrelated alloca i64 that is zeroed later and then bound is classified as TempRoot. This bypasses the unknown-slot-family failure that this reader must enforce. Inspect only the entry block for the initialization store. Add a fixture with a late zero store.
🤖 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/testing/root_slots.rs` around lines 76 - 90, Update
classify to recognize SlotKind::TempRoot only when the zero-initializing store
for the alloca appears in the function’s entry block, rather than anywhere in
fn_ir. Preserve the panic for bound allocas without a recognized entry-block
initialization, and add a fixture covering an alloca that is zeroed and bound
only in a later block.
| pub fn enclosing_function<'a>(ir: &'a str, needle: &str) -> &'a str { | ||
| let at = ir | ||
| .find(needle) | ||
| .unwrap_or_else(|| panic!("no `{needle}` in:\n{ir}")); | ||
| let start = ir[..at] | ||
| .rfind("\ndefine ") | ||
| .map(|i| i + 1) | ||
| .unwrap_or_else(|| panic!("`{needle}` is outside any function in:\n{ir}")); | ||
| let end = ir[start..] | ||
| .find("\n}") | ||
| .map(|i| start + i + 2) | ||
| .unwrap_or(ir.len()); | ||
| &ir[start..end] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Handle a function that starts at byte zero.
Line 218 only finds "\ndefine ". A valid single-function IR string begins with define and has no preceding newline. enclosing_function then panics for every needle in that first function. Accept a define marker at index zero and add a single-function test.
🤖 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/testing/root_slots.rs` around lines 213 - 225,
Update enclosing_function so its start-marker search recognizes define at byte
index zero as well as occurrences preceded by a newline, while preserving the
existing function-boundary behavior. Add a focused test covering a
single-function IR string that begins with define and verifies the enclosing
function is returned.
| #[test] | ||
| fn the_buffer_gep_reader_sees_an_unchecked_native_element_address() { | ||
| let body = probe_body(UNCHECKED_NATIVE_BUFFER_PROBE); | ||
| assert_eq!( | ||
| buffer_data_slots(body).into_iter().collect::<Vec<_>>(), | ||
| vec!["%d".to_string()], | ||
| "the data slot must be found from its initialising store" | ||
| ); | ||
| assert_eq!( | ||
| native_buffer_element_geps(body), | ||
| vec!["%r44".to_string()], | ||
| "the inbounds GEP off the data pointer is the unchecked access" | ||
| ); | ||
| assert_native_buffer_element_access(body, "self-test control"); | ||
| } | ||
|
|
||
| /// The regression that made the old assertion unable to fail: the shadow-stack | ||
| /// lowering's inline slot addressing emits `getelementptr inbounds i8` off a | ||
| /// pointer loaded out of a slot that is NOT a buffer view. | ||
| /// | ||
| /// Sabotage record — deleting the `slots.contains(&slot)` filter in | ||
| /// [`native_buffer_element_geps`] makes this test report `%r20`, `%r21` and | ||
| /// fail, which is the whole point: without that filter the reader degenerates | ||
| /// into the module-wide grep it replaced. | ||
| #[test] | ||
| fn the_buffer_gep_reader_ignores_the_shadow_frames_own_inline_slot_gep() { | ||
| let shadow_frame_probe = "\ | ||
| define double @perry_fn_m_ts__probe() { | ||
| entry.0: | ||
| %r3 = alloca ptr | ||
| %r4 = call ptr @js_shadow_frame_enter(i32 1) | ||
| store ptr %r4, ptr %r3 | ||
| %r9 = load ptr, ptr %r3 | ||
| %r18 = load ptr, ptr %r9 | ||
| %r19 = shl i64 %r14, 4 | ||
| %r20 = getelementptr inbounds i8, ptr %r18, i64 %r19 | ||
| %r21 = getelementptr inbounds i8, ptr %r20, i64 8 | ||
| ret double 0.0 | ||
| } | ||
| "; | ||
| let body = probe_body(shadow_frame_probe); | ||
| assert!( | ||
| buffer_data_slots(body).is_empty(), | ||
| "a shadow frame's state slot is not a Buffer data slot" | ||
| ); | ||
| assert!( | ||
| native_buffer_element_geps(body).is_empty(), | ||
| "the shadow frame's inline slot GEPs must not read as buffer accesses \ | ||
| — this is the exact text that turned sixteen `PERRY_RS4GC=0` tests \ | ||
| red for a reason unrelated to their subject (#7505)" | ||
| ); | ||
| } | ||
|
|
||
| /// …and a body with no buffer view at all is REFUSED, not reported clean. | ||
| #[test] | ||
| fn a_body_with_no_buffer_view_refuses_to_certify_the_absence_of_one() { | ||
| let no_buffer_probe = "\ | ||
| define double @perry_fn_m_ts__probe() { | ||
| entry.0: | ||
| ret double 0.0 | ||
| } | ||
| "; | ||
| let refused = std::panic::catch_unwind(|| { | ||
| assert_no_native_buffer_element_access(probe_body(no_buffer_probe), "self-test"); | ||
| }); | ||
| assert!( | ||
| refused.is_err(), | ||
| "a function with no Buffer view must not satisfy `no unchecked native \ | ||
| buffer access` — that is the vacuous pass this reader exists to stop" | ||
| ); | ||
| } | ||
|
|
||
| /// The slice must be the function, not the first line that mentions it. | ||
| #[test] | ||
| fn probe_body_slices_the_definition_and_not_its_wrapper() { | ||
| let ir = "\ | ||
| define double @__perry_wrap_perry_fn_m_ts__probe(i64 %this_closure) { | ||
| entry.0: | ||
| %w1 = getelementptr inbounds i8, ptr %w0, i32 0 | ||
| ret double 0.0 | ||
| } | ||
|
|
||
| define double @perry_fn_m_ts__probe() { | ||
| entry.0: | ||
| %r1 = add i32 0, 0 | ||
| ret double 0.0 | ||
| } | ||
| "; | ||
| let body = probe_body(ir); | ||
| assert!( | ||
| body.starts_with("define double @perry_fn_m_ts__probe()"), | ||
| "the wrapper must not be mistaken for the function under test: {body}" | ||
| ); | ||
| assert!( | ||
| !body.contains("%w1"), | ||
| "the slice must stop at the function it opened: {body}" | ||
| ); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Move PR-gating Buffer-proof coverage into in-crate unit tests.
These checks are acceptance coverage, but they exist only in integration-test targets under tests/. Put the required coverage in an in-crate test module that cargo-test executes. Retain integration coverage only as supplemental end-to-end coverage.
crates/perry-codegen/tests/native_proof_support/mod.rs#L429-L526: move the IR-reader self-tests into an in-crate test module.crates/perry-codegen/tests/native_proof_buffer_views.rs#L676-L693: add in-crate coverage for the multibyte-read negative case under both lowerings.crates/perry-codegen/tests/native_proof_regressions/invalidation.rs#L33-L87: add in-crate coverage for the proven-store control and invalidated-store fallback.
As per coding guidelines, “Do not rely on integration tests under crates/*/tests/*.rs for per-PR coverage; place acceptance coverage in unit tests visible to cargo-test when it must gate changes.”
📍 Affects 3 files
crates/perry-codegen/tests/native_proof_support/mod.rs#L429-L526(this comment)crates/perry-codegen/tests/native_proof_buffer_views.rs#L676-L693crates/perry-codegen/tests/native_proof_regressions/invalidation.rs#L33-L87
🤖 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/tests/native_proof_support/mod.rs` around lines 429 -
526, Move the IR-reader self-tests using probe_body, native_buffer_element_geps,
and assert_no_native_buffer_element_access from
crates/perry-codegen/tests/native_proof_support/mod.rs:429-526 into an in-crate
test module executed by cargo-test, retaining integration copies only as
supplemental coverage. Add equivalent in-crate multibyte-read negative coverage
for both lowerings from
crates/perry-codegen/tests/native_proof_buffer_views.rs:676-693, and equivalent
in-crate proven-store and invalidated-store fallback coverage from
crates/perry-codegen/tests/native_proof_regressions/invalidation.rs:33-87.
Source: Coding guidelines
|
|
||
| assert!( | ||
| bind_calls(&ir) > 0, | ||
| value_slot_binds(main_ir(&ir)) > 0, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Add unit-level acceptance coverage for these required contracts.
These assertions remain only in crates/*/tests/*.rs. Keep the integration tests, but add equivalent codegen acceptance coverage in a crates/perry-codegen/src test module so it gates each PR without relying on integration-suite selection.
crates/perry-codegen/tests/scalar_replaced_slot_roots.rs#L228-L228: add unit coverage for value-slot attribution, barriers, and entry-hoisted scalar-slot behavior.crates/perry-codegen/tests/shadow_slot_hygiene.rs#L927-L973: add unit coverage for reserved-slot bind-or-clear hygiene.
Based on coding guidelines, “Do not rely on integration tests under crates/*/tests/*.rs for per-PR coverage; place acceptance coverage in unit tests visible to cargo-test when it must gate changes.”
📍 Affects 2 files
crates/perry-codegen/tests/scalar_replaced_slot_roots.rs#L228-L228(this comment)crates/perry-codegen/tests/shadow_slot_hygiene.rs#L927-L973
🤖 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/tests/scalar_replaced_slot_roots.rs` at line 228, The
required contracts lack unit-level codegen acceptance coverage. Add equivalent
tests in a crates/perry-codegen/src test module for
crates/perry-codegen/tests/scalar_replaced_slot_roots.rs:228-228, covering
value-slot attribution, barriers, and entry-hoisted scalar-slot behavior, and
for crates/perry-codegen/tests/shadow_slot_hygiene.rs:927-973, covering
reserved-slot bind-or-clear hygiene; retain both existing integration tests.
Source: Coding guidelines
| let body = main_ir(&ir); | ||
| let value_slot = perry_codegen::testing::root_slots::bound_slots(body) | ||
| .into_iter() | ||
| .find(|(_, (kind, _))| *kind == perry_codegen::testing::root_slots::SlotKind::Value) | ||
| .map(|(slot, _)| slot) | ||
| .unwrap_or_else(|| panic!("no element slot was bound at all in:\n{body}")); | ||
| let bind = body | ||
| .find("call void @js_shadow_slot_bind(") | ||
| .expect("bind was located by enclosing_function"); | ||
| .find(&format!(", ptr {value_slot})")) | ||
| .unwrap_or_else(|| panic!("no bind naming {value_slot} in:\n{body}")); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Check initialization of the selected element slot.
The later store check counts any undefined store before this bind. A store to another alloca can satisfy it while value_slot remains uninitialized. Match store double {undef}, ptr {value_slot} before the bind.
Proposed fix
- let undef_store = format!("store double {undef}, ptr ");
+ let undef_store = format!("store double {undef}, ptr {value_slot}");🤖 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/tests/scalar_replaced_slot_roots.rs` around lines 687 -
695, Update the bind validation in the test around main_ir and the selected
value_slot so it searches, before the bind, for the exact initialization pattern
store double {undef}, ptr {value_slot}. Ensure the assertion targets value_slot
specifically rather than counting an undefined store to any other alloca.
| let touched: std::collections::BTreeSet<u32> = (0..reserved) | ||
| .filter(|idx| { | ||
| main_ir.contains(&format!("call void @js_shadow_slot_bind(i32 {idx}, ptr %")) | ||
| || main_ir.contains(&format!("call void @js_shadow_slot_set(i32 {idx}")) | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant test section and determine whether the delimiter fix is needed.
wc -l crates/perry-codegen/tests/shadow_slot_hygiene.rs
sed -n '920,980p' crates/perry-codegen/tests/shadow_slot_hygiene.rs
# Behavioral probe: reproduce the matching expression over sample main_ir strings containing slot indices.
python3 - <<'PY'
sample = """call void `@js_shadow_slot_set`(i32 1,
call void `@js_shadow_slot_set`(i32 10,
call void `@js_shadow_slot_bind`(i32 1, ptr %
call void `@js_shadow_slot_bind`(i32 10, ptr %
"""
reserved=11
touched=set()
for idx in range(reserved):
if sample.find(f"call void `@js_shadow_slot_bind`(i32 {idx}, ptr %") >= 0 or sample.find(f"call void `@js_shadow_slot_set`(i32 {idx}") >= 0:
touched.add(idx)
print("current touched:", sorted(touched))
print("current includes slot 1:", 1 in touched)
touched2=set()
for idx in range(reserved):
if sample.find(f"call void `@js_shadow_slot_bind`(i32 {idx}, ptr %") >= 0 or sample.find(f"call void `@js_shadow_slot_set`(i32 {idx},") >= 0:
touched2.add(idx)
print("delimited touched:", sorted(touched2))
PYRepository: PerryTS/perry
Length of output: 2857
Delimit the js_shadow_slot_set slot index.
The current predicate matches i32 1 inside i32 10, so slot 10 can count as a touch of slot 1 because the search string does not require the argument delimiter after idx.
🤖 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/tests/shadow_slot_hygiene.rs` around lines 952 - 956,
Update the touched-slot predicate in the BTreeSet construction to require an
argument delimiter after the index in js_shadow_slot_set calls, preventing an
index such as 10 from matching slot 1; preserve the existing js_shadow_slot_bind
matching behavior.
| let f = init_ir(&ir); | ||
| let slots = temp_root_slots(f); | ||
| assert!( | ||
| ir.contains("call i32 @js_gc_temp_root_push"), | ||
| !slots.is_empty(), | ||
| "the concat accumulator must be rooted across an allocating argument \ | ||
| (#6971):\n{ir}" | ||
| ); | ||
| (#6971):\n{f}" | ||
| ); | ||
| let traffic = slot_traffic(f); | ||
| let written_back = slots.iter().any(|slot| { | ||
| traffic[slot] | ||
| .iter() | ||
| .filter(|e| matches!(e, SlotEvent::Store { .. })) | ||
| .count() | ||
| >= 2 | ||
| }); | ||
| assert!( | ||
| ir.contains("call void @js_gc_temp_root_set"), | ||
| written_back, | ||
| "each js_string_concat yields a NEW address, so the accumulator must be \ | ||
| written back into its slot — otherwise the next argument's lowering \ | ||
| keeps the INPUT alive and sweeps the string under construction:\n{ir}" | ||
| ); | ||
| written BACK into its slot — otherwise the next argument's lowering \ | ||
| keeps the INPUT alive and sweeps the string under construction. Slot \ | ||
| traffic: {traffic:#?}\n{f}" | ||
| ); | ||
| let re_read = slots.iter().any(|slot| { | ||
| traffic[slot] | ||
| .iter() | ||
| .any(|e| matches!(e, SlotEvent::Load { .. })) | ||
| }); | ||
| assert!( | ||
| ir.contains("call i64 @js_gc_temp_root_get"), | ||
| re_read, | ||
| "the accumulator must be re-read after the argument (and its ToString \ | ||
| coercion, which also allocates):\n{ir}" | ||
| coercion, which also allocates):\n{f}" | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Three migrated assertions search every temp slot instead of the slot under test. The new helpers make value-specific claims possible, but these three clauses fold slot_traffic over all slots and pass if any slot shows the required event. An unrelated pooled slot in the same @main can satisfy them. This file states the standard these clauses break, at Lines 411-414: a module-wide search "would be answered by an unrelated named local's slot, which is the class of mistake this whole issue is about." Resolve the value's slot with slot_holding, then assert on that slot only.
crates/perry-codegen/tests/temp_root_operand_temporaries.rs#L241-L272: resolve the concat accumulator's slot withslot_holding, then apply the two-store write-back count and the load check to that slot alone.crates/perry-codegen/tests/temp_root_operand_temporaries.rs#L403-L410: resolve the slot for each constructor argument, then require aLoadafterinstance_allocin each of those slots, rather than oneLoadanywhere.crates/perry-codegen/tests/temp_root_operand_temporaries.rs#L1186-L1192: resolve theObject.assignaccumulator's slot, then require two or moreStoreevents in that slot, rather than in any temp slot.
📍 Affects 1 file
crates/perry-codegen/tests/temp_root_operand_temporaries.rs#L241-L272(this comment)crates/perry-codegen/tests/temp_root_operand_temporaries.rs#L403-L410crates/perry-codegen/tests/temp_root_operand_temporaries.rs#L1186-L1192
🤖 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/tests/temp_root_operand_temporaries.rs` around lines 241
- 272, Update all three assertions in
crates/perry-codegen/tests/temp_root_operand_temporaries.rs:241-272, 403-410,
and 1186-1192. Use slot_holding to resolve the specific accumulator or
constructor-argument slot, then inspect only that slot’s slot_traffic: require
the concat accumulator’s two Stores and Load, each constructor argument’s Load
after instance_alloc, and the Object.assign accumulator’s two or more Stores. Do
not search across all temp slots.
| let (arg1_line, _) = &arg_allocs[1]; | ||
| let (_, arg0) = &arg_allocs[0]; | ||
| let slot = slot_holding(f, arg0) | ||
| .unwrap_or_else(|| panic!("constructor argument 0 ({arg0}) is never rooted (#6969):\n{f}")); | ||
| let arg0_store = traffic[&slot] | ||
| .iter() | ||
| .find_map(|e| match e { | ||
| SlotEvent::Store { line, .. } => Some(*line), | ||
| _ => None, | ||
| }) | ||
| .expect("slot_holding just found it"); | ||
| assert!( | ||
| pushes[pushes.len() - 1] < instance_alloc && instance_alloc < get, | ||
| "every argument must still be rooted across the instance allocation, \ | ||
| and re-read after it:\n{ir}" | ||
| arg0_store < *arg1_line, | ||
| "argument 0 must be rooted BEFORE argument 1 is lowered — rooting the \ | ||
| whole list after the loop publishes an already-dangling pointer \ | ||
| (#6969). Store at {arg0_store}, argument 1 allocates at {arg1_line}:\n{f}" | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
arg0_store takes the first store in the slot without checking the stored value.
slot_holding on Line 381 proves that some store in slot derives from arg0. Lines 383-389 then take the first Store event in that slot, whatever value it names. Temp slots are pooled, so an earlier unrelated value can occupy the same slot and produce an earlier store line. The ordering assertion on Line 390 then passes on the wrong store, and a regression that roots argument 0 too late still passes. Filter the store by the same derivation rule slot_holding used.
🐛 Proposed fix: select the store that names argument 0
let arg0_store = traffic[&slot]
.iter()
.find_map(|e| match e {
- SlotEvent::Store { line, .. } => Some(*line),
+ SlotEvent::Store { value, line } if value == arg0 => Some(*line),
_ => None,
})
.expect("slot_holding just found it");If the stored register can be a boxed form of arg0, expose a helper in perry_codegen::testing::temp_slots that returns the store line for a value, so this test reuses the same derivation walk instead of an exact comparison.
🤖 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/tests/temp_root_operand_temporaries.rs` around lines 379
- 395, Update the store lookup associated with arg0 in the temp-slot ordering
assertion to filter by the same derivation rule used by slot_holding, rather
than selecting the first Store event in the pooled slot. Prefer exposing and
reusing a perry_codegen::testing::temp_slots helper that returns the store line
for a value, including boxed-derived forms, then compare that arg0-specific line
with arg1_line.
| "crates/perry-codegen/src/": [ | ||
| ("perry-codegen", "shadow_slot_hygiene"), | ||
| ("perry-codegen", "scalar_replaced_slot_roots"), | ||
| ("perry-codegen", "temp_root_operand_temporaries"), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Move required acceptance coverage into cargo-test.
SOURCE_SUITE_MAP selects only integration suites under crates/perry-codegen/tests/. Do not rely on this CI selection to gate required source-level acceptance coverage.
scripts/ci_e2e_scope.py#L87-L90: Keep this map supplemental, or replace its required contract coverage with in-crate unit tests visible tocargo-test.changelog.d/7675-stale-codegen-test-contract.md#L67-L76: Do not state that the mapping closes the per-PR coverage gap until the required assertions run incargo-test.
As per coding guidelines, “Do not rely on integration tests under crates/*/tests/*.rs for per-PR coverage; place acceptance coverage in unit tests visible to cargo-test when it must gate changes.”
📍 Affects 2 files
scripts/ci_e2e_scope.py#L87-L90(this comment)changelog.d/7675-stale-codegen-test-contract.md#L67-L76
🤖 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 `@scripts/ci_e2e_scope.py` around lines 87 - 90, Move the required acceptance
assertions represented by SOURCE_SUITE_MAP into in-crate unit tests for
perry-codegen that run under cargo-test; keep the mapping in
scripts/ci_e2e_scope.py supplemental rather than treating it as required
coverage. Update changelog.d/7675-stale-codegen-test-contract.md lines 67-76 to
remove the claim that this mapping closes the per-PR coverage gap, while
retaining any accurate supplemental-coverage description.
Source: Coding guidelines
Audit — merging as v0.5.1385Four issues resolved, one deliberately partial, and three corrections to my brief — all three of which found something nobody had named. The replacements can fail; I checked one myselfI sabotaged #7504's replacement independently: neutering Classifying each alloca and panicking on an unclassifiable one is the right shape — a counter that silently absorbs an unrecognised slot is how the original got into this state. Your three corrections
#7503 lists 10 failures; there are 11. The FFI fallback arm is not covered because CodeRabbit found an eleventh vacuity, in the PR that removes that defect class
Both declines are right. Broadening #7507 left partial, correctlyWiring in #6988 done the right wayDeleting Gates: 22/22 lint, fmt clean, |
|
Thanks — two of these are the exact defect class this PR exists to remove, and both are now fixed with a regression test each. Fixed: the Fixed: the write-back exact compare ( Fixed: the redundant load branch ( Declined: broaden Declined here: dedup |
…ollow-up) (#7681) * test(codegen): match the alloca TYPE, not its whole def text An `align` suffix would have emptied `temp_root_slots` and made every `assert_no_temp_rooting` vacuous again — the exact failure this PR removes. Matched through one shared `root_slots::alloca_type` in all three places that compared whole def text, with a regression test each that fails against the old compare. Also routes the accumulator write-back check through `slot_holding` so it tolerates the same boxing step its neighbours do, and drops a redundant load-specific pass in `zero_seeded_slots` that re-inserted what the generic `, ptr %s` scan already caught. Raised by review on #7675. Claude-Session: https://claude.ai/code/session_01Y1QZ5wUP9gRSwpiweT4Wix * docs: changelog fragment for the alloca-type prefix match Claude-Session: https://claude.ai/code/session_01Y1QZ5wUP9gRSwpiweT4Wix * chore: bump version to 0.5.1386 Claude-Session: https://claude.ini/code/session_01Y1QZ5wUP9gRSwpiweT4Wix --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
1 participant
Closes #7505, #7504, #7503, #6988. Partially addresses #7507 (see "What is NOT here").
Five codegen tests asserted something that is no longer the contract, or asserted it in a way that could not fail. Every replacement below was watched go red against a planted defect; the sabotages are named per issue.
#7505 —
assert_buffer_store_uses_dynamic_fallbackhad no subject!ir.contains("getelementptr inbounds i8")over the whole module. Any unrelatedinbounds i8satisfied it, and the shadow-stack lowering's own inline slot addressing (#7088) emits exactly that text — so underPERRY_RS4GC=0sixteen tests reported a stale proof that was never emitted. #7493 pinned them toNativeRootsPin::native(), which stopped the false alarm without making the assertion right.It now follows the data flow.
native_proof_support::native_buffer_element_gepsaccepts only aninboundsGEP taken off a pointer loaded out of a Buffer view's DATA slot, where a data slot is identified by either shape codegen materialises one with (the unboxedand/inttoptr/header-skip store, or the!invariant.loadheader length read). It is scoped to@probevia adefine-line anchor, not a substring — #7669'sfast_clone_sliceis why that is spelled out.assert_no_native_buffer_element_accesspanics when the function lowered no buffer view at all, so "there is no native GEP" and "there is nothing here to have one" stay distinguishable.All sixteen pins are gone; the tests pass under both lowerings.
slots.contains(&slot)filter — i.e. degrading the reader back into the module-wide grep — turns 19 tests red underPERRY_RS4GC=0, including the self-test that plants the shadow-frame GEP shape.invalidation::the_native_buffer_gep_detector_fires_on_a_proven_storecompiles the same store with its proof intact and requires the reader to SEE the GEP, under both lowerings, then requires the invalidated twin not to.#7504 — the bind count measured the wrong slots
bind_callscountedjs_shadow_slot_bindmodule-wide, and since #7487 a pooled temp root emits the identical call. Every fixture ends inconsole.log(o.a, o.b), whose argument accumulator contributes three binds. So== 0was a claim about the accumulator and== 1a coincidence.New
perry_codegen::testing::root_slotskeys every bind and every root-shading barrier by the entry alloca it names, classifies that alloca (Value= named local / scalar-replaced field,TempRoot= #7487's pool) and panics on one it cannot classify — so a third slot family goes red naming itself rather than being folded into a total. The threenumeric_only_*tests additionally asserttemp_root_slot_binds > 0: their zero is only interesting in a module where binds exist.root_scalar_replaced_slot'sroot_entry_allocacall — gc: scalar-replaced object/array locals holding heap values are not precise roots #6968 reverted — turns 7 positives red.expr_is_known_non_pointer_shadow_valuegate — fix(codegen): don't root typed-array element reads as GC temporaries (#6996) #6997 reverted — turns all threenumeric_only_*negatives red. That is the direction that was silently broken, so it is the one that most needed the proof.flat_const_row_aliases_do_not_reserve_shadow_slotsis renamed and re-pointed. Its two causes separate cleanly and the temp pool is not one of them: measured here the pool contributes 0 binds and 0 reservations, and all three reserved slots are locals'. The assertion it made was also not the contract —kernellowers to a real heap array, sokrow/khold heap pointers and leaving them unrooted would be #6968. Its second assertion had gone toothless independently: it forbadejs_shadow_slot_set(i32 1while #7013 moved the traffic tojs_shadow_slot_bind. It now asserts the hygiene property the suite exists for — every reserved slot is bound or cleared, i.e. no #7184-shaped reserved-but-untouched entry — and refuses an empty frame.#7503 — the temp-root contract had no working coverage in either direction
#7487 re-lowered temp roots onto pooled frame allocas;
js_gc_temp_root_push/_get/_set/_truncatesurvive only on an FFI fallback arm neither shipped lowering takes. Ten assertions failed and eight!ir.contains(…push)negatives held for every program in the language.New
perry_codegen::testing::temp_slotsreads slot traffic in both spellings (the RS4GC retype preserves register names, which is what lets one reader serve both) and states the contract as a claim about a value:assert_rooted_across(ir, producer_result, consumer)proves this producer's result reached a slot and this call read its operand back out of it. That is strictly stronger than the call-existence check it replaces.temp_root_operand_temporaries.rsis 19/19 green under both lowerings, unpinned.ctx.block().store(…)intemp_slot_storeturns all 4 positives in the moved suite red and correctly leaves the 3 negatives green.an_unrooted_accumulator_is_caught(pre-gc: console.log argument temporaries are not precise roots — a precise-roots-only collection drops string-literal args (minimal repro, no evacuation needed) #6951 SSA threading) androoted_but_not_re_read_is_caught(String literal operand is not GC-rooted across an allocating call in the same expression (stale handle after evacuation) #7114: rooted, location rewritten, consumer still reads the stale register — whichcontains("…temp_root_push")passes happily).#6988 — the contract now runs per-PR
tests/temp_root_argument_temporaries.rsis deleted; its seven tests live incrates/perry-codegen/src/temp_root_coverage/, an in-crate#[cfg(test)]module following #7653'snative_root_coveragepattern, so they run in the requiredcargo-testjob instead of the nightly-only tier (#5960). Each runs once per lowering viaunder_both_lowerings.What is NOT here (#7507)
SOURCE_SUITE_MAPis added toscripts/ci_e2e_scope.pywith the three suites that are green today, plus the on-disk cross-check (an entry naming a missing suite FAILS the scope step) and four new self-test cases including a sabotage of that cross-check against the real repo.native_proof_regressionsandnative_proof_buffer_viewsare deliberately absent. Each carries two pre-existing failures that are not this PR's — verified against pristinea853135aa, same four before and after — in typed-array artifact records and integer-modulo / typed-f64 clone lowering. A new gate has never been green, and wiring in a suite that is red on arrival makese2e-scopedred on most perry-codegen PRs; since that job is not in branch protection it cannot block anything, which is CLAUDE.md hazard 2 with extra steps. The comment in the map says so and says to add them in the commit that turns them green. #7507 stays open for that.Verification
cargo fmt --all -- --checkclean; all 21 script gates fromtest.yml'slintjob pass (22 enumerated, minus fmt run separately).cargo test -p perry-codegen --lib776 passed;-p perry-runtime --lib1917 passed;cargo check --all-targetsno errors.native_root_coveragetests pass.PERRY_RS4GC=0:shadow_slot_hygiene12/12,scalar_replaced_slot_roots11/11,temp_root_operand_temporaries19/19,native_proof_buffer_views34/36,native_proof_regressions258/260 — the 2+2 being the pre-existing failures above.#[cfg(test)]/feature = "testing"module, or a CI script.src/lib.rs's only change is a#[cfg(test)] modline. The shipped compiler is unchanged, so the emitted IR those arms read cannot move.Summary by CodeRabbit
Tests
Chores