From d314bafea85ea169f7298a52a792cdcf868fa9f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 3 Aug 2026 18:09:59 +0200 Subject: [PATCH] test(gc): enable the knob loop_safepoint_purity asserts on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six of seven tests were red on main. Every assertion in the file is about which loops keep their back-edge poll, but the poll is gated behind PERRY_GC_MOVING_LOOP_POLLS, which defaults off — so the emitter returned before the purity analysis ran, and the sabotage cases saw no poll for a reason that has nothing to do with what they test. The suite now sets the knob before the first codegen call, since moving_safepoint_polls_enabled caches in a OnceLock. It stayed red because integration suites under crates/*/tests/ do not run per-PR. Note this covers the poll-ON path only; the default OFF path, which is what ships, remains untested. --- .../7320-loop-safepoint-purity-knob.md | 23 +++++++++++++++++++ .../tests/loop_safepoint_purity.rs | 18 +++++++++++++++ 2 files changed, 41 insertions(+) create mode 100644 changelog.d/7320-loop-safepoint-purity-knob.md diff --git a/changelog.d/7320-loop-safepoint-purity-knob.md b/changelog.d/7320-loop-safepoint-purity-knob.md new file mode 100644 index 0000000000..cb9801e868 --- /dev/null +++ b/changelog.d/7320-loop-safepoint-purity-knob.md @@ -0,0 +1,23 @@ +### Fix `loop_safepoint_purity`, red on `main` since the moving-GC work + +Six of the seven tests in `crates/perry-codegen/tests/loop_safepoint_purity.rs` +were failing on `main`. Every assertion in that file is about *which* loops keep +their back-edge `js_gc_loop_safepoint()` poll, but the poll is gated behind +`PERRY_GC_MOVING_LOOP_POLLS`, which defaults off — so the emitter returned +before the purity analysis ran and the six "sabotage" cases (a call in the body, +an object literal, string concatenation, a module-global accumulator, coercible +bound/accumulator) saw no poll for a reason unrelated to what they test. The one +test asserting a *pure* numeric loop drops its poll passed for the same wrong +reason. + +The suite now enables the knob it asserts on, before the first codegen call — +`moving_safepoint_polls_enabled` caches in a `OnceLock`. + +Worth noting how it stayed red: integration suites under `crates/*/tests/` do +not run per-PR (nightly/tag only), which CLAUDE.md already calls out as a way a +regression "can land green and sit red for days". + +This makes the suite exercise the poll-ON path. The **default** path — polls +off, which is what ships — remains untested, and `PERRY_GC_MOVING_LOOP_POLLS` +is a GC knob with no CI arm covering either state. That is a kill-policy +question for the owner, not something this change decides. diff --git a/crates/perry-codegen/tests/loop_safepoint_purity.rs b/crates/perry-codegen/tests/loop_safepoint_purity.rs index f312c95a6c..d81c35467d 100644 --- a/crates/perry-codegen/tests/loop_safepoint_purity.rs +++ b/crates/perry-codegen/tests/loop_safepoint_purity.rs @@ -125,7 +125,24 @@ fn module_with_init(name: &str, init: Vec) -> Module { } } +/// The back-edge poll is gated behind `PERRY_GC_MOVING_LOOP_POLLS`, which +/// defaults OFF. Every assertion in this file is about WHICH loops keep the +/// poll, so with the knob unset the emitter returns before the purity analysis +/// runs and six of these tests fail for a reason that has nothing to do with +/// what they test. +/// +/// That is how they went red: the knob arrived with the moving-GC work and +/// nothing here turned it on, while integration suites under `crates/*/tests/` +/// do not run per-PR, so nobody saw it. `moving_safepoint_polls_enabled` +/// caches in a `OnceLock`, so this must run before the first codegen call — +/// every helper that produces IR goes through here. +fn enable_back_edge_polls() { + // Safe on edition 2021, and every test in this binary wants the same value. + std::env::set_var("PERRY_GC_MOVING_LOOP_POLLS", "1"); +} + fn ir_for(name: &str, init: Vec) -> String { + enable_back_edge_polls(); String::from_utf8(compile_module(&module_with_init(name, init), entry_opts()).unwrap()) .expect("LLVM IR should be UTF-8") } @@ -133,6 +150,7 @@ fn ir_for(name: &str, init: Vec) -> String { /// Same, but `exported` names are exported module variables — which is what /// promotes a module-level `let` to a `@perry_global_*` slot. fn ir_for_with_exported_vars(name: &str, init: Vec, exported: &[&str]) -> String { + enable_back_edge_polls(); let mut m = module_with_init(name, init); m.exported_objects = exported.iter().map(|s| s.to_string()).collect(); String::from_utf8(compile_module(&m, entry_opts()).unwrap()).expect("LLVM IR should be UTF-8")