diff --git a/crates/perry-hir/src/destructuring/var_decl.rs b/crates/perry-hir/src/destructuring/var_decl.rs index f23a8e9c6d..072b1b5e5e 100644 --- a/crates/perry-hir/src/destructuring/var_decl.rs +++ b/crates/perry-hir/src/destructuring/var_decl.rs @@ -333,12 +333,29 @@ pub(crate) fn lower_var_decl_with_destructuring( // every property of `x` is a data field rather than an accessor — // `is_closed_shape` rejects getters and setters — and the // loop-invariant property hoist refuses to fire without it. + // + // A `const` alias of such a binding inherits the proof: neither + // name can ever be rebound, so both always denote the object the + // literal created. This is what lets the hoist reach receivers + // read through an alias, including one captured by a closure — + // the capture keeps the same `LocalId`, so no extra plumbing is + // needed once the alias is recorded. if !mutable { - if let Some(Expr::New { class_name, .. }) = init.as_ref() { - if class_name.starts_with("__AnonShape_") { + match init.as_ref() { + Some(Expr::New { class_name, .. }) + if class_name.starts_with("__AnonShape_") => + { ctx.closed_shape_literal_locals .insert(id, class_name.clone()); } + Some(Expr::LocalGet(source)) => { + if let Some(class_name) = + ctx.closed_shape_literal_locals.get(source).cloned() + { + ctx.closed_shape_literal_locals.insert(id, class_name); + } + } + _ => {} } } result.push(Stmt::Let { diff --git a/crates/perry-hir/src/lower/property_array_hoist.rs b/crates/perry-hir/src/lower/property_array_hoist.rs index c818a29b2d..c0a9a9e1ca 100644 --- a/crates/perry-hir/src/lower/property_array_hoist.rs +++ b/crates/perry-hir/src/lower/property_array_hoist.rs @@ -200,7 +200,39 @@ fn stmt_is_hoist_safe(stmt: &Stmt, recv_id: u32) -> bool { .as_ref() .is_none_or(|b| b.iter().all(|s| stmt_is_hoist_safe(s, recv_id))) } + // Nested loops are the common case this pass exists for — `m.rows[i]` + // in an outer loop with an inner loop over the row — so recurse rather + // than refuse. A nested loop is safe on exactly the same terms: it may + // not rebind the receiver and may not call anything. + Stmt::While { condition, body } | Stmt::DoWhile { body, condition } => { + expr_is_hoist_safe(condition, recv_id) + && body.iter().all(|s| stmt_is_hoist_safe(s, recv_id)) + } + Stmt::For { + init, + condition, + update, + body, + } => { + init.as_ref().is_none_or(|s| stmt_is_hoist_safe(s, recv_id)) + && condition + .as_ref() + .is_none_or(|e| expr_is_hoist_safe(e, recv_id)) + && update + .as_ref() + .is_none_or(|e| expr_is_hoist_safe(e, recv_id)) + && body.iter().all(|s| stmt_is_hoist_safe(s, recv_id)) + } + Stmt::Labeled { body, .. } => stmt_is_hoist_safe(body, recv_id), + // Leaving the loop early is fine: the hoisted `Let` is evaluated + // before the loop either way, and the property read it replaces was + // never reached on this path. + Stmt::Return(value) => value + .as_ref() + .is_none_or(|e| expr_is_hoist_safe(e, recv_id)), + Stmt::Throw(value) => expr_is_hoist_safe(value, recv_id), Stmt::Break | Stmt::Continue => true, + Stmt::LabeledBreak(_) | Stmt::LabeledContinue(_) => true, _ => false, } } @@ -265,6 +297,35 @@ fn stmt_reads_property(stmt: &Stmt, recv_id: u32, property: &str) -> bool { .as_ref() .is_some_and(|b| b.iter().any(|s| stmt_reads_property(s, recv_id, property))) } + Stmt::While { condition, body } | Stmt::DoWhile { body, condition } => { + expr_reads_property(condition, recv_id, property) + || body + .iter() + .any(|s| stmt_reads_property(s, recv_id, property)) + } + Stmt::For { + init, + condition, + update, + body, + } => { + init.as_ref() + .is_some_and(|s| stmt_reads_property(s, recv_id, property)) + || condition + .as_ref() + .is_some_and(|e| expr_reads_property(e, recv_id, property)) + || update + .as_ref() + .is_some_and(|e| expr_reads_property(e, recv_id, property)) + || body + .iter() + .any(|s| stmt_reads_property(s, recv_id, property)) + } + Stmt::Labeled { body, .. } => stmt_reads_property(body, recv_id, property), + Stmt::Return(value) => value + .as_ref() + .is_some_and(|e| expr_reads_property(e, recv_id, property)), + Stmt::Throw(value) => expr_reads_property(value, recv_id, property), _ => false, } } @@ -345,10 +406,54 @@ fn rewrite_stmt(stmt: &Stmt, recv_id: u32, property: &str, hoist_id: u32) -> Stm .collect() }), }, + // These mirror the arms `stmt_is_hoist_safe` admits. Anything it + // admits must be rewritten here too, or the read it vouched for keeps + // the per-iteration lookup. + Stmt::While { condition, body } => Stmt::While { + condition: rewrite_expr(condition, recv_id, property, hoist_id), + body: rewrite_block(body, recv_id, property, hoist_id), + }, + Stmt::DoWhile { body, condition } => Stmt::DoWhile { + body: rewrite_block(body, recv_id, property, hoist_id), + condition: rewrite_expr(condition, recv_id, property, hoist_id), + }, + Stmt::For { + init, + condition, + update, + body, + } => Stmt::For { + init: init + .as_ref() + .map(|s| Box::new(rewrite_stmt(s, recv_id, property, hoist_id))), + condition: condition + .as_ref() + .map(|e| rewrite_expr(e, recv_id, property, hoist_id)), + update: update + .as_ref() + .map(|e| rewrite_expr(e, recv_id, property, hoist_id)), + body: rewrite_block(body, recv_id, property, hoist_id), + }, + Stmt::Labeled { label, body } => Stmt::Labeled { + label: label.clone(), + body: Box::new(rewrite_stmt(body, recv_id, property, hoist_id)), + }, + Stmt::Return(value) => Stmt::Return( + value + .as_ref() + .map(|e| rewrite_expr(e, recv_id, property, hoist_id)), + ), + Stmt::Throw(value) => Stmt::Throw(rewrite_expr(value, recv_id, property, hoist_id)), other => other.clone(), } } +fn rewrite_block(body: &[Stmt], recv_id: u32, property: &str, hoist_id: u32) -> Vec { + body.iter() + .map(|s| rewrite_stmt(s, recv_id, property, hoist_id)) + .collect() +} + fn rewrite_expr(expr: &Expr, recv_id: u32, property: &str, hoist_id: u32) -> Expr { if is_target_property(expr, recv_id, property) { return Expr::LocalGet(hoist_id); diff --git a/crates/perry/tests/loop_property_array_hoist.rs b/crates/perry/tests/loop_property_array_hoist.rs index 9a14dce21d..c63846c671 100644 --- a/crates/perry/tests/loop_property_array_hoist.rs +++ b/crates/perry/tests/loop_property_array_hoist.rs @@ -204,3 +204,87 @@ fn handles_nested_loops_and_string_elements() { "10 abc 0", ); } + +#[test] +fn hoists_through_a_const_alias_and_a_capture() { + // `const h = holder` cannot be rebound and neither can `holder`, so the + // alias inherits the data-field proof. The closure capture keeps the same + // LocalId, so the same rewrite reaches a receiver read inside an arrow. + assert_same_with_and_without_hoist( + "alias and capture", + r#" + const holder = { arr: [1, 2, 3, 4], n: 4 }; + function aliased(): number { + const h = holder; + let s = 0; + for (let i = 0; i < h.arr.length; i++) s += h.arr[i]; + return s; + } + function captured(): number { + const h = holder; + const f = (): number => { + let s = 0; + for (let i = 0; i < h.arr.length; i++) s += h.arr[i]; + return s; + }; + return f(); + } + console.log(aliased() + " " + captured()); + "#, + "10 10", + ); +} + +#[test] +fn refuses_an_alias_of_a_reassignable_binding() { + // `let base` is not in the registry, so the alias inherits nothing and the + // loop keeps its per-iteration lookup. Pinned because the alias rule would + // be unsound if it ever followed a mutable source. + assert_same_with_and_without_hoist( + "mutable source", + r#" + let base: any = { get arr() { return [1, 2]; } }; + const h = base; + let s = 0; + for (let i = 0; i < h.arr.length; i++) s += h.arr[i]; + console.log(s); + "#, + "3", + ); +} + +#[test] +fn hoists_an_outer_loop_containing_a_nested_loop() { + assert_same_with_and_without_hoist( + "nested loop hoisted", + r#" + const g = { cells: [1, 2, 3, 4], n: 4 }; + let s = 0; + for (let r = 0; r < 3; r++) { + for (let i = 0; i < g.cells.length; i++) { + for (let k = 0; k < 2; k++) s += g.cells[i]; + } + } + console.log(s); + "#, + "60", + ); +} + +#[test] +fn hoists_a_loop_that_returns_early() { + assert_same_with_and_without_hoist( + "early return", + r#" + const h = { arr: [3, 7, 11, 15], n: 4 }; + function firstOver(limit: number): number { + for (let i = 0; i < h.arr.length; i++) { + if (h.arr[i] > limit) return h.arr[i]; + } + return -1; + } + console.log(firstOver(8) + " " + firstOver(100)); + "#, + "11 -1", + ); +}