From ead7ff5b84c35633a93c5e99c0ef04cae4be5bfd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 17 Sep 2026 10:18:07 +0000 Subject: [PATCH 1/2] fix(transform): keep statement-position inlines from returning out of the caller A call used as a statement (`f(x);`) whose inlined callee ended in an `if` containing `return` had that return spliced into the caller: the statement arm of inline_calls_in_stmts only looked for nested returns in take(len - 1) and only rewrote a bare trailing Return. The caller then returned the callee's value (decimal.js pow() returned true), and at module top level the stray ret produced invalid LLVM IR. The statement arm now removes every return structurally (discard_inlined_returns): `return e` becomes `e;`, and statements after an `if` that returns move into the branch that falls through to them. No loop wrapper is added. It declines (keeps the call) instead of duplicating or dropping statements, or when a return sits under a loop, switch, try or label. Two siblings of the same blind spot are fixed as well: the declined-inline fallback replaced the call statement with the setup hoisted out of its arguments (deleting the call), and the void-method expression inliner spliced statements that follow `return;`. --- .../src/inline/call_inliner.rs | 76 +-- .../src/inline/discarded_result.rs | 507 ++++++++++++++++++ crates/perry-transform/src/inline/mod.rs | 1 + .../test_gap_10416_inliner_stmt_return.ts | 417 ++++++++++++++ 4 files changed, 940 insertions(+), 61 deletions(-) create mode 100644 crates/perry-transform/src/inline/discarded_result.rs create mode 100644 test-files/test_gap_10416_inliner_stmt_return.ts diff --git a/crates/perry-transform/src/inline/call_inliner.rs b/crates/perry-transform/src/inline/call_inliner.rs index a03640ee69..759d88e212 100644 --- a/crates/perry-transform/src/inline/call_inliner.rs +++ b/crates/perry-transform/src/inline/call_inliner.rs @@ -2,6 +2,7 @@ use perry_hir::types::{FuncId, LocalId, Type}; use perry_hir::{Expr, Function, Param, Stmt}; use std::collections::{HashMap, HashSet}; +use super::discarded_result::discard_inlined_returns; use super::*; pub fn stmt_contains_return(s: &Stmt) -> bool { @@ -378,7 +379,13 @@ pub fn inline_calls_in_stmts( match &mut stmts[i] { Stmt::Expr(expr) => { - if let Some((mut inlined_stmts, _result_expr)) = try_inline_call( + // Statement position: the result is discarded. The spliced + // body must not keep a `return` — it would return from the + // CALLER (#10416), including from an `if` that is the callee's + // last statement. When `discard_inlined_returns` declines, + // the call stays and only calls nested in its arguments are + // inlined, spliced BEFORE the (kept) call statement. + let inlined = try_inline_call( expr, func_candidates, method_candidates, @@ -387,65 +394,10 @@ pub fn inline_calls_in_stmts( next_local_id, enclosing_class, class_field_types, - ) { - // When inlining into Stmt::Expr context (result discarded), - // convert Stmt::Return(Some(expr)) to Stmt::Expr(expr) and - // remove Stmt::Return(None). This prevents emitting a - // `ret` terminator mid-block (e.g., inside a for loop body). - // Only do this if returns are in safe positions (trailing). - let has_nested_return = inlined_stmts - .iter() - .take(inlined_stmts.len().saturating_sub(1)) - .any(|s| { - fn stmt_has_return(s: &Stmt) -> bool { - match s { - Stmt::Return(_) => true, - Stmt::If { - then_branch, - else_branch, - .. - } => { - then_branch.iter().any(stmt_has_return) - || else_branch - .as_ref() - .is_some_and(|eb| eb.iter().any(stmt_has_return)) - } - _ => false, - } - } - stmt_has_return(s) - }); - if has_nested_return { - // Can't safely convert early returns; skip inlining - let hoisted = inline_calls_in_expr( - expr, - func_candidates, - method_candidates, - local_types, - exact_receiver_facts, - next_local_id, - enclosing_class, - class_field_types, - ); - if !hoisted.is_empty() { - new_stmts = Some(hoisted); - } - } else { - // Convert trailing return to expression (discard result) - if let Some(last) = inlined_stmts.last_mut() { - match last { - Stmt::Return(Some(ret_expr)) => { - let e = std::mem::replace(ret_expr, Expr::Undefined); - *last = Stmt::Expr(e); - } - Stmt::Return(None) => { - inlined_stmts.pop(); - } - _ => {} - } - } - new_stmts = Some(inlined_stmts); - } + ) + .and_then(|(inlined_stmts, _result_expr)| discard_inlined_returns(inlined_stmts)); + if inlined.is_some() { + new_stmts = inlined; } else { let hoisted = inline_calls_in_expr( expr, @@ -1909,7 +1861,9 @@ pub fn try_inline_simple_call( for stmt in &method_candidate.func.body { match stmt { - Stmt::Return(None) => {} + // Anything after `return;` is dead: splicing + // it would run it (#10416's sibling shape). + Stmt::Return(None) => break, Stmt::Expr(e) => { let mut expr = e.clone(); substitute_locals(&mut expr, &shared_param_map, next_local_id); diff --git a/crates/perry-transform/src/inline/discarded_result.rs b/crates/perry-transform/src/inline/discarded_result.rs new file mode 100644 index 0000000000..a54f13cf69 --- /dev/null +++ b/crates/perry-transform/src/inline/discarded_result.rs @@ -0,0 +1,507 @@ +//! Inline boundary for a call whose result is discarded (`f(x);`). +//! +//! A statement-position inline splices the callee body into the caller's +//! statement list, so any `return` left in that body returns from the CALLER +//! (#10416: decimal.js `intPow` returned the `true` of its `truncate(…);` +//! helper, and at module top level the stray `ret double` did not even match +//! `main`'s `i32` result type). The value arm (`let r = f(x)`) solves the same +//! problem with a `do { … } while (false)` wrapper whose converted returns +//! `break` out after writing `r`. A discarded result has no variable to +//! write, so the returns are removed structurally instead, without adding a +//! loop to the caller (a loop brings a GC back-edge poll with it and changes +//! the shape of any hot loop the call sits in): +//! +//! * `return e;` becomes `e;` (the value is dropped, its evaluation is not), +//! and `return;` becomes nothing; +//! * the statements after an `if` that returns move into the branch that can +//! still fall through to them: `if (c) { a; return v; } rest` becomes +//! `if (c) { a; v; } else { rest }`. +//! +//! The rewrite adds no loop or label, so a `break`/`continue` in the callee +//! keeps its target. It declines (`None`: keep the call) rather than +//! duplicate or drop statements: when both branches of such an `if` can fall +//! through to a non-empty continuation, when statements follow a `return` or +//! an `if` whose branches all exit, and when a `return` sits under anything +//! other than `if` nesting (loop, `switch`, `try`, label). +//! `has_simple_control_flow` keeps most of those shapes out of inline +//! candidates today; the boundary does not rely on it. + +use perry_hir::Stmt; + +use super::call_inliner::stmt_contains_return; + +/// Remove every `return` from an inlined callee body whose result is +/// discarded, or `None` when that cannot be done without duplicating or +/// dropping statements (see the module docs). +pub(crate) fn discard_inlined_returns(stmts: Vec) -> Option> { + let mut out = Vec::with_capacity(stmts.len()); + let mut stmts = stmts.into_iter(); + while let Some(stmt) = stmts.next() { + match stmt { + Stmt::Return(value) => { + if !stmts.as_slice().is_empty() { + return None; + } + out.extend(value.map(Stmt::Expr)); + return Some(out); + } + Stmt::If { + condition, + then_branch, + else_branch, + } if then_branch.iter().any(stmt_contains_return) + || else_branch + .as_ref() + .is_some_and(|branch| branch.iter().any(stmt_contains_return)) => + { + let continuation: Vec = stmts.collect(); + let mut then_branch = then_branch; + let mut else_branch = else_branch.unwrap_or_default(); + if !continuation.is_empty() { + match ( + can_complete_normally(&then_branch), + can_complete_normally(&else_branch), + ) { + (true, false) => then_branch.extend(continuation), + (false, true) => else_branch.extend(continuation), + // Both fall through: the continuation would have to be + // duplicated. Neither does: it is unreachable, and + // dropping it is not this rewrite's decision. + (true, true) | (false, false) => return None, + } + } + let then_branch = discard_inlined_returns(then_branch)?; + let else_branch = discard_inlined_returns(else_branch)?; + out.push(Stmt::If { + condition, + then_branch, + else_branch: (!else_branch.is_empty()).then_some(else_branch), + }); + return Some(out); + } + other if stmt_contains_return(&other) => return None, + other => out.push(other), + } + } + Some(out) +} + +/// `false` only when every path through `stmts` ends in `return` or `throw`. +/// Answering `true` for a list that cannot fall through is safe (the +/// continuation lands after an exit and the rewrite then declines, or it is +/// dead code after a `throw`); answering `false` for one that can would skip +/// the continuation on that path. +fn can_complete_normally(stmts: &[Stmt]) -> bool { + !stmts.iter().any(|stmt| match stmt { + Stmt::Return(_) | Stmt::Throw(_) => true, + Stmt::If { + then_branch, + else_branch: Some(else_branch), + .. + } => !can_complete_normally(then_branch) && !can_complete_normally(else_branch), + _ => false, + }) +} + +#[cfg(test)] +mod tests { + use super::*; + use perry_hir::Expr; + + fn ret(value: i64) -> Stmt { + Stmt::Return(Some(Expr::Integer(value))) + } + + fn effect(value: i64) -> Stmt { + Stmt::Expr(Expr::Integer(value)) + } + + fn cond(value: i64) -> Expr { + Expr::LocalGet(value as u32) + } + + fn if_stmt(condition: i64, then_branch: Vec, else_branch: Option>) -> Stmt { + Stmt::If { + condition: cond(condition), + then_branch, + else_branch, + } + } + + fn contains_return(stmts: &[Stmt]) -> bool { + stmts.iter().any(stmt_contains_return) + } + + fn dump(stmts: &[Stmt]) -> String { + format!("{stmts:?}") + } + + #[test] + fn trailing_bare_return_becomes_its_value() { + let out = discard_inlined_returns(vec![effect(1), ret(2)]).unwrap(); + assert_eq!(dump(&out), dump(&[effect(1), effect(2)])); + let out = discard_inlined_returns(vec![effect(1), Stmt::Return(None)]).unwrap(); + assert_eq!(dump(&out), dump(&[effect(1)])); + } + + /// #10416's shape: the callee's LAST statement is an `if` that returns. + /// The pre-fix statement arm only inspected `take(len - 1)` and only + /// rewrote a bare trailing `Return`, so this `return` reached the caller. + #[test] + fn trailing_if_return_loses_its_return() { + let body = vec![effect(1), if_stmt(7, vec![effect(2), ret(3)], None)]; + let out = discard_inlined_returns(body).unwrap(); + assert!(!contains_return(&out), "{}", dump(&out)); + assert_eq!( + dump(&out), + dump(&[effect(1), if_stmt(7, vec![effect(2), effect(3)], None)]) + ); + } + + #[test] + fn guard_moves_the_continuation_into_the_falling_branch() { + // if (c) { return 1; } a; return 2; => if (c) { 1; } else { a; 2; } + let body = vec![if_stmt(7, vec![ret(1)], None), effect(5), ret(2)]; + let out = discard_inlined_returns(body).unwrap(); + assert_eq!( + dump(&out), + dump(&[if_stmt( + 7, + vec![effect(1)], + Some(vec![effect(5), effect(2)]) + )]) + ); + + // if (c) { a; } else { return 1; } b; => if (c) { a; b; } else { 1; } + let body = vec![if_stmt(7, vec![effect(4)], Some(vec![ret(1)])), effect(5)]; + let out = discard_inlined_returns(body).unwrap(); + assert_eq!( + dump(&out), + dump(&[if_stmt( + 7, + vec![effect(4), effect(5)], + Some(vec![effect(1)]) + )]) + ); + } + + #[test] + fn both_branches_returning_and_nested_guards() { + let out = + discard_inlined_returns(vec![if_stmt(7, vec![ret(1)], Some(vec![ret(2)]))]).unwrap(); + assert_eq!( + dump(&out), + dump(&[if_stmt(7, vec![effect(1)], Some(vec![effect(2)]))]) + ); + + // if (a) { if (b) return 1; x; } else { throw } y; + let body = vec![ + if_stmt( + 7, + vec![if_stmt(8, vec![ret(1)], None), effect(3)], + Some(vec![Stmt::Throw(Expr::Integer(9))]), + ), + effect(4), + ]; + let out = discard_inlined_returns(body).unwrap(); + assert!(!contains_return(&out), "{}", dump(&out)); + assert_eq!( + dump(&out), + dump(&[if_stmt( + 7, + vec![if_stmt( + 8, + vec![effect(1)], + Some(vec![effect(3), effect(4)]) + )], + Some(vec![Stmt::Throw(Expr::Integer(9))]), + )]) + ); + } + + #[test] + fn declines_instead_of_duplicating_or_dropping_statements() { + // Both branches can fall through to `rest`. + let body = vec![ + if_stmt(7, vec![if_stmt(8, vec![ret(1)], None), effect(2)], None), + effect(3), + ]; + assert!(discard_inlined_returns(body).is_none()); + + // Statements after an unconditional return / an if whose branches all exit. + assert!(discard_inlined_returns(vec![ret(1), effect(2)]).is_none()); + let body = vec![if_stmt(7, vec![ret(1)], Some(vec![ret(2)])), effect(3)]; + assert!(discard_inlined_returns(body).is_none()); + } + + #[test] + fn declines_returns_outside_if_nesting() { + let in_loop = Stmt::While { + condition: cond(7), + body: vec![if_stmt(8, vec![ret(1)], None), Stmt::Break], + }; + assert!(discard_inlined_returns(vec![in_loop]).is_none()); + let labeled = Stmt::Labeled { + label: "l".to_string(), + body: Box::new(if_stmt(8, vec![ret(1)], None)), + }; + assert!(discard_inlined_returns(vec![effect(1), labeled]).is_none()); + let nested = if_stmt( + 7, + vec![Stmt::DoWhile { + body: vec![ret(1)], + condition: Expr::Bool(false), + }], + None, + ); + assert!(discard_inlined_returns(vec![nested]).is_none()); + } + + #[test] + fn return_free_bodies_are_untouched() { + let body = vec![ + effect(1), + Stmt::While { + condition: cond(7), + body: vec![if_stmt(8, vec![Stmt::Break], None), Stmt::Continue], + }, + if_stmt(9, vec![Stmt::Throw(Expr::Integer(2))], None), + ]; + let expected = dump(&body); + assert_eq!(dump(&discard_inlined_returns(body).unwrap()), expected); + } + + // ---- through the inliner ------------------------------------------------ + + use crate::inline::call_inliner::try_inline_simple_call; + use crate::inline::{inline_functions, ExactReceiverFact, ExactReceiverFacts, MethodCandidate}; + use perry_hir::types::{FuncId, LocalId, Type}; + use perry_hir::{CompareOp, Function, Module, Param}; + use std::collections::HashMap; + + fn function(id: FuncId, params: Vec, body: Vec) -> Function { + Function { + id, + name: format!("f{id}"), + type_params: Vec::new(), + params: params + .into_iter() + .map(|id| Param { + id, + name: format!("p{id}"), + ty: Type::Number, + default: None, + decorators: Vec::new(), + is_rest: false, + arguments_object: None, + }) + .collect(), + return_type: Type::Any, + body, + is_async: false, + is_generator: false, + is_exported: false, + captures: Vec::new(), + decorators: Vec::new(), + was_plain_async: false, + was_unrolled: false, + is_strict: false, + } + } + + fn call(func: FuncId, args: Vec) -> Expr { + Expr::Call { + callee: Box::new(Expr::FuncRef(func)), + args, + type_args: Vec::new(), + byte_offset: 0, + } + } + + fn greater(local: LocalId, value: i64) -> Expr { + Expr::Compare { + op: CompareOp::Gt, + left: Box::new(Expr::LocalGet(local)), + right: Box::new(Expr::Integer(value)), + } + } + + fn inline(module: &mut Module) { + inline_functions( + module, + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + ); + } + + /// The #10416 repro, end to end: `early(5);` in a function that is not + /// itself a candidate (it has a loop) and in module init. + #[test] + fn statement_call_to_trailing_if_return_helper_no_longer_returns_from_caller() { + let early = function( + 1, + vec![1], + vec![if_stmt_on( + greater(1, 1), + vec![Stmt::Return(Some(Expr::String("early".into())))], + )], + ); + let caller = function( + 2, + Vec::new(), + vec![ + Stmt::Expr(call(1, vec![Expr::Integer(5)])), + Stmt::While { + condition: Expr::Bool(false), + body: Vec::new(), + }, + Stmt::Return(Some(Expr::String("after".into()))), + ], + ); + let mut module = Module::new("inline-10416.ts"); + module.functions = vec![early, caller]; + module.init = vec![Stmt::Expr(call(1, vec![Expr::Integer(5)]))]; + + inline(&mut module); + + let body = &module.functions[1].body; + assert!(!format!("{body:?}").contains("FuncRef(1)"), "{body:?}"); + assert_eq!(body.len(), 3, "{body:?}"); + assert!(!stmt_contains_return(&body[0]), "{body:?}"); + assert!(matches!(&body[2], Stmt::Return(Some(Expr::String(s))) if s == "after")); + let init = &module.init; + assert!(!format!("{init:?}").contains("FuncRef(1)"), "{init:?}"); + assert!(!contains_return(init), "{init:?}"); + } + + /// When the discarded-result rewrite declines, the call statement must + /// stay. The pre-fix fallback replaced it with the setup statements + /// hoisted out of its arguments, silently deleting the call. + #[test] + fn declined_statement_call_is_kept_after_its_hoisted_argument_setup() { + // f(x) { if (x > 0) { if (x > 5) return 1; 2; } 3; } + let f = function( + 1, + vec![1], + vec![ + if_stmt_on( + greater(1, 0), + vec![if_stmt_on(greater(1, 5), vec![ret(1)]), effect(2)], + ), + effect(3), + ], + ); + // g(y) { const z = y + 0; return z; } + let g = function( + 2, + vec![2], + vec![ + Stmt::Let { + id: 3, + name: "z".into(), + ty: Type::Number, + mutable: false, + init: Some(Expr::LocalGet(2)), + }, + Stmt::Return(Some(Expr::LocalGet(3))), + ], + ); + let caller = function( + 3, + vec![10], + vec![ + Stmt::Expr(call(1, vec![call(2, vec![Expr::LocalGet(10)])])), + Stmt::While { + condition: Expr::Bool(false), + body: Vec::new(), + }, + ], + ); + let mut module = Module::new("inline-10416-declined.ts"); + module.functions = vec![f, g, caller]; + + inline(&mut module); + + let body = &module.functions[2].body; + assert!(!format!("{body:?}").contains("FuncRef(2)"), "{body:?}"); + let call_at = body + .iter() + .position(|stmt| matches!(stmt, Stmt::Expr(Expr::Call { callee, .. }) if matches!(callee.as_ref(), Expr::FuncRef(1)))) + .unwrap_or_else(|| panic!("the call to f must survive: {body:?}")); + assert!( + matches!(&body[..call_at], [Stmt::Let { .. }]), + "g's setup must precede the kept call: {body:?}" + ); + } + + /// A void method whose `return;` precedes more statements must not have + /// those dead statements spliced into an expression-position call site. + #[test] + fn void_method_statements_after_return_are_not_spliced() { + let set_x = Stmt::Expr(Expr::PropertySet { + object: Box::new(Expr::This), + property: "x".into(), + value: Box::new(Expr::Integer(1)), + }); + let candidate = |body: Vec| { + let mut methods = HashMap::new(); + methods.insert( + ("Dead".to_string(), "m".to_string()), + MethodCandidate { + func: function(1, Vec::new(), body), + this_param_id: None, + method_lookup_safe: true, + required_extern_imports: Vec::new(), + }, + ); + methods + }; + let mut facts = ExactReceiverFacts::new(); + facts.insert( + 7, + ExactReceiverFact { + class_name: "Dead".into(), + }, + ); + let method_call = Expr::Call { + callee: Box::new(Expr::PropertyGet { + object: Box::new(Expr::LocalGet(7)), + property: "m".into(), + byte_offset: 0, + }), + args: Vec::new(), + type_args: Vec::new(), + byte_offset: 0, + }; + let try_inline = |methods: &HashMap<(String, String), MethodCandidate>| { + let mut next_local_id = 100; + try_inline_simple_call( + &method_call, + &HashMap::new(), + methods, + &HashMap::new(), + &facts, + &mut next_local_id, + None, + &HashMap::new(), + ) + }; + + let dead = candidate(vec![Stmt::Return(None), set_x.clone()]); + assert!(try_inline(&dead).is_none()); + let live = candidate(vec![set_x, Stmt::Return(None)]); + let (stmts, result) = try_inline(&live).expect("a trailing `return;` still inlines"); + assert_eq!(stmts.len(), 1); + assert!(matches!(result, Expr::Undefined)); + } + + fn if_stmt_on(condition: Expr, then_branch: Vec) -> Stmt { + Stmt::If { + condition, + then_branch, + else_branch: None, + } + } +} diff --git a/crates/perry-transform/src/inline/mod.rs b/crates/perry-transform/src/inline/mod.rs index c0d149c90d..219d36c3d2 100644 --- a/crates/perry-transform/src/inline/mod.rs +++ b/crates/perry-transform/src/inline/mod.rs @@ -12,6 +12,7 @@ mod call_inliner; mod clamp; mod closure_analysis; mod cross_module; +mod discarded_result; mod exact_receivers; mod factory_specialize; mod imul; diff --git a/test-files/test_gap_10416_inliner_stmt_return.ts b/test-files/test_gap_10416_inliner_stmt_return.ts new file mode 100644 index 0000000000..2157a0f743 --- /dev/null +++ b/test-files/test_gap_10416_inliner_stmt_return.ts @@ -0,0 +1,417 @@ +// #10416: a small helper called as a STATEMENT (result discarded) whose last +// statement is an `if` containing `return` had that `return` spliced into the +// caller by the HIR inliner, so the caller returned the helper's value +// (decimal.js `pow()` returned `true`), and at module top level the stray +// return produced invalid LLVM IR. Every caller below must return its own +// value; each helper's side effects must happen exactly as in Node. + +// ---- callees --------------------------------------------------------------- + +// `if (c) { return v; }` as the only statement. +function onlyGuard(x: number) { + if (x > 1) { + return "only"; + } +} + +// decimal.js `truncate`: `if` with a side effect and `return true` last. +function truncate(arr: number[], len: number) { + if (arr.length > len) { + arr.length = len; + return true; + } +} + +// `const y = …; if (y) return v;` +function constGuard(n: number) { + const y = n % 2 === 0; + if (y) return "even"; +} + +// if/else with a return in each branch. +function bothBranches(x: number) { + if (x > 1) { + return "a"; + } else { + return "b"; + } +} + +// Early return, then more work (return not in the last statement). +function guardThenWork(log: string[], x: number) { + if (x < 0) { + log.push("neg" + x); + return "neg"; + } + log.push("work" + x); + return "done"; +} + +// The else branch returns, the then branch falls through to the tail. +function elseReturns(log: string[], x: number) { + if (x % 2 === 0) { + log.push("even" + x); + } else { + return "odd"; + } + log.push("after" + x); +} + +// A conditional return nested in a branch that also falls through. +function nestedFallthrough(log: string[], x: number) { + if (x > 0) { + if (x > 5) { + return "big"; + } + log.push("small" + x); + } + log.push("tail" + x); +} + +// `return;` with no value. +function voidGuard(log: string[], x: number) { + if (x === 3) return; + log.push("v" + x); +} + +// Controls that were already correct. +function guardThenReturn(x: number) { + if (x > 1) { + return "v"; + } + return "w"; +} + +function guardThenThrow(x: number) { + if (x > 1) { + return "v"; + } + throw new Error("small " + x); +} + +function twice(y: number) { + const z = y * 2; + return z + 1; +} + +// ---- call statements inside loops ------------------------------------------ +// Loop bounds come from parameters so the loops stay loops. + +function inFor(n: number): string { + const arr = [1, 2, 3, 4, 5]; + for (let i = 0; i < n; i++) { + truncate(arr, 4 - i); + onlyGuard(i + 1); + } + return "inFor:" + arr.join(","); +} + +function inWhile(n: number): string { + const log: string[] = []; + let i = 0; + while (i < n) { + constGuard(i); + bothBranches(i); + elseReturns(log, i); + i++; + } + return "inWhile:" + i + ":" + log.join(","); +} + +function inDoWhile(n: number): string { + const log: string[] = []; + let i = -2; + do { + guardThenWork(log, i); + voidGuard(log, i + 5); + constGuard(i); + i++; + } while (i < n); + return "inDoWhile:" + log.join(","); +} + +// decimal.js `intPow` shape: `truncate(…)` as a statement inside `for (;;)`. +function intPowLike(k: number): string { + const digits = [9, 9, 9, 9, 9, 9, 9, 9]; + let n = 13; + let rounds = 0; + for (;;) { + rounds++; + if (n % 2) { + truncate(digits, k + rounds); + } + n = Math.floor(n / 2); + if (n === 0) break; + truncate(digits, k + 4); + } + return "intPow:" + rounds + ":" + digits.length; +} + +function nestedInLoop(n: number): string { + const log: string[] = []; + for (let i = 0; i < n; i += 3) { + nestedFallthrough(log, i); + // The call is kept here; the call nested in its argument is still inlined. + nestedFallthrough(log, twice(i)); + } + return "nested:" + log.join(","); +} + +// ---- other caller shapes --------------------------------------------------- + +// More than 10 statements, so the caller is not itself an inline candidate. +function manyStatements(): string { + const log: string[] = []; + log.push("s1"); + onlyGuard(5); + log.push("s2"); + bothBranches(0); + log.push("s3"); + constGuard(8); + log.push("s4"); + guardThenWork(log, -1); + guardThenWork(log, 1); + elseReturns(log, 2); + elseReturns(log, 3); + voidGuard(log, 3); + voidGuard(log, 4); + log.push("s5"); + return "many:" + log.join(","); +} + +class Store { + items: number[] = []; + log: string[] = []; + + add(v: number) { + if (v < 0) { + return false; + } + this.items.push(v); + } + + trimTo(n: number) { + if (this.items.length > n) { + this.items.length = n; + return true; + } + } + + withLoop(n: number): string { + for (let i = 0; i < n; i++) { + onlyGuard(5); + guardThenWork(this.log, i - 1); + } + return "withLoop:" + this.log.join(","); + } + + noLoop(): string { + bothBranches(3); + constGuard(2); + return "noLoop"; + } +} + +class Gate { + open = 0; + check(n: number) { + if (n > 2) { + this.open = n; + return "opened"; + } + } +} + +// Method callees on an exact receiver. +function methodCallee(n: number): string { + const g = new Gate(); + g.check(5); + const s = new Store(); + for (let i = -2; i < n; i++) { + s.add(i); + s.trimTo(3); + } + return "method:" + g.open + ":" + s.items.join(","); +} + +// Unreachable code after `return;` in a method used as a value. +class Dead { + x = 0; + m() { + return; + this.x = 1; + } +} + +function deadAfterReturn(): string { + const d = new Dead(); + console.log("dead m():", d.m()); + for (let i = 0; i < 1; i++) {} + return "dead:" + d.x; +} + +function inArrowInsideFunction(): string { + const arr = [1, 2, 3]; + const run = (): string => { + truncate(arr, 1); + constGuard(4); + return "arrow-in-fn:" + arr.length; + }; + return run(); +} + +const topArrow = (): string => { + constGuard(2); + onlyGuard(9); + bothBranches(9); + return "top-arrow"; +}; + +const topArrowLoop = (n: number): string => { + let hits = 0; + for (let i = 0; i < n; i++) { + onlyGuard(i); + hits++; + } + return "top-arrow-loop:" + hits; +}; + +// ---- early returns before more statements, in other caller contexts ----------- + +// `var` declared before the return is still visible (undefined) afterwards. +function varBeforeReturn(log: string[], x: number) { + if (x > 0) { + var y = "set" + x; + return; + } + log.push("y=" + y); +} + +function effectReturn(log: string[], c: boolean) { + if (c) return log.push("ret-effect"); + log.push("no-ret"); +} + +function inner(log: string[], x: number) { + if (x === 2) return "two"; + log.push("inner" + x); +} + +function outer(log: string[], x: number) { + if (x === 0) return "zero"; + inner(log, x); + log.push("outer" + x); +} + +function throwOrReturn(log: string[], x: number) { + if (x > 10) { + throw new Error("big" + x); + } else if (x > 5) { + return "mid"; + } + log.push("low" + x); +} + +function controlFlowCaller(n: number): string { + const log: string[] = []; + for (let i = -1; i < n; i++) { + varBeforeReturn(log, i); + effectReturn(log, i === 0); + outer(log, i); + switch (i) { + case 1: + inner(log, 5); + break; + default: + inner(log, 2); + } + try { + throwOrReturn(log, i * 6); + } catch (e) { + log.push((e as Error).message); + } finally { + inner(log, i + 100); + } + } + return "control-flow:" + log.join(","); +} + +function* generatorCaller(n: number) { + const log: string[] = []; + for (let i = 0; i < n; i++) { + outer(log, i); + yield log.length; + onlyGuard(i); + inner(log, i); + } + return log.join(","); +} + +async function asyncCaller(n: number): Promise { + const log: string[] = []; + for (let i = 0; i < n; i++) { + outer(log, i); + await Promise.resolve(i); + constGuard(i); + inner(log, i); + } + return "async:" + log.join(","); +} + +// ---- controls ---------------------------------------------------------------- + +function controls(): string { + const out: string[] = []; + let r: string | undefined = onlyGuard(5); + out.push(String(r)); + r = onlyGuard(0); + out.push(String(r)); + const b = bothBranches(0); + out.push(b); + out.push(guardThenReturn(5), guardThenReturn(0)); + guardThenReturn(5); + guardThenReturn(0); + out.push(guardThenThrow(5)); + guardThenThrow(5); + try { + guardThenThrow(0); + } catch (e) { + out.push((e as Error).message); + } + onlyGuard(0); + for (let i = 0; i < 1; i++) {} + return "controls:" + out.join(","); +} + +console.log(inFor(3)); +console.log(inWhile(4)); +console.log(inDoWhile(2)); +console.log(intPowLike(2)); +console.log(nestedInLoop(8)); +console.log(manyStatements()); +const store = new Store(); +console.log(store.withLoop(3)); +console.log(store.noLoop()); +console.log(methodCallee(6)); +console.log(deadAfterReturn()); +console.log(inArrowInsideFunction()); +console.log(topArrow()); +console.log(topArrowLoop(3)); +console.log(controls()); +console.log(controlFlowCaller(3)); +const gen = generatorCaller(3); +const yields: string[] = []; +let step = gen.next(); +while (!step.done) { + yields.push(String(step.value)); + step = gen.next(); +} +console.log("generator:" + yields.join(",") + ":" + step.value); +asyncCaller(3).then((s) => console.log(s)); + +// Module top level: must compile and keep running past each call. +onlyGuard(5); +bothBranches(0); +constGuard(4); +truncate([1, 2, 3], 1); +console.log("toplevel-after"); From 8b731092acc822194bb54d0bcc73904a1ad39e6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 17 Sep 2026 12:23:00 +0000 Subject: [PATCH 2/2] docs(changelog): fragment for #10530 --- changelog.d/10530-inliner-stmt-return.md | 41 ++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 changelog.d/10530-inliner-stmt-return.md diff --git a/changelog.d/10530-inliner-stmt-return.md b/changelog.d/10530-inliner-stmt-return.md new file mode 100644 index 0000000000..729e7b667a --- /dev/null +++ b/changelog.d/10530-inliner-stmt-return.md @@ -0,0 +1,41 @@ +### Fixed + +- A small helper called as a statement (`f(x);`, result discarded) whose + inlined body ended in an `if` containing `return` no longer returns out of + the caller (#10416). The HIR inliner spliced that `return` into the calling + function, so the caller returned the helper's value: decimal.js `intPow` + calls `truncate(x.d, k);` inside `for (;;)`, and `new Decimal(2).pow(100)` + was `true`. At module top level the stray `ret double` did not match + `main`'s `i32` result, and the module failed to compile. + + `inline_calls_in_stmts`'s statement arm checked for nested returns only in + `take(len - 1)` and rewrote the last statement only when it was a bare + `Return`, so a trailing `if` passed both checks unchanged. The arm now runs + the inlined body through `discard_inlined_returns` + (`crates/perry-transform/src/inline/discarded_result.rs`), which removes + every return structurally. `return e` becomes `e;`, and the statements after + an `if` that returns move into the branch that falls through to them. It + adds no `do { } while (false)` wrapper, so the caller's hot loop gains no + nested loop, GC back-edge poll or cleared receiver facts. It declines, which + keeps the call, rather than duplicate or drop statements, and whenever a + return sits under a loop, `switch`, `try` or label. + + Two siblings of the same blind spot are fixed too. When the arm declined to + inline, its fallback replaced the call statement with the setup hoisted out + of the call's arguments: `f(g(i));` lost the call to `f`. Separately, the + void-method expression inliner kept going past `return;`, so + `m() { return; this.x = 1; }` used as a value ran `this.x = 1`. + + Helpers with an early return before more statements + (`if (v < 0) return; acc[0] += v;`) were never inlined at statement call + sites before this change. They are now, as they already were for + `let r = f(x)`. In a 50M-iteration hot loop that is 40.5e9 → 10.6e9 + instructions. The #10416 shape itself measures the same as before when its + `if` never fires (6.31e9 on both), and `09_method_calls` and a decimal.js + arithmetic loop are unchanged within noise. + + Covered by `test_gap_10416_inliner_stmt_return` (statement calls in + `for`/`while`/`do-while`/`for (;;)`, a function with more than 10 + statements, class methods, arrows, module top level, generator and async + callers, with value-use controls) and by rewrite and inliner unit tests in + `discarded_result.rs`, of which the inliner-level ones fail on the old arm.