fix(test): make the root-lowering pin reachable from integration suites, and say which lowering each asserts (#7493) - #7509
Merged
Conversation
proggeramlug
pushed a commit
that referenced
this pull request
Aug 6, 2026
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (16)
📝 WalkthroughWalkthroughThe change exposes ChangesNative-root testing support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant IntegrationTest
participant NativeRootsPin
participant Codegen
participant ArtifactHarness
IntegrationTest->>NativeRootsPin: select native or shadow lowering
IntegrationTest->>Codegen: compile module with selected lowering
Codegen->>ArtifactHarness: produce and locate module artifact
ArtifactHarness-->>IntegrationTest: return matching JSON artifact
NativeRootsPin-->>IntegrationTest: restore prior lowering state
Possibly related issues
Possibly related PRs
✨ 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 |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/codegen/helpers.rs`:
- Around line 173-203: Make NativeRootsPin non-Send so it cannot be moved across
threads and dropped on a different thread. Update the NativeRootsPin definition
using an existing thread-affinity marker pattern, while preserving its shadow(),
native(), and Drop behavior on the creating thread.
In `@crates/perry-codegen/src/codegen/testing_feature_gate_tests.rs`:
- Around line 109-129: Update the dependency scanning logic around
names_this_crate and the section-processing flow to resolve Cargo dependency
aliases via each table’s package field before accepting the testing-feature
gate. Recognize tables such as dependencies.codegen with package =
"perry-codegen", preserve existing direct-name handling, and add a planted
renamed-dependency test case covering features = ["testing"].
🪄 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: 9be38679-88fc-4abe-8a0d-e7de8e423326
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
changelog.d/7509-integration-suites-native-roots.mdcrates/perry-codegen/Cargo.tomlcrates/perry-codegen/src/codegen/helpers.rscrates/perry-codegen/src/codegen/mod.rscrates/perry-codegen/src/codegen/testing_feature_gate_tests.rscrates/perry-codegen/src/lib.rscrates/perry-codegen/src/testing.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.rs
Comment on lines
+173
to
203
| #[cfg(any(test, feature = "testing"))] | ||
| pub struct NativeRootsPin(Option<bool>); | ||
|
|
||
| #[cfg(test)] | ||
| #[cfg(any(test, feature = "testing"))] | ||
| impl NativeRootsPin { | ||
| /// Pin this thread to the shadow-stack lowering for the guard's lifetime. | ||
| pub(crate) fn shadow() -> Self { | ||
| /// Pin this thread to the **shadow-stack** lowering for the guard's | ||
| /// lifetime — Perry's heap-backed shadow frame, `js_shadow_frame_enter` + | ||
| /// per-slot binds. | ||
| pub fn shadow() -> Self { | ||
| NativeRootsPin(NATIVE_ROOTS_OVERRIDE.with(|c| c.replace(Some(false)))) | ||
| } | ||
|
|
||
| /// Pin this thread to the **native-roots** (RS4GC statepoint) lowering: | ||
| /// `ptr addrspace(1)` root allocas, `gc "statepoint-example"`, relocations | ||
| /// inserted by LLVM. | ||
| /// | ||
| /// This is today's default on every target the runtime can walk, so a test | ||
| /// that wants it does not strictly *need* the pin — but a pin is not | ||
| /// redundant: it also overrides `PERRY_RS4GC` from the environment, so the | ||
| /// assertion means the same thing during a `PERRY_RS4GC=0` bisection run as | ||
| /// it does in CI. Without it, a whole-suite sweep under the process-global | ||
| /// env knob silently retargets every unpinned test at the other lowering. | ||
| pub fn native() -> Self { | ||
| NativeRootsPin(NATIVE_ROOTS_OVERRIDE.with(|c| c.replace(Some(true)))) | ||
| } | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| #[cfg(any(test, feature = "testing"))] | ||
| impl Drop for NativeRootsPin { | ||
| fn drop(&mut self) { | ||
| NATIVE_ROOTS_OVERRIDE.with(|c| c.set(self.0)); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 3 'struct NativeRootsPin|impl (Send|Sync) for NativeRootsPin|NATIVE_ROOTS_OVERRIDE|thread::spawn' \
crates/perry-codegen/src crates/perry-codegen/testsRepository: PerryTS/perry
Length of output: 5148
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import pathlib, re
p = pathlib.Path("crates/perry-codegen/src/codegen/helpers.rs")
text = p.read_text()
struct = re.search(r'#[^\n]*\npub struct NativeRootsPin\(([^)]+)\)', text, re.M)
drop = re.search(r'impl Drop for NativeRootsPin\s*\{[^}]+fn drop\(&mut self\)\s*\{([^}]+)\}\s*\}', text, re.S)
print("struct:", struct.group(0) if struct else "NOT FOUND")
fields = struct.group(1) if struct else None
print("field_type:", fields)
print("drop_body:", drop.group(1).strip() if drop else "NOT FOUND")
print("mentions_NATIVE_ROOTS_OVERRIDE_in_drop:", "NATIVE_ROOTS_OVERRIDE" in (drop.group(1).strip() if drop else ""))
print("has_phantom_Rc :", "PhantomData<&'static (); PhantomData<std" in text or "PhantomData<std::rc::Rc" in text)
# Deterministic Rust marker model: a struct is Send/Sync iff all fields are Send/Sync, etc.
python3 - <<'PY'
fields = ("Option<bool>",)
sentinel = ("Rc<()>",)
def has_send(t):
return t in ("Option<bool>",)
def has_sync(t):
return t in ("Option<bool>",)
print("NativeRootsPin currently Send/Sync?", all(has_send(fields[0])), all(has_sync(fields[0])))
print("NativeRootsPin with PhantomData<Rc<()>> Send/Sync?", all(f in sentinel or f in ("Option<bool>",) for f in fields))
PY
PYRepository: PerryTS/perry
Length of output: 242
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import pathlib, re
p = pathlib.Path("crates/perry-codegen/src/codegen/helpers.rs")
text = p.read_text()
struct = re.search(r'#[^\n]*\npub struct NativeRootsPin\(([^)]+)\)', text, re.M)
drop = re.search(r'impl Drop for NativeRootsPin\s*\{[^}]+fn drop\(&mut self\)\s*\{([^}]+)\}\s*\}', text, re.S)
print("struct:", struct.group(0) if struct else "NOT FOUND")
fields = struct.group(1) if struct else None
print("field_type:", fields)
print("drop_body:", drop.group(1).strip() if drop else "NOT FOUND")
if drop:
print("has_NATIVE_ROOTS_OVERRIDE_in_drop:", "NATIVE_ROOTS_OVERRIDE" in drop.group(1).strip())
print("has_phantom_Rc :", "PhantomData<std::rc::Rc" in text)
print("NativeRootsPin currently Send/Sync?", (fields and fields.strip().startswith("Option<bool>")))
PYRepository: PerryTS/perry
Length of output: 425
Make NativeRootsPin thread-affine.
NativeRootsPin(Option<bool>) is Send, so a caller can move the guard to another thread. Drop restores NATIVE_ROOTS_OVERRIDE on that thread instead, while the original thread remains pinned and can use the wrong lowering.
Proposed fix
-pub struct NativeRootsPin(Option<bool>);
+pub struct NativeRootsPin(
+ Option<bool>,
+ std::marker::PhantomData<std::rc::Rc<()>>,
+);
@@
- NativeRootsPin(NATIVE_ROOTS_OVERRIDE.with(|c| c.replace(Some(false))))
+ NativeRootsPin(
+ NATIVE_ROOTS_OVERRIDE.with(|c| c.replace(Some(false))),
+ std::marker::PhantomData,
+ )
@@
- NativeRootsPin(NATIVE_ROOTS_OVERRIDE.with(|c| c.replace(Some(true))))
+ NativeRootsPin(
+ NATIVE_ROOTS_OVERRIDE.with(|c| c.replace(Some(true))),
+ std::marker::PhantomData,
+ )📝 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.
Suggested change
| #[cfg(any(test, feature = "testing"))] | |
| pub struct NativeRootsPin(Option<bool>); | |
| #[cfg(test)] | |
| #[cfg(any(test, feature = "testing"))] | |
| impl NativeRootsPin { | |
| /// Pin this thread to the shadow-stack lowering for the guard's lifetime. | |
| pub(crate) fn shadow() -> Self { | |
| /// Pin this thread to the **shadow-stack** lowering for the guard's | |
| /// lifetime — Perry's heap-backed shadow frame, `js_shadow_frame_enter` + | |
| /// per-slot binds. | |
| pub fn shadow() -> Self { | |
| NativeRootsPin(NATIVE_ROOTS_OVERRIDE.with(|c| c.replace(Some(false)))) | |
| } | |
| /// Pin this thread to the **native-roots** (RS4GC statepoint) lowering: | |
| /// `ptr addrspace(1)` root allocas, `gc "statepoint-example"`, relocations | |
| /// inserted by LLVM. | |
| /// | |
| /// This is today's default on every target the runtime can walk, so a test | |
| /// that wants it does not strictly *need* the pin — but a pin is not | |
| /// redundant: it also overrides `PERRY_RS4GC` from the environment, so the | |
| /// assertion means the same thing during a `PERRY_RS4GC=0` bisection run as | |
| /// it does in CI. Without it, a whole-suite sweep under the process-global | |
| /// env knob silently retargets every unpinned test at the other lowering. | |
| pub fn native() -> Self { | |
| NativeRootsPin(NATIVE_ROOTS_OVERRIDE.with(|c| c.replace(Some(true)))) | |
| } | |
| } | |
| #[cfg(test)] | |
| #[cfg(any(test, feature = "testing"))] | |
| impl Drop for NativeRootsPin { | |
| fn drop(&mut self) { | |
| NATIVE_ROOTS_OVERRIDE.with(|c| c.set(self.0)); | |
| #[cfg(any(test, feature = "testing"))] | |
| pub struct NativeRootsPin( | |
| Option<bool>, | |
| std::marker::PhantomData<std::rc::Rc<()>>, | |
| ); | |
| #[cfg(any(test, feature = "testing"))] | |
| impl NativeRootsPin { | |
| /// Pin this thread to the **shadow-stack** lowering for the guard's | |
| /// lifetime — Perry's heap-backed shadow frame, `js_shadow_frame_enter` + | |
| /// per-slot binds. | |
| pub fn shadow() -> Self { | |
| NativeRootsPin( | |
| NATIVE_ROOTS_OVERRIDE.with(|c| c.replace(Some(false))), | |
| std::marker::PhantomData, | |
| ) | |
| } | |
| /// Pin this thread to the **native-roots** (RS4GC statepoint) lowering: | |
| /// `ptr addrspace(1)` root allocas, `gc "statepoint-example"`, relocations | |
| /// inserted by LLVM. | |
| /// | |
| /// This is today's default on every target the runtime can walk, so a test | |
| /// that wants it does not strictly *need* the pin — but a pin is not | |
| /// redundant: it also overrides `PERRY_RS4GC` from the environment, so the | |
| /// assertion means the same thing during a `PERRY_RS4GC=0` bisection run as | |
| /// it does in CI. Without it, a whole-suite sweep under the process-global | |
| /// env knob silently retargets every unpinned test at the other lowering. | |
| pub fn native() -> Self { | |
| NativeRootsPin( | |
| NATIVE_ROOTS_OVERRIDE.with(|c| c.replace(Some(true))), | |
| std::marker::PhantomData, | |
| ) | |
| } | |
| } | |
| #[cfg(any(test, feature = "testing"))] | |
| impl Drop for NativeRootsPin { | |
| fn drop(&mut self) { | |
| NATIVE_ROOTS_OVERRIDE.with(|c| c.set(self.0)); | |
| } | |
| } |
🤖 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/codegen/helpers.rs` around lines 173 - 203, Make
NativeRootsPin non-Send so it cannot be moved across threads and dropped on a
different thread. Update the NativeRootsPin definition using an existing
thread-affinity marker pattern, while preserving its shadow(), native(), and
Drop behavior on the creating thread.
Comment on lines
+109
to
+129
| // Only dependency-table lines matter, and only ones naming this crate. | ||
| let names_this_crate = | ||
| line.contains("perry-codegen") || section.iter().any(|s| s == "perry-codegen"); | ||
| if !names_this_crate { | ||
| continue; | ||
| } | ||
| // `features = [..., "testing", ...]` — quoted, so a `testing = []` | ||
| // feature DEFINITION in `[features]` is not mistaken for an edge. | ||
| if !line.contains("\"testing\"") { | ||
| continue; | ||
| } | ||
| if is_production_dependency_section(§ion) { | ||
| offenders.push(format!( | ||
| "{label}:{}: [{}] enables perry-codegen's `testing` feature: {}", | ||
| lineno + 1, | ||
| section.join("."), | ||
| line.trim() | ||
| )); | ||
| } else if section.iter().any(|seg| seg == "dev-dependencies") { | ||
| saw_dev_edge = true; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Resolve Cargo dependency aliases before accepting this gate.
Line 111 does not identify a renamed dependency table. Cargo accepts [dependencies.codegen], package = "perry-codegen", and features = ["testing"]. The feature line contains neither perry-codegen nor a perry-codegen section segment, so this production edge passes the gate.
Parse dependency tables with TOML semantics, or track each table’s package field before processing its features field. Add a planted renamed-dependency case to this 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/codegen/testing_feature_gate_tests.rs` around lines
109 - 129, Update the dependency scanning logic around names_this_crate and the
section-processing flow to resolve Cargo dependency aliases via each table’s
package field before accepting the testing-feature gate. Recognize tables such
as dependencies.codegen with package = "perry-codegen", preserve existing
direct-name handling, and add a planted renamed-dependency test case covering
features = ["testing"].
added 4 commits
August 6, 2026 11:30
…es (#7493) #7370 made native roots (RS4GC statepoints) the default lowering. `NativeRootsPin` was added so a test asserting on shadow-stack IR could say so, the in-crate unit tests were repaired with it — and the five integration suites that assert the same mechanics had no pin to reach for, because `NativeRootsPin` is `#[cfg(test)]` and `tests/*.rs` link this crate as an external consumer. They run nightly/at-tag only, so nothing went red at merge time and `shadow_slot_hygiene` sat at 0/12 on `main`. The pin is now reachable as `perry_codegen::testing::NativeRootsPin`, behind a `testing` cargo feature that only this crate's own `[dev-dependencies]` entry enables. With the feature off the pin, its thread-local and the branch it adds to `rs4gc_enabled()` are `#[cfg]`-ed out of the artifact — not merely private, absent — and cargo builds dev-dependencies for test/bench targets only, so no production profile can reach it. `NativeRootsPin::native()` joins `shadow()`, because a pin also outranks `PERRY_RS4GC` and that is what keeps an assertion meaning the same thing during a `PERRY_RS4GC=0` sweep. Per-test classification (not per file — the files disagree internally): * `shadow_slot_hygiene` — 12/12 shadow. The file's subject IS the shadow frame; 0/12 -> 11/12. * `scalar_replaced_slot_roots` — 11/11 shadow (every test counts `js_shadow_slot_bind` sites). 2/11 -> 5/11. * `temp_root_operand_temporaries` — 2 shadow, the rest unpinned and lowering-independent. 12/19 -> 13/19. * `temp_root_argument_temporaries` — none. `PERRY_RS4GC=0` moves it not at all; its failures are #7487's, not #7370's. * `native_proof_regressions` — 2 shadow, 15 native in `invalidation`. 249/253 -> 253/255 single-threaded, 198/253 -> 253/255 in parallel. * `native_proof_buffer_views` — 1 native. 28/30 -> 30/32. Two tests were pinned though they were PASSING: `numeric_only_scalar_replaced_{object,array}_emits_no_rooting` and `a_collection_free_construction_emits_no_this_slot_root` assert `bind_calls(&ir) == 0` / `!contains("@js_shadow_slot_bind")`, which under the native default is true of every program. They were green without their subject running (CLAUDE.md hazard 4). Pinned, the first two now fail for a real reason (#7497). Also fixed here because it hid this suite's real signal: `native_proof_ regressions` reported 55 failures under default parallelism and 4 under `--test-threads=1`. 51 of the 55 were `PoisonError` — #7490's shape again. The `PERRY_NATIVE_REPS*` env vars are process-global and the restore was hand-written after the compile, so a panic inside `compile_module` left them installed and every later unlocked compile wrote artifact JSON into a directory another test was reading; the torn read panicked inside the lock and poisoned it. The harness is now one shared `tests/native_proof_support/mod.rs`: a poison-tolerant accessor, an RAII env guard, and an artifact reader that treats a foreign or half-written neighbour as noise. Two sabotage tests plant each failure shape and assert the fix is what prevents it. Finally, a tripwire in `src/` — so it runs in the REQUIRED `cargo-test` job, which is the tier this whole issue is about not being in: `host_target_lowering_default_is_native_roots` fails the moment the default flips again, naming the suites that then need re-pinning. It asserts its subject is live (both pins must give different answers; the unsupported-target arm must give the opposite default), so a constant-folded `rs4gc_enabled()` fails it rather than passing it. A second gate scans every workspace manifest and fails if a non-dev dependency edge ever enables the `testing` feature. Refs #7493.
proggeramlug
force-pushed
the
fix/7493-integration-suites-native-roots
branch
from
August 6, 2026 09:30
10d8f3d to
13fd833
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #7493.
What was wrong
#7370 made native roots (RS4GC statepoints) the default lowering.
NativeRootsPinwas added so a test asserting on shadow-stack IR could say so; the in-crate unit tests were repaired with it. Five integration suites assert the same mechanics and had no pin to reach for —NativeRootsPinis#[cfg(test)], andtests/*.rslink the crate as an ordinary external consumer, so the item does not exist for them. Those suites run nightly/at-tag only, so nothing turned red at merge time andshadow_slot_hygienesat at 0/12 onmain.1. The pin mechanism, and why it cannot leak
perry_codegen::testing::NativeRootsPin, behind atestingcargo feature that only this crate's own[dev-dependencies]entry enables.Three shapes were available; the other two were rejected:
#[doc(hidden)] pub, unconditional — the pin would ship in everyperrybinary, and the thread-local read it adds at the top ofrs4gc_enabled()would be compiled into the production decision path. A knob that is merely undocumented is still a knob, and this repo has a written policy about modes nobody decided to have (CLAUDE.md, "GC knob kill-policy").#[cfg(test)]modules (CI: PR cargo-test never executes crates/perry integration suites; main-push full run starves under merge trains (documented near-miss) #5960's suggestion, and the issue's). Correct in principle — it would also put them in the per-PRcargo-testgate — butnative_proof_regressions.rsalone is 14k lines against a 2000-line file cap. Not a mechanical move.The feature's safety is structural, not a promise:
With
testingoff,NativeRootsPin, its backing thread-local, and theif let Some(pinned) = …arm inrs4gc_enabled()are#[cfg]-ed out of the artifact — absent, not private-and-unreachable.Cargo resolves dev-dependency features only when a test/bench target is built. Verified on this branch:
No
testing. With dev-dependencies included it appears; that is the only edge.A production edge added later would be a silent regression, so it is gated:
codegen::testing_feature_gate_testsscans every workspace manifest and fails if any[dependencies]/[build-dependencies]/[target.*.dependencies]table enablesperry-codegen'stesting. It lives insrc/, so it runs incargo-test— a required context. It asserts its own subject was live (must find >10 manifests, must find this crate's, must find the legitimate dev edge) and is sabotage-tested against a planted production edge.NativeRootsPin::native()joinsshadow(). Native roots being today's default does not make it redundant: a pin also outranksPERRY_RS4GC, so a pinned assertion means the same thing during thePERRY_RS4GC=0sweep a GC engineer actually runs.2. Per-test classification
Per test, not per file — several files disagree internally, and one of the findings below is exactly a case where a blanket pin would have been wrong.
shadow_slot_hygienescalar_replaced_slot_rootstemp_root_operand_temporariestemp_root_argument_temporariesnative_proof_regressionsnative_proof_buffer_viewstyped_feedback(#7492, untouched)Reasoning:
shadow_slot_hygiene— 12/12 shadow. The file's subject is the shadow frame: slot reservation, bind/clear ordering, slot indices, the post-init frame region. Native roots have no frame, no index and no bind — nothing to translate.scalar_replaced_slot_roots— 11/11 shadow. Every test measuresjs_shadow_slot_bind/js_shadow_frame_entercall sites.temp_root_operand_temporaries— 2 shadow.the_inline_ctor_this_slot_is_bound_as_a_shadow_slotanda_collection_free_construction_emits_no_this_slot_rootassert the shadow spelling of thethis-slot root. The other 17 are lowering-independent and stay unpinned.temp_root_argument_temporaries— none.PERRY_RS4GC=0moves it not at all (3/7 either way). Its failures are perf(codegen): lower temp roots onto pooled frame allocas — three FFI calls per temporary become a store and a load #7487's, not feat(gc): make native roots (statepoints) the default #7370's.native_proof_regressions— 2 shadow, 15 native. The two assert the shadow spelling of a box-pointer slot (store i64 <bits>, ptr %slotvsstore ptr addrspace(1) %rs4gc.sN). The fifteen ininvalidationare pinned native for a different reason, given below.native_proof_buffer_views— 1 native, same reason asinvalidation.Two corrections to the issue's own data, both from running the tests alone:
typed_f64_receiver_method_clone_raw_loads_after_composed_guardsis listed there as healed byPERRY_RS4GC=0. It is not — alone it fails under both lowerings. That reading was a whole-suite ordering artifact. It is left deliberately unpinned, with a comment saying so, and filed as typed_f64_receiver_method_clone_raw_loads_after_composed_guards: the guard-failure edge no longer calls $generic — miscompile or intentional collapse? #7506.native_proof_regressionsreported 55 failures at default parallelism and 4 single-threaded; see §4.Two tests were pinned even though they were passing
numeric_only_scalar_replaced_{object,array}_emits_no_rootingassertbind_calls(&ir) == 0, anda_collection_free_construction_emits_no_this_slot_rootasserts!contains("@js_shadow_slot_bind"). Under the post-#7370 default those hold for every program, rooted or not — they were green without their subject ever running (CLAUDE.md hazard 4). Pinning them makes them assert their subject again. The first two now fail for a real reason (#7504); the third passes and is now meaningful.That is a deliberate green→red: a red test that is measuring something beats a green one that is not.
Why the
invalidationfifteen are pinned nativeassert_buffer_store_uses_dynamic_fallbackproves the absence of a native buffer GEP with a module-wide!ir.contains("getelementptr inbounds i8"). The shadow lowering's inline slot addressing (#7088) emits exactly that instruction for reasons unrelated to any buffer, so underPERRY_RS4GC=0fifteen tests report a stale proof that was never there. The pin stops the false alarm; the assertion is still wrong and is filed as #7505 with the data-flow fix it should have.3. Which shipped-lowering mechanics are now uncovered — #7502
This is the part "the tests are green again" hides. 23 tests now pin the shadow stack, which is not what ships on aarch64 or x86_64. Nine distinct mechanics have no native-roots assertion anywhere:
addrspace(1)and is in the stack mapaddrspace(1)js_gc_initaddrspace(1)thisslot of an inline ctor is rooted (#7202/#7207)vardecls stay inside the frame (#7184's shape)Six of the seven "no" rows are shapes
docs/src/internals/gc-rooting-invariant.mdrecords as having already shipped broken.gc-root-dominance.ymlis not a substitute: it checks store ordering in emitted IR, not whether a value is a root at all — a value the lowering declines to put in the map has no store for it to check.Three of those tests were found passing vacuously in this PR. There is no reason to think they are the only three; they are the three that happened to sit next to a lowering pin.
#7502 carries the full table, the three candidate assertion surfaces (pre-
optIR / post-RS4GC live sets / the emitted__perry_gcmapsection) and a sabotage requirement for each new test, including for negative assertions specifically.4. Also fixed: the poison cascade that hid this suite's signal
native_proof_regressionsreported 55 failures at default parallelism and 4 under--test-threads=1. 51 of the 55 werePoisonErroratARTIFACT_ENV_LOCK.lock().unwrap()— #7490's shape again, and the scheduler picked the victims, which reads as order-dependent codegen state.Root cause, two defects in one hand-rolled helper (present in identical copy-pasted form in both
native_proof_*suites):PERRY_NATIVE_REPS*env vars are read bycompile_modulefrom the process environment and the restore was written out by hand after the compile — so a panic insidecompile_moduleleft them installed for the rest of the binary. Every later compile, including the manycompile_irones that never take this lock, then wrote artifact JSON into a directory another test was reading.serde_json→EOF while parsing a value) panicked inside the lock, poisoning it.Both copies are replaced by one shared
tests/native_proof_support/mod.rs: a poison-tolerant accessor, an RAII env guard, and an artifact reader that treats a foreign or half-written neighbour as noise while still naming everything it skipped if the subject turns up missing. Two sabotage tests plant each failure shape and assert the fix is what prevents it — they fail against the pre-fix code, which is what makes a green run evidence.Result: 198/253 → 253/255 at default parallelism, identical to single-threaded.
(Extracting the helper also brought
native_proof_buffer_views.rsfrom 2059 back to 1963 lines, under the file-size gate.)5. CI visibility — what was decided and why
ci_e2e_scope.pyalready runs the suites a diff names, so all six run on this PR. What it deliberately refuses is the source→suite direction, and that is the hole #7370 fell through: it changedcrates/perry-codegen/src/, named no suite, and the per-PRcargo-testgate is--lib --bins.Cost is not the objection. These are in-process compiles of hand-built HIR with
emit_ir_only: true— noperry compilesubprocess, no link. Measured,--test-threads=1: 0.17s, 0.18s, 0.08s, 0.39s, 0.50s, 5.13s — ~6.5s for all six.The mapping is not added here, on purpose. Fifteen tests across these suites are red for three causes this PR does not own (#7494, #7503, #7504).
e2e-scopedis not in branch protection's required contexts, so wiringcrates/perry-codegen/src/** → these sixtoday would produce a job that is red on most perry-codegen PRs and cannot block anything — CLAUDE.md hazard 2 with extra steps. Reviewers learn to ignore it, and it can then never be promoted. A new gate has never been green; this one would start red by construction.Filed as #7507 with the concrete
SOURCE_SUITE_MAPchange, the--self-testextension, and the "an entry that matches nothing FAILS" cross-check — blocked on the residue issues.What IS added, and is green today, is the tripwire that catches the exact #7370 shape in a required context:
host_target_lowering_default_is_native_roots, a unit test insrc/(so it runs incargo-test). It fails the moment the root-lowering default flips again, with the follow-on work named in the failure message. It asserts its subject is live — the two pins must give different answers, and the unsupported-target arm (arm64_32-apple-watchos) must give the opposite default — so anrs4gc_enabled()folded to a constant fails it rather than passing it.Verification
Three consecutive runs at default parallelism and three at
--test-threads=1, all seven suites. Results identical in all six sweeps — no order dependence, no parallelism dependence:cargo fmt --all -- --checkclean;scripts/check_file_size.shclean;scripts/check_test_registration.pyclean (162 files / 4 registries);scripts/ci_e2e_scope.py --self-testok;cargo check -p perryclean.The 15 remaining failures, every one filed
No test was deleted, skipped or weakened.
integer_modulo::…keep_frem,proven_buffer_and_typed_array_reads…,reassigned_typed_array_store…temp_root_{argument,operand}_temporariesscalar_replaced_slot_roots+flat_const_row_aliases…bind_callscounts module-wide and #7487's pooled temp roots emitjs_shadow_slot_bindtootyped_f64_receiver_method_clone…$genericTwo of those (#7503, #7504) are themselves coverage findings, not just red tests: between them, the #6951/#6969/#6970/#6971/#7114/#7154/#7200 temp-root contract currently has no working assertion in either direction, and #6997's "numeric literals pay no rooting" is unmeasurable.
Follow-ups filed
bind_callsmeasures the wrong slots since perf(codegen): lower temp roots onto pooled frame allocas — three FFI calls per temporary become a store and a load #7487.assert_buffer_store_uses_dynamic_fallback's module-wide GEP grep.typed_f64_receiver_method_clone…'s missing$genericedge.ci_e2e_scopesource→suite map, blocked on the above being green.Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores