Repository navigation
test(gc): temp-root codegen contract lives only in crates/perry-codegen/tests/, which does not run per-PR #6988
Description
Split out of #6983, where it was raised in review and explicitly declined rather
than silently skipped.
The gap
All of the temp_root family's codegen coverage lives in
crates/perry-codegen/tests/ — temp_root_argument_temporaries.rs (#6972) and
temp_root_operand_temporaries.rs (#6983), 10 tests between them. Per CLAUDE.md:
Integration suites under
crates/*/tests/*.rsdo not run per-PR (nightly/tag
only) — a regression there can land green and sit red for days. Prefer putting
acceptance coverage incargo-test-visible unit tests (#5960).
So the assertions pinning the rooting emission contract — push → re-read →
consuming call → release, and the interleaving requirement — are CI-invisible on
the PRs most likely to break them. The runtime half (gc::tests::temp_roots,
in src/) does run per-PR; the codegen half does not.
That matters more than usual for this family, because the failure mode is
silent. A lowering path that quietly goes back to threading a value through an
SSA register produces correct output under the default configuration — the
conservative native-stack scan pins it by accident — and only diverges under
PERRY_CONSERVATIVE_STACK_SCAN=off or an evacuating minor. The emission-contract
tests are the fast signal; the corpus arms are the slow one.
Why it was not fixed in #6983
The assertions need a built Module plus compile_module, so moving them into
src/ unit tests means either constructing a FnCtx by hand or duplicating the
~90 lines of entry_opts() / module_with_init() harness. Doing that once for
the whole family is worth it; doing it piecemeal for one PR's three tests is not,
and would leave the two files inconsistent.
Suggested shape
Extract the harness (entry_opts, module_with_init, ir_for, plus the
ir_for_new class builder from #6983) into something both a src/ unit-test
module and the existing integration tests can call — a #[cfg(test)] helper
module in perry-codegen, or a small pub(crate) test-support module — then
move at least the ordering/interleaving assertions across:
- argument accumulator: push → fused push → re-read → truncate (fix(gc): make argument temporaries precise roots (#6951) #6972)
MapSet: receiver rooted before the key is lowered (fix(gc): operand temporaries are precise roots — collection methods, constructor args, string-method receiver (#6970, #6969, #6971) #6983)- constructor args:
push(arg0)before argument 1's own allocation (fix(gc): operand temporaries are precise roots — collection methods, constructor args, string-method receiver (#6970, #6969, #6971) #6983) - the negative gates: shapes that must emit no rooting calls at all
The negative gates matter as much as the positive ones — they are what keeps the
fix from silently becoming a cost on hot paths.
Activity
jdalton commentedon Jul 30, 2026
Heads-up on a small campaign I'm lining up around the rooting family, routed through this issue first — flagging it here so nobody double-works it and so the sequencing is easy to veto.
The plan, in order:
maintoday and mergeable again; independent measurement infra, not part of the stack.cargo test -p perry-runtimeis green on macOS while working in the GC area. Also independent.cargo testrather than nightly-onlycrates/perry-codegen/tests/. This becomes the base of a stack.lower_new's non-class branches), gc: a closure's capturedthisis stored in an alloca that is not shadow-bound (methods bind theirs) — same family as #7055 #7066 (closure-capturedthisshadow binding), and runtime: a fourth unrooted-operand family — js_string_coerce as a plain ToString argument coercion (string built-ins, constructors) #6949 (thejs_string_coerceToString-coercion rooting family) — each carrying a sabotage-verified regression test that lands in the per-PR-visible harness from step 3.How the stack works from a fork
I contribute from a fork and can't create base branches in this repo, so this is an annotated stack rather than a GitHub-native one: each PR targets
main, is opened in dependency order with a "stacked on #X" note, and until the layer below merges its diff will include that layer's commits. As each layer squash-merges I'llrebase --ontothe layers above so the diffs collapse to just their own change. If any of these is already spoken for, or you'd rather the harness extraction wait, say the word and I'll reorder.proggeramlug commentedon Aug 9, 2026
Done in #7675.
crates/perry-codegen/tests/temp_root_argument_temporaries.rsis deleted. Its seven tests live incrates/perry-codegen/src/temp_root_coverage/— an in-crate#[cfg(test)]module, so they run in the required per-PRcargo-testjob instead of the nightly/tag-only integration tier (#5960).The shape follows #7653's
native_root_coveragerather than being invented:src/module, hand-built HIR,emit_ir_only: true, noperry compilesubprocess. The ~90 lines ofentry_opts()/module_with_init()harness the original issue balked at are written once there.Coverage moved, per the issue's list:
console_argument_accumulator_is_rooted_re_read_and_written_back: alloc → store into slot → re-read → push → write back → re-read → consume → release, with the ordering asserted between events rather than between textual offsets.a_non_allocating_element_list_pays_for_no_temp_root,operands::a_proven_index_typed_array_read_is_not_temp_rooted,operands::a_buffer_index_read_is_not_temp_rooted. Each is paired with a structurally identical twin that must go the other way (an_array_literal_roots_elements_before_an_allocating_element,operands::a_symbol_keyed_typed_array_read_is_still_temp_rooted,operands::an_unproven_key_typed_array_read_is_still_temp_rooted), so a compiler that roots nothing fails one half and one that roots everything fails the other.Each test runs once per lowering through
under_both_lowerings, so aPERRY_RS4GC=0bisection and CI assert the same thing — the divergence #7493 spent a week diagnosing.The
MapSetand constructor-argument interleaving assertions the issue also names stay intests/temp_root_operand_temporaries.rs, re-pointed at the same reader (#7503) and green under both lowerings. They are the half that still needs theClass/Modulebuilders; moving them is a follow-up, not a blocker — the family now has a per-PR-visible floor where it had none.One caveat worth recording: this does not by itself put the whole family in the required tier. What it puts there is the emission contract's core plus its negative gates, which is what the issue asked for ("move at least the ordering/interleaving assertions across"). #7507 covers getting the remaining suites scoped into
e2e-scopedon a perry-codegen source diff.test(codegen): make the stale codegen-test contracts able to fail aga…
proggeramlug commentedon Aug 9, 2026
Merged as 6cdcd79 (#7675). The temp-root emission contract now runs in the required per-PR
cargo-testjob viacrates/perry-codegen/src/temp_root_coverage/, following #7653'snative_root_coveragepattern;tests/temp_root_argument_temporaries.rsis deleted. Details in the comment above.