From bbff0e4389fdd83416bb374917c3aa3c026e6134 Mon Sep 17 00:00:00 2001 From: TheHypnoo Date: Sat, 25 Jul 2026 18:20:41 +0200 Subject: [PATCH 1/3] fix(test): typed-feedback entry-return ordering assertion compared unrelated sites typed_feedback_trace_dump_runs_before_entry_return was red on main. Codegen is correct: add_pre_return_void_call splices the trace dump before every ret in main. The test compared rfind of the dump call against rfind of "ret i32 0" over the whole module. main has two returns, and the host-return early exit is the one that returns a literal i32 0, so the two searches landed on unrelated sites. Slice the IR to main's body and assert every ret is immediately preceded by the dump call. Deleting the add_pre_return_void_call in entry.rs now fails the test. --- ...40-typed-feedback-entry-return-ordering.md | 20 +++++++++ crates/perry-codegen/tests/typed_feedback.rs | 43 ++++++++++++++++--- 2 files changed, 58 insertions(+), 5 deletions(-) create mode 100644 changelog.d/6840-typed-feedback-entry-return-ordering.md diff --git a/changelog.d/6840-typed-feedback-entry-return-ordering.md b/changelog.d/6840-typed-feedback-entry-return-ordering.md new file mode 100644 index 0000000000..efa538513c --- /dev/null +++ b/changelog.d/6840-typed-feedback-entry-return-ordering.md @@ -0,0 +1,20 @@ +### Fixed + +- **`typed_feedback_trace_dump_runs_before_entry_return` has been red on `main`.** + Codegen was never wrong: `add_pre_return_void_call` splices + `js_typed_feedback_maybe_dump_trace()` in front of every `ret` in `main`, and + it still does. The test was measuring the wrong thing. It compared + `ir.rfind("call void @js_typed_feedback_maybe_dump_trace()")` against + `ir.rfind("ret i32 0")` over the whole module text. `main` has more than one + return: the host-return early exit returns a literal `i32 0`, the event-loop + exit returns the pending exit code. So the two `rfind`s landed on unrelated + sites — the last dump sat after the first return, and the assertion failed + while the property it names held. + + The test now slices the IR to `main`'s body and checks every return site: each + `ret` must be immediately preceded by the dump call. Removing the + `add_pre_return_void_call` in `crates/perry-codegen/src/codegen/entry.rs` + makes it fail, which the old shape could not guarantee. + + Integration suites under `crates/*/tests/` don't run per-PR (#5960), which is + why this sat red. diff --git a/crates/perry-codegen/tests/typed_feedback.rs b/crates/perry-codegen/tests/typed_feedback.rs index a0587c317a..c577b9d4ef 100644 --- a/crates/perry-codegen/tests/typed_feedback.rs +++ b/crates/perry-codegen/tests/typed_feedback.rs @@ -217,11 +217,44 @@ fn typed_feedback_trace_dump_runs_before_entry_return() { )); assert!(ir.contains("declare void @js_typed_feedback_maybe_dump_trace()")); - let dump_pos = ir - .rfind("call void @js_typed_feedback_maybe_dump_trace()") - .expect("entry should call typed-feedback trace dump"); - let ret_pos = ir.rfind("ret i32 0").expect("entry should return i32 0"); - assert!(dump_pos < ret_pos); + + // Scope the ordering check to `main`'s body, and check every return site. + // `main` has more than one `ret` (the host-return early exit returns + // `i32 0`, the event-loop exit returns the pending exit code), and later + // functions in the module carry their own returns. A positional + // `rfind(dump) < rfind("ret i32 0")` over the whole module text therefore + // compares two unrelated sites and proves nothing about the ordering. + let body = entry_fn_body(&ir); + let mut returns = 0; + let mut prev = ""; + for line in body.lines() { + let trimmed = line.trim(); + if trimmed.starts_with("ret ") { + returns += 1; + assert_eq!( + prev, "call void @js_typed_feedback_maybe_dump_trace()", + "every `main` return must dump the typed-feedback trace first, \ + else a PERRY_TYPED_FEEDBACK trace is truncated at exit; \ + found `{trimmed}` preceded by `{prev}`" + ); + } + prev = trimmed; + } + assert!(returns > 0, "expected at least one return in `main`"); +} + +/// Body of the entry `main` function, without its `define`/`}` lines. +fn entry_fn_body(ir: &str) -> &str { + let header = "define i32 @main() {\n"; + let start = ir + .find(header) + .expect("entry module should define `i32 @main()`") + + header.len(); + let rest = &ir[start..]; + let end = rest + .find("\n}\n") + .expect("`main` should be terminated by a closing brace"); + &rest[..end] } #[test] From 815f3700ba4f85d196f25ea5066b94e9f819afe0 Mon Sep 17 00:00:00 2001 From: TheHypnoo Date: Wed, 29 Jul 2026 23:16:07 +0200 Subject: [PATCH 2/3] docs: clarify scoped integration test selection --- changelog.d/6840-typed-feedback-entry-return-ordering.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/changelog.d/6840-typed-feedback-entry-return-ordering.md b/changelog.d/6840-typed-feedback-entry-return-ordering.md index efa538513c..c137b3c33c 100644 --- a/changelog.d/6840-typed-feedback-entry-return-ordering.md +++ b/changelog.d/6840-typed-feedback-entry-return-ordering.md @@ -16,5 +16,5 @@ `add_pre_return_void_call` in `crates/perry-codegen/src/codegen/entry.rs` makes it fail, which the old shape could not guarantee. - Integration suites under `crates/*/tests/` don't run per-PR (#5960), which is - why this sat red. + The PR workflow now uses diff-based selection in `e2e-scoped`, so changing + this integration test pulls its suite into the per-PR run. From 55f21fc5078c448aa7293ec27e62bc951579d7da Mon Sep 17 00:00:00 2001 From: TheHypnoo Date: Thu, 30 Jul 2026 00:01:03 +0200 Subject: [PATCH 3/3] style(codegen): format loop purity match arm --- crates/perry-codegen/src/loop_purity.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/perry-codegen/src/loop_purity.rs b/crates/perry-codegen/src/loop_purity.rs index aeace93d48..364d34cb3b 100644 --- a/crates/perry-codegen/src/loop_purity.rs +++ b/crates/perry-codegen/src/loop_purity.rs @@ -121,9 +121,7 @@ fn expr_alloc_free(e: &Expr) -> bool { // Element READS never allocate — they return an existing element / a // number. Recurse so the object and index are themselves alloc-free. Expr::IndexGet { object, index } => expr_alloc_free(object) && expr_alloc_free(index), - Expr::BufferIndexGet { buffer, index } => { - expr_alloc_free(buffer) && expr_alloc_free(index) - } + Expr::BufferIndexGet { buffer, index } => expr_alloc_free(buffer) && expr_alloc_free(index), Expr::Uint8ArrayGet { array, index } => expr_alloc_free(array) && expr_alloc_free(index), // `arr[i]++` / `--`: read-modify-write of an existing numeric slot, no // growth, no allocation.