From 5f86350b96869dc6eee303670b8c41748cfa521e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 16 Sep 2026 08:42:08 +0200 Subject: [PATCH 1/2] fix(hir): fold a builder whose stores are separated from its `{}` (#10353) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `fold_builder_sequences` (#6812) only matched when the `o.k = v` assignments followed the `const o = {}` binding immediately, so a single ordinary declaration in between — the usual way initialisation code names its constants — dropped the whole sequence. The unfolded `{}` lowers to a 0-field `__AnonShape_…`, which denies `Ptr` containment (every key really is undeclared on a shape that declares nothing) and sends every store down `js_put_value_set`, re-interning and re-coercing the key per execution: 108,444,840 instructions against 1,400,471 for the same program with the constants written inline. The scan now skips up to 64 statements between an EMPTY literal and its first assignment, sinking the allocation below them. A statement qualifies only when moving the allocation past it is unobservable, which is the pair of conditions the value side already carries: it must not name the binding, and it must not be able to execute user code (a call can reach a hoisted `function peek() { return o; }` that names the binding without naming it in the statement, turning a successful read into a TDZ ReferenceError). Destructuring patterns, `enum`/`namespace` and populated literals are excluded; `type`/`interface` are erased and are skipped. Skipped statements keep their relative order and still run before every folded value. --- changelog.d/10355-builder-fold-gap.md | 37 ++ crates/perry-hir/src/lower/builder_fold.rs | 178 +++++++- crates/perry-hir/tests/builder_fold_gap.rs | 384 ++++++++++++++++++ .../perry/tests/builder_fold_gap_semantics.rs | 137 +++++++ 4 files changed, 716 insertions(+), 20 deletions(-) create mode 100644 changelog.d/10355-builder-fold-gap.md create mode 100644 crates/perry-hir/tests/builder_fold_gap.rs create mode 100644 crates/perry/tests/builder_fold_gap_semantics.rs diff --git a/changelog.d/10355-builder-fold-gap.md b/changelog.d/10355-builder-fold-gap.md new file mode 100644 index 0000000000..b6a597f2be --- /dev/null +++ b/changelog.d/10355-builder-fold-gap.md @@ -0,0 +1,37 @@ +Fixed a 75× property-store cliff on `const o = {}; const X = 1; o.a = X;` +(#10353). The straight-line builder fold (#6812) rewrites `const o = {}` +plus its following `o.k = v` assignments into the object literal they spell +out, which is what gives the object a closed anon shape, a shape-stamped +allocation and direct stores. It only matched when the assignments followed +the binding *immediately*, so a single ordinary declaration in between — the +usual way initialisation code names its constants — dropped the whole +sequence back onto the dynamic `js_put_value_set` path, where every store +re-interns and re-coerces the key and transitions the object's shape. The +same program with the value passed as a parameter, or with the constants +written inline, was 75× faster, which is what made the cliff look like a +property of the stored *value*. + +`fold_builder_sequences` now skips up to 64 statements between an **empty** +`{}` binding and its first assignment, sinking the allocation below them. A +statement is skippable only when moving the allocation past it is +unobservable, which is the pair of conditions the value side already carries +(`gap_stmt_is_hoistable`): it must not name the binding, and it must not be +able to execute user code — a call can reach a hoisted +`function peek() { return o; }` that names the binding without the statement +naming it, which would turn a successful read into a TDZ `ReferenceError`. +Destructuring patterns (getter-bearing property reads) and populated +literals are excluded; sinking `const o = { a: y }` below `const y = 1` +would hide a TDZ throw. Skipped statements keep their relative order and +still run before every folded value. + +Measured with `perf stat -e instructions:u` on x86_64, 2400 iterations +building a six-property object with `--no-auto-optimize`: 108,447,339 → +1,399,772 instructions (77×), matching the same program with the constants +written inline (1,401,872) or the value passed as a parameter (1,411,774). +Nothing that folded before folds differently — the gap is an additional +match, and a statement that fails the test leaves the original dynamic +writes exactly as they were: `benchmarks/object-write-6812` and the +`bench_*` corpus move by at most 0.006%, and a 12k-line file whose gaps +never reach an assignment (maximum pre-scan work, zero folds) costs 0.019% +more to compile. A file where the fold now applies compiles 51% cheaper, +because 1,200 dynamic store sites become 200 stamped allocations. diff --git a/crates/perry-hir/src/lower/builder_fold.rs b/crates/perry-hir/src/lower/builder_fold.rs index 936b51ee74..e91b777692 100644 --- a/crates/perry-hir/src/lower/builder_fold.rs +++ b/crates/perry-hir/src/lower/builder_fold.rs @@ -15,6 +15,21 @@ //! - The appended value expressions run in the same order at the same //! sequence points; only the allocation moves AFTER them, and a bare //! object allocation has no user-visible effects. +//! - #10353: the assignments need not follow the binding IMMEDIATELY. The +//! scan skips up to `MAX_FOLD_GAP_STMTS` statements in between when +//! moving the allocation below them is unobservable by the same argument +//! — `gap_stmt_is_hoistable` requires exactly what the value side already +//! requires: the statement must not name the binding, and it must not be +//! able to execute user code. Skipped statements keep their relative +//! order and still run before every appended value, so +//! `const o = {}; const X = 1; o.a = X;` folds to +//! `const X = 1; const o = { a: X };`. That gap is the ordinary shape of +//! initialisation code, and before #10353 it cost 75×: the unfolded form +//! leaves a 0-field anon shape that denies `Ptr` containment, so +//! every store takes the dynamic `PutValueSet` path. A gap is allowed only +//! for an EMPTY literal — sinking a populated one would move its own value +//! expressions below the skipped statements, and `const o = { a: y }; +//! const y = 1; o.b = 2;` must keep throwing on `y`'s TDZ. //! - Values must not reference the bound name (checked conservatively by //! symbol name anywhere in the value expression, ignoring shadowing), so //! no expression can observe the half-built object. @@ -47,6 +62,12 @@ use swc_ecma_visit::{Visit, VisitWith}; /// literal machinery's inline-slot benefits taper off anyway. const MAX_FOLDED_PROPS: usize = 64; +/// How many statements the scan may skip between the binding and its first +/// assignment (#10353). Real builders separate the two by a handful of +/// constant bindings at most; the cap keeps the forward scan O(n) over a +/// statement list instead of O(n²) on a long run of hoistable declarations. +const MAX_FOLD_GAP_STMTS: usize = 64; + /// Returns a folded clone when at least one builder sequence was folded; /// `None` means "nothing to do — lower the original". pub(crate) fn fold_builder_sequences(module: &ast::Module) -> Option { @@ -155,17 +176,25 @@ fn is_object_prototype_expr(expr: &ast::Expr) -> bool { /// Cheap read-only pre-scan: is any statement list anywhere (including /// function bodies nested in expressions) a `const/let/var x = {…}` -/// immediately followed by a static member assignment to the same name? -/// False positives only cost the clone; a false negative would skip a -/// fold, so the walk mirrors the mutating one's reach. +/// followed — across a hoistable gap (#10353) — by a static member +/// assignment to the same name? False positives only cost the clone; a +/// false negative would skip a fold, so the walk mirrors the mutating +/// one's reach, gap included. fn module_has_candidate(module: &ast::Module) -> bool { - for pair in module.body.windows(2) { - if let (ast::ModuleItem::Stmt(a), ast::ModuleItem::Stmt(b)) = (&pair[0], &pair[1]) { - if let (Some(name), _) = decl_object_binding(a) { - if assign_to_name_key(b, name.as_str()).is_some() { - return true; - } - } + for (i, item) in module.body.iter().enumerate() { + let ast::ModuleItem::Stmt(a) = item else { + continue; + }; + let (Some(name), _) = decl_object_binding(a) else { + continue; + }; + let item_stmt = |k: usize| match module.body.get(i + 1 + k) { + Some(ast::ModuleItem::Stmt(s)) => Some(s), + _ => None, + }; + let gap = fold_gap_len(name.as_str(), item_stmt); + if item_stmt(gap).is_some_and(|b| assign_to_name_key(b, name.as_str()).is_some()) { + return true; } } module.body.iter().any(|item| match item { @@ -177,11 +206,16 @@ fn module_has_candidate(module: &ast::Module) -> bool { } fn stmts_have_candidate(stmts: &[ast::Stmt]) -> bool { - for pair in stmts.windows(2) { - if let (Some(name), _) = decl_object_binding(&pair[0]) { - if assign_to_name_key(&pair[1], name.as_str()).is_some() { - return true; - } + for (i, s) in stmts.iter().enumerate() { + let (Some(name), _) = decl_object_binding(s) else { + continue; + }; + let gap = fold_gap_len(name.as_str(), |k| stmts.get(i + 1 + k)); + if stmts + .get(i + 1 + gap) + .is_some_and(|b| assign_to_name_key(b, name.as_str()).is_some()) + { + return true; } } stmts.iter().any(scan_stmt) @@ -368,10 +402,23 @@ fn fold_module_stmt_run(items: &mut [ast::ModuleItem], changed: &mut bool) { idx += 1; continue; } + // A gap is only skippable for an EMPTY literal: sinking a populated + // one would move its own value expressions below the skipped + // statements, and `const o = { a: y }; const y = 1; o.b = 2;` must + // keep throwing on `y`'s TDZ. + let gap = if existing.is_empty() { + fold_gap_len(&name_start, |k| match items.get(idx + 1 + k) { + Some(ast::ModuleItem::Stmt(s)) => Some(s), + _ => None, + }) + } else { + 0 + }; + let first = idx + 1 + gap; let mut keys = existing_keys(existing); let mut appended: Vec<(ast::PropName, Box)> = Vec::new(); let mut consumed = 0usize; - for follower in items[idx + 1..].iter() { + for follower in items[first..].iter() { let ast::ModuleItem::Stmt(fs) = follower else { break; }; @@ -392,16 +439,28 @@ fn fold_module_stmt_run(items: &mut [ast::ModuleItem], changed: &mut bool) { idx += 1; continue; } - // Apply: extend the literal, blank out the consumed statements. + // Apply: extend the literal, sink the declaration below the skipped + // statements so the appended values still evaluate after them, and + // blank out the consumed statements. if let ast::ModuleItem::Stmt(s) = &mut items[idx] { append_props(s, appended); } - for follower in items[idx + 1..idx + 1 + consumed].iter_mut() { + if gap > 0 { + items[idx..first].rotate_left(1); + } + for follower in items[first..first + consumed].iter_mut() { *follower = ast::ModuleItem::Stmt(ast::Stmt::Empty(ast::EmptyStmt { span: swc_common::DUMMY_SP, })); } *changed = true; + if gap > 0 { + // `items[idx]` is now the first skipped statement, which may open + // a builder of its own (`const a = {}; const b = {}; a.x = 1; + // b.y = 2;`). Re-examining it terminates: each fold blanks at + // least one assignment statement, and the run holds finitely many. + continue; + } idx += 1 + consumed; } } @@ -419,9 +478,16 @@ fn fold_stmts(stmts: &mut Vec, changed: &mut bool) { idx += 1; continue; }; + // Empty literals only — see `fold_module_stmt_run`. + let gap = if existing_len == 0 { + fold_gap_len(&name, |k| stmts.get(idx + 1 + k)) + } else { + 0 + }; + let first = idx + 1 + gap; let mut appended: Vec<(ast::PropName, Box)> = Vec::new(); let mut consumed = 0usize; - for follower in stmts[idx + 1..].iter() { + for follower in stmts[first..].iter() { let Some((key, value)) = assign_to_name_key(follower, &name) else { break; }; @@ -437,8 +503,20 @@ fn fold_stmts(stmts: &mut Vec, changed: &mut bool) { } if consumed > 0 { append_props(&mut stmts[idx], appended); - stmts.drain(idx + 1..idx + 1 + consumed); + // Sink the declaration below the skipped statements: the appended + // values evaluate where the literal now sits, so they must still + // run after everything that used to precede them. + if gap > 0 { + stmts[idx..first].rotate_left(1); + } + stmts.drain(first..first + consumed); *changed = true; + if gap > 0 { + // `stmts[idx]` is now the first skipped statement, which may + // open a builder of its own. Re-examining it terminates: each + // fold removes at least one statement from the list. + continue; + } } idx += 1; } @@ -500,6 +578,66 @@ fn assign_to_name_key<'a>( Some((key, &a.right)) } +/// How many statements between the `{ … }` binding and its first fold-able +/// assignment the scan may skip (#10353). +/// +/// `at(k)` yields the k-th follower of the binding, or `None` when the run +/// ends (a non-`Stmt` module item, or the end of the list). The walk stops at +/// the first statement that is an assignment to `name` — that is where the +/// fold proper takes over — and at the first statement the declaration may +/// not move below. +fn fold_gap_len<'a>(name: &str, at: impl Fn(usize) -> Option<&'a ast::Stmt>) -> usize { + let mut gap = 0usize; + while gap < MAX_FOLD_GAP_STMTS { + let Some(s) = at(gap) else { break }; + if assign_to_name_key(s, name).is_some() || !gap_stmt_is_hoistable(s, name) { + break; + } + gap += 1; + } + gap +} + +/// May the `{ … }` declaration move BELOW this statement? +/// +/// The fold evaluates the appended values where the literal ends up, so a +/// statement standing between the binding and its first assignment is only +/// skippable when moving the ALLOCATION past it is unobservable. That is the +/// same pair of conditions `value_is_fold_safe` already enforces on the value +/// side, for the same two reasons: +/// +/// - the statement must not NAME the binding — it would otherwise read, write +/// or capture an object that no longer exists at that point (`const o = {}; +/// f(o); o.a = 1` keeps its dynamic writes); +/// - the statement must not be able to execute user code, because a call can +/// reach a hoisted `function peek() { return o; }` that names the binding +/// WITHOUT the statement naming it, turning a successful read into a TDZ +/// `ReferenceError`. Reusing `value_is_fold_safe` for initializers and +/// expression statements buys exactly that test, and deliberately shares +/// its precision: that predicate admits the implicit conversions (`a + b`, +/// a template substitution) that can still reach a user `valueOf`, which +/// is a pre-existing imprecision of the value side, tracked separately — +/// the two sides must not drift apart here. +/// +/// Destructuring patterns are excluded: the binding itself performs property +/// reads, which can run a getter. Type-only declarations are erased before +/// codegen, so they carry no runtime effect at all and always qualify — +/// `enum` and `namespace` do emit code and do not. +fn gap_stmt_is_hoistable(s: &ast::Stmt, name: &str) -> bool { + match s { + ast::Stmt::Empty(_) => true, + ast::Stmt::Decl(ast::Decl::TsInterface(_) | ast::Decl::TsTypeAlias(_)) => true, + ast::Stmt::Expr(es) => value_is_fold_safe(&es.expr, name), + ast::Stmt::Decl(ast::Decl::Var(var)) => var.decls.iter().all(|d| { + matches!(&d.name, ast::Pat::Ident(bi) if bi.id.sym.as_ref() != name) + && d.init + .as_deref() + .is_none_or(|init| value_is_fold_safe(init, name)) + }), + _ => false, + } +} + /// The literal may only contain plain key/value + shorthand props; anything /// else (accessors, spreads, computed keys, methods) disables folding. fn literal_is_foldable(props: &[ast::PropOrSpread]) -> bool { diff --git a/crates/perry-hir/tests/builder_fold_gap.rs b/crates/perry-hir/tests/builder_fold_gap.rs new file mode 100644 index 0000000000..13ac6ea4af --- /dev/null +++ b/crates/perry-hir/tests/builder_fold_gap.rs @@ -0,0 +1,384 @@ +//! #10353: a builder whose assignments do not IMMEDIATELY follow the `{}` +//! binding must still fold into the literal — `const o = {}; const X = 1; +//! o.a = X;` is the ordinary shape of initialisation code, and leaving it +//! unfolded costs 75× (a 0-field anon shape that every store then transitions +//! dynamically through `js_put_value_set`). +//! +//! The gap is only skippable while moving the ALLOCATION below it stays +//! unobservable, so these tests pin both directions: the fold happens for +//! inert statements, and it does NOT happen for a statement that names the +//! binding, can run user code, or destructures. + +use perry_diagnostics::SourceCache; +use perry_hir::{lower_module, Expr, Stmt}; +use perry_parser::parse_typescript_with_cache; + +fn lower_src(src: &str) -> perry_hir::Module { + let mut cache = SourceCache::new(); + let parsed = parse_typescript_with_cache(src, "builder_fold_gap.ts", &mut cache) + .expect("parse should succeed"); + lower_module(&parsed.module, "test", "builder_fold_gap.ts").expect("lower should succeed") +} + +/// Number of constructor args on the `__AnonShape_…` allocation bound to +/// `name`, i.e. how many properties the literal carries after folding. +fn anon_shape_arity(stmts: &[Stmt], name: &str) -> Option { + stmts.iter().find_map(|stmt| match stmt { + Stmt::Let { + name: binding, + init: Some(Expr::New { + class_name, args, .. + }), + .. + } if binding == name && class_name.starts_with("__AnonShape_") => Some(args.len()), + _ => None, + }) +} + +fn runtime_set_keys(stmts: &[Stmt]) -> Vec { + stmts + .iter() + .filter_map(|stmt| match stmt { + Stmt::Expr(Expr::PutValueSet { key, .. }) => match key.as_ref() { + Expr::String(k) => Some(k.clone()), + _ => None, + }, + _ => None, + }) + .collect() +} + +fn fn_body<'a>(module: &'a perry_hir::Module, name: &str) -> &'a [Stmt] { + module + .functions + .iter() + .find(|f| f.name == name) + .map(|f| f.body.as_slice()) + .unwrap_or_else(|| panic!("function `{name}` not found")) +} + +/// Index of the `Let` binding `name` within a statement list. +fn let_position(stmts: &[Stmt], name: &str) -> Option { + stmts + .iter() + .position(|stmt| matches!(stmt, Stmt::Let { name: binding, .. } if binding == name)) +} + +#[test] +fn a_constant_binding_between_the_literal_and_its_stores_still_folds() { + // The issue's `vA.ts`, at module scope. + let module = lower_src( + r#" + const o: any = {}; + const x = 1; + o.p1 = x; o.p2 = x; o.p3 = x; o.p4 = x; o.p5 = x; o.p6 = x; + "#, + ); + + assert_eq!( + anon_shape_arity(&module.init, "o"), + Some(6), + "all six stores should have folded into the allocation: {:?}", + module.init + ); + assert!( + runtime_set_keys(&module.init).is_empty(), + "no store should survive as a dynamic [[Set]]: {:?}", + runtime_set_keys(&module.init) + ); +} + +#[test] +fn the_skipped_binding_is_still_initialized_before_the_literal() { + // Sinking the allocation below `const x` is the whole point: the folded + // values read `x`, so `x` must still be initialized first or the literal + // would hit `x`'s TDZ. + let module = lower_src( + r#" + const o: any = {}; + const x = 1; + o.p1 = x; + "#, + ); + + let x = let_position(&module.init, "x").expect("`x` binding not found"); + let o = let_position(&module.init, "o").expect("`o` binding not found"); + assert!( + x < o, + "`x` must be initialized before the folded literal: {:?}", + module.init + ); +} + +#[test] +fn a_gap_inside_a_function_body_folds_too() { + let module = lower_src( + r#" + export function build(): any { + const o: any = {}; + const x = 1; + o.p1 = x; o.p2 = x; + return o; + } + "#, + ); + + let body = fn_body(&module, "build"); + assert_eq!( + anon_shape_arity(body, "o"), + Some(2), + "both stores should have folded inside the function body: {body:?}" + ); + assert!( + runtime_set_keys(body).is_empty(), + "no store should survive as a dynamic [[Set]]: {:?}", + runtime_set_keys(body) + ); +} + +#[test] +fn several_skipped_bindings_are_all_kept_in_order() { + let module = lower_src( + r#" + const o: any = {}; + const a = 1; + const b = a + 1; + o.p1 = a; + o.p2 = b; + "#, + ); + + assert_eq!( + anon_shape_arity(&module.init, "o"), + Some(2), + "both stores should have folded: {:?}", + module.init + ); + let a = let_position(&module.init, "a").expect("`a` binding not found"); + let b = let_position(&module.init, "b").expect("`b` binding not found"); + let o = let_position(&module.init, "o").expect("`o` binding not found"); + assert!( + a < b && b < o, + "the skipped bindings must keep their order and precede the literal: {:?}", + module.init + ); +} + +#[test] +fn a_second_builder_in_the_gap_folds_as_well() { + let module = lower_src( + r#" + const a: any = {}; + const b: any = {}; + a.x = 1; + b.y = 2; + "#, + ); + + assert_eq!( + anon_shape_arity(&module.init, "a"), + Some(1), + "the outer builder should fold: {:?}", + module.init + ); + assert_eq!( + anon_shape_arity(&module.init, "b"), + Some(1), + "the builder found in the gap should fold too: {:?}", + module.init + ); +} + +#[test] +fn a_gap_statement_that_names_the_binding_blocks_the_fold() { + // `alias` reads `o` before the allocation would happen — sinking the + // declaration past it would be a TDZ ReferenceError. + let module = lower_src( + r#" + const o: any = {}; + const alias = o; + o.p1 = 1; + "#, + ); + + assert_eq!( + anon_shape_arity(&module.init, "o"), + Some(0), + "the allocation must stay empty: {:?}", + module.init + ); + assert_eq!( + runtime_set_keys(&module.init), + vec!["p1".to_string()], + "the store must survive as a dynamic [[Set]]: {:?}", + module.init + ); +} + +#[test] +fn a_gap_statement_that_can_run_user_code_blocks_the_fold() { + // The gap statement does not name `o`, but `peek()` reads it — sinking + // the allocation below the call would turn that read into a TDZ + // ReferenceError. + let module = lower_src( + r#" + function peek(): any { return o; } + const o: any = {}; + const seen = peek(); + o.p1 = 1; + "#, + ); + + assert_eq!( + anon_shape_arity(&module.init, "o"), + Some(0), + "a call in the gap must block the fold: {:?}", + module.init + ); + assert_eq!( + runtime_set_keys(&module.init), + vec!["p1".to_string()], + "the store must survive as a dynamic [[Set]]: {:?}", + module.init + ); +} + +#[test] +fn a_destructuring_gap_blocks_the_fold() { + // The pattern itself performs property reads, which can run a getter. + let module = lower_src( + r#" + const src: any = { a: 1 }; + const o: any = {}; + const { a } = src; + o.p1 = a; + "#, + ); + + assert_eq!( + anon_shape_arity(&module.init, "o"), + Some(0), + "a destructuring gap must block the fold: {:?}", + module.init + ); + assert_eq!( + runtime_set_keys(&module.init), + vec!["p1".to_string()], + "the store must survive as a dynamic [[Set]]: {:?}", + module.init + ); +} + +#[test] +fn a_member_read_in_the_gap_blocks_the_fold() { + // A getter on `src` could reach the binding. + let module = lower_src( + r#" + const src: any = { a: 1 }; + const o: any = {}; + const a = src.a; + o.p1 = a; + "#, + ); + + assert_eq!( + anon_shape_arity(&module.init, "o"), + Some(0), + "a member read in the gap must block the fold: {:?}", + module.init + ); + assert_eq!( + runtime_set_keys(&module.init), + vec!["p1".to_string()], + "the store must survive as a dynamic [[Set]]: {:?}", + module.init + ); +} + +#[test] +fn a_populated_literal_does_not_skip_a_gap() { + // Sinking `{ a: y }` below `const y` would turn a TDZ ReferenceError into + // a successful build, so only empty literals may skip statements. + let module = lower_src( + r#" + const o: any = { a: 1 }; + const y = 2; + o.b = y; + "#, + ); + + assert_eq!( + anon_shape_arity(&module.init, "o"), + Some(1), + "the populated literal must keep its single property: {:?}", + module.init + ); + assert_eq!( + runtime_set_keys(&module.init), + vec!["b".to_string()], + "the store must survive as a dynamic [[Set]]: {:?}", + module.init + ); +} + +#[test] +fn an_adjacent_builder_still_folds_into_a_populated_literal() { + // The pre-#10353 behaviour is unchanged when there is no gap. + let module = lower_src( + r#" + const o: any = { a: 1 }; + o.b = 2; + "#, + ); + + assert_eq!( + anon_shape_arity(&module.init, "o"), + Some(2), + "an adjacent store should still fold: {:?}", + module.init + ); + assert!( + runtime_set_keys(&module.init).is_empty(), + "no store should survive as a dynamic [[Set]]: {:?}", + runtime_set_keys(&module.init) + ); +} + +#[test] +fn a_type_only_declaration_in_the_gap_is_skipped() { + let module = lower_src( + r#" + const o: any = {}; + type Width = number; + interface Shape { w: Width } + const x: Width = 1; + o.p1 = x; + "#, + ); + + assert_eq!( + anon_shape_arity(&module.init, "o"), + Some(1), + "erased declarations should not block the fold: {:?}", + module.init + ); +} + +#[test] +fn an_enum_in_the_gap_blocks_the_fold() { + // Unlike `type`/`interface`, an enum emits an initializer at run time. + let module = lower_src( + r#" + const o: any = {}; + enum Color { Red } + o.p1 = 1; + "#, + ); + + assert_eq!( + anon_shape_arity(&module.init, "o"), + Some(0), + "an enum in the gap must block the fold: {:?}", + module.init + ); +} diff --git a/crates/perry/tests/builder_fold_gap_semantics.rs b/crates/perry/tests/builder_fold_gap_semantics.rs new file mode 100644 index 0000000000..1909593d75 --- /dev/null +++ b/crates/perry/tests/builder_fold_gap_semantics.rs @@ -0,0 +1,137 @@ +//! #10353: the builder fold may skip statements between a `{}` binding and +//! its stores, which sinks the ALLOCATION below them. These are the observable +//! consequences — the HIR tests in `perry-hir/tests/builder_fold_gap.rs` pin +//! which shapes fold, this file pins that folding them changes nothing a +//! program can see. + +use std::path::PathBuf; +use std::process::Command; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +fn compile_and_run(source: &str) -> String { + let dir = tempfile::tempdir().expect("tempdir"); + let entry = dir.path().join("main.ts"); + let output = dir.path().join("main_bin"); + std::fs::write(&entry, source).expect("write entry"); + + let compile = Command::new(perry_bin()) + .current_dir(dir.path()) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .arg("--no-cache") + .output() + .expect("run perry compile"); + assert!( + compile.status.success(), + "perry compile failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + + let run = Command::new(&output) + .current_dir(dir.path()) + .output() + .expect("run compiled binary"); + assert!( + run.status.success(), + "compiled binary failed (exit {:?})\nstdout:\n{}\nstderr:\n{}", + run.status.code(), + String::from_utf8_lossy(&run.stdout), + String::from_utf8_lossy(&run.stderr) + ); + String::from_utf8_lossy(&run.stdout).into_owned() +} + +#[test] +fn a_skipped_constant_binding_leaves_the_object_unchanged() { + // The issue's repro plus the shape observations a fold could disturb: + // key order, own-property-ness and `in` must all match the unfolded form. + let stdout = compile_and_run( + r#" +function build(): any { + const o: any = {}; + const x = 1; + o.p1 = x; o.p2 = x + 1; o.p3 = x + 2; + return o; +} +const o = build(); +console.log(JSON.stringify(o)); +console.log(Object.keys(o).join(",")); +console.log("p1" in o, "p9" in o, Object.prototype.hasOwnProperty.call(o, "p3")); +"#, + ); + assert_eq!( + stdout, + "{\"p1\":1,\"p2\":2,\"p3\":3}\np1,p2,p3\ntrue false true\n" + ); +} + +#[test] +fn skipped_bindings_still_run_before_the_literal() { + let stdout = compile_and_run( + r#" +const o: any = {}; +const a = 2; +const b = a * 3; +o.sum = a + b; +o.b = b; +console.log(JSON.stringify(o)); +"#, + ); + assert_eq!(stdout, "{\"sum\":8,\"b\":6}\n"); +} + +#[test] +fn a_gap_statement_that_reads_the_binding_still_sees_the_object() { + // `peek()` does not name `o` at the call site, but it reads it. Sinking + // the allocation below the call would make this a TDZ ReferenceError. + let stdout = compile_and_run( + r#" +function peek(): any { return o; } +const o: any = {}; +const seen = peek(); +o.p1 = 1; +console.log(seen === o, seen.p1, JSON.stringify(o)); +"#, + ); + assert_eq!(stdout, "true 1 {\"p1\":1}\n"); +} + +#[test] +fn an_alias_taken_in_the_gap_still_observes_the_stores() { + let stdout = compile_and_run( + r#" +const o: any = {}; +const alias = o; +o.p1 = 1; +o.p2 = 2; +console.log(alias === o, JSON.stringify(alias)); +"#, + ); + assert_eq!(stdout, "true {\"p1\":1,\"p2\":2}\n"); +} + +#[test] +fn a_populated_literal_keeps_its_own_evaluation_order() { + let stdout = compile_and_run( + r#" +function run(): string { + try { + const o: any = { a: (y as any) }; + const y = 1; + o.b = 2; + return JSON.stringify(o); + } catch (e: any) { + return "threw " + e.name; + } +} +console.log(run()); +"#, + ); + assert_eq!(stdout, "threw ReferenceError\n"); +} From 9148f107eff3515822cb2e9ef642a5f96bd07e40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 16 Sep 2026 10:39:11 +0200 Subject: [PATCH 2/2] fix(hir): keep observable user code out of a folded builder (#10357) Folding `const o = {}; o.a = v;` into `const o = { a: v }` evaluates `v` before `o` is initialized. `value_is_fold_safe` claimed such values run no user code, yet admitted two ways to run it. First, every converting operator: `"" + w`, `-w`, `w < 1`, `w == 1` and `` `${w}` `` all call `w`'s `valueOf`/`toString`/`Symbol.toPrimitive`. Second, every bare identifier: a name that is no binding reads the global object, whose property may be an accessor. When that code reads `o`, node builds the object and perry threw a TDZ ReferenceError. Dropping both would stop the fold's own motivating example (`o.b = r + i`) from folding. Instead, a new `FoldScope` asks whether anything can read the binding early. User code can only read a binding it names, so the scan looks for function-likes in the enclosing function (or module) that mention the builder's name. To avoid giving up folds, it ignores a function-like created after a `let`/`const` builder (the binding is fresh per loop pass and execution within a pass only moves forward) and one that re-binds the name at its own function level. It always counts a hoisted function declaration and any observer of a `var`, and treats `eval`, `with`, an export of the name and a module-level `var` as observers outright. Unobservable builders fold exactly as before. On an observable one, a conversion needs operands that are primitive by construction, and a read needs a proven declarative binding. `Visible` provides that proof from declarations that cover the whole region: a list's own declarations, parameters, function-scoped `var`s, loop heads, catch parameters, and module imports and declarations. It never counts a sibling block, an ambient `declare` (#10363), or anything in a module containing `with`. The gap statements from #10355 take the same answers. --- changelog.d/10361-builder-fold-toprimitive.md | 44 + crates/perry-hir/src/lower/builder_fold.rs | 1022 ++++++++++++++--- .../tests/builder_fold_conversion.rs | 561 +++++++++ .../builder_fold_conversion_semantics.rs | 195 ++++ 4 files changed, 1692 insertions(+), 130 deletions(-) create mode 100644 changelog.d/10361-builder-fold-toprimitive.md create mode 100644 crates/perry-hir/tests/builder_fold_conversion.rs create mode 100644 crates/perry/tests/builder_fold_conversion_semantics.rs diff --git a/changelog.d/10361-builder-fold-toprimitive.md b/changelog.d/10361-builder-fold-toprimitive.md new file mode 100644 index 0000000000..c7d7ec5b1c --- /dev/null +++ b/changelog.d/10361-builder-fold-toprimitive.md @@ -0,0 +1,44 @@ +Fixed the builder fold (#6812) turning a successful read into a TDZ +`ReferenceError` when a folded value runs an implicit conversion (#10357). +Folding `const o = {}; o.a = v;` into `const o = { a: v }` evaluates `v` before +`o` is initialized, which is unobservable only if `v` runs no user code that +can read `o`. `value_is_fold_safe` claimed exactly that, yet admitted every +converting operator: `"" + w`, `-w`, `w < 1`, `w == 1` and `` `${w}` `` all call +`w`'s `valueOf`/`toString`/`Symbol.toPrimitive`. With +`w = { valueOf() { return o; } }` node builds `{ a: "[object Object]" }` and +perry threw. + +Every such conversion could have been dropped from the predicate, but that +would have stopped the fold's own motivating example (`o.b = r + i`) from +folding. Instead the fold now asks whether anything *can* read the binding +early. A binding is only readable by code that names it, so user code reached +through a conversion must be a function-like nested in the builder's scope +that mentions the name. `FoldScope` scans each function body (or the module) +for such observers, and keeps that scan precise because every false positive +costs a fold: + +- a function-like created after a `let`/`const` builder cannot run before it + (the binding is fresh per loop pass and execution within a pass only moves + forward); a hoisted function declaration always counts, and so does any + observer of a `var`, whose binding every loop pass shares; +- a nested function-like that re-binds the name as a parameter or top-level + body declaration is not an observer (body declarations do not shadow + parameter defaults); +- `eval` (Perry compiles a literal `eval("o")` into a closure that reads + `o`), `with`, an exported name and a module-level `var` make a builder + observable outright. + +The same hazard hides in a bare identifier read: a name that resolves to no +binding reads the global object, and `Object.defineProperty(globalThis, "g", +{ get() { return o; } })` makes that read user code. `o.a = g` threw exactly +like the conversion. An unobservable builder folds exactly as before. On an +observable builder, a conversion needs operands that are primitive by +construction, and a read needs a proven declarative binding. `Visible` +proves that from a chain built only of declarations that cover the whole +region: a statement list's own declarations, parameters and +function-scoped `var`s, loop heads, catch parameters, and module imports and +declarations. A sibling block's declaration, an ambient `declare` (see +#10363), or any `with` in the module does not count. #10355's gap statements +share the same answers. The scan covers the enclosing function rather than +the folded statement list, because a `var` is function-scoped and a `let` in +one `case` is visible to every other case of its `switch`. diff --git a/crates/perry-hir/src/lower/builder_fold.rs b/crates/perry-hir/src/lower/builder_fold.rs index e91b777692..549c205573 100644 --- a/crates/perry-hir/src/lower/builder_fold.rs +++ b/crates/perry-hir/src/lower/builder_fold.rs @@ -33,6 +33,18 @@ //! - Values must not reference the bound name (checked conservatively by //! symbol name anywhere in the value expression, ignoring shadowing), so //! no expression can observe the half-built object. +//! - #10357: values must not run user code that could read the binding +//! either. Calls, getters and the like never qualify. An implicit +//! conversion (`"" + w`, `-w`, `` `${w}` ``) calls a user `valueOf` too, +//! and a bare read of a name that is no binding reads the global object, +//! whose property may be an accessor. Both qualify only when nothing in +//! the builder's scope can read the binding early (`FoldScope`: no +//! function-like that names it and can already exist, no `eval`, `with` or +//! export, not a module-level `var`); otherwise a conversion needs operands +//! that are primitive by construction and a read needs a proven +//! declarative binding (`Visible`). Before #10357 both always qualified, +//! and `const o = {}; o.a = "" + w;` threw a TDZ `ReferenceError` whenever +//! `w.valueOf` read `o`. //! - If a value throws, the original leaves a partially-built object bound //! to a local no live code can reach (the following statements never run, //! and the values captured no reference to it) — indistinguishable. @@ -55,6 +67,7 @@ //! A miss here is only a missed optimization: unmatched shapes lower //! exactly as before. +use swc_common::{BytePos, Spanned}; use swc_ecma_ast as ast; use swc_ecma_visit::{Visit, VisitWith}; @@ -74,9 +87,11 @@ pub(crate) fn fold_builder_sequences(module: &ast::Module) -> Option bool { Some(ast::ModuleItem::Stmt(s)) => Some(s), _ => None, }; - let gap = fold_gap_len(name.as_str(), item_stmt); + let gap = fold_gap_len(name.as_str(), false, &Visible::ROOT, item_stmt); if item_stmt(gap).is_some_and(|b| assign_to_name_key(b, name.as_str()).is_some()) { return true; } @@ -210,7 +225,9 @@ fn stmts_have_candidate(stmts: &[ast::Stmt]) -> bool { let (Some(name), _) = decl_object_binding(s) else { continue; }; - let gap = fold_gap_len(name.as_str(), |k| stmts.get(i + 1 + k)); + let gap = fold_gap_len(name.as_str(), false, &Visible::ROOT, |k| { + stmts.get(i + 1 + k) + }); if stmts .get(i + 1 + gap) .is_some_and(|b| assign_to_name_key(b, name.as_str()).is_some()) @@ -354,7 +371,12 @@ fn scan_expr(e: &ast::Expr) -> bool { } } -fn process_module_items(items: &mut [ast::ModuleItem], changed: &mut bool) { +fn process_module_items( + items: &mut [ast::ModuleItem], + changed: &mut bool, + scope: &FoldScope, + visible: &Visible<'_>, +) { // Fold across consecutive top-level Stmt items. let mut i = 0; while i < items.len() { @@ -365,16 +387,16 @@ fn process_module_items(items: &mut [ast::ModuleItem], changed: &mut bool) { j += 1; } // Temporarily extract the run as &mut [Stmt]-alike processing. - fold_module_stmt_run(&mut items[i..j], changed); + fold_module_stmt_run(&mut items[i..j], changed, scope, visible); for item in items[i..j].iter_mut() { if let ast::ModuleItem::Stmt(s) = item { - walk_stmt(s, changed); + walk_stmt(s, changed, scope, visible); } } i = j; } else { if let ast::ModuleItem::ModuleDecl(ast::ModuleDecl::ExportDecl(ed)) = &mut items[i] { - walk_decl(&mut ed.decl, changed); + walk_decl(&mut ed.decl, changed, visible); } i += 1; } @@ -383,7 +405,12 @@ fn process_module_items(items: &mut [ast::ModuleItem], changed: &mut bool) { /// Fold within a run of top-level ModuleItem::Stmt entries. Consumed /// assignment statements are replaced with `;` (EmptyStmt). -fn fold_module_stmt_run(items: &mut [ast::ModuleItem], changed: &mut bool) { +fn fold_module_stmt_run( + items: &mut [ast::ModuleItem], + changed: &mut bool, + scope: &FoldScope, + visible: &Visible<'_>, +) { let mut idx = 0; while idx < items.len() { let Some((name_start, existing)) = ({ @@ -402,14 +429,20 @@ fn fold_module_stmt_run(items: &mut [ast::ModuleItem], changed: &mut bool) { idx += 1; continue; } + let observable = match &items[idx] { + ast::ModuleItem::Stmt(s) => scope.observes(&name_start, s), + _ => true, + }; // A gap is only skippable for an EMPTY literal: sinking a populated // one would move its own value expressions below the skipped // statements, and `const o = { a: y }; const y = 1; o.b = 2;` must // keep throwing on `y`'s TDZ. let gap = if existing.is_empty() { - fold_gap_len(&name_start, |k| match items.get(idx + 1 + k) { - Some(ast::ModuleItem::Stmt(s)) => Some(s), - _ => None, + fold_gap_len(&name_start, observable, visible, |k| { + match items.get(idx + 1 + k) { + Some(ast::ModuleItem::Stmt(s)) => Some(s), + _ => None, + } }) } else { 0 @@ -425,7 +458,9 @@ fn fold_module_stmt_run(items: &mut [ast::ModuleItem], changed: &mut bool) { let Some((key, value)) = assign_to_name_key(fs, &name_start) else { break; }; - if !fold_key_ok(&key, &keys) || !value_is_fold_safe(value, &name_start) { + if !fold_key_ok(&key, &keys) + || !value_is_fold_safe(value, &name_start, observable, visible) + { break; } if existing.len() + appended.len() >= MAX_FOLDED_PROPS { @@ -465,7 +500,14 @@ fn fold_module_stmt_run(items: &mut [ast::ModuleItem], changed: &mut bool) { } } -fn fold_stmts(stmts: &mut Vec, changed: &mut bool) { +fn fold_stmts( + stmts: &mut Vec, + changed: &mut bool, + scope: &FoldScope, + visible: &Visible<'_>, +) { + // A list's own declarations are bindings for the whole list. + let visible = &visible.child(declared_names(stmts)); let mut idx = 0; while idx < stmts.len() { let foldable = match decl_object_binding(&stmts[idx]) { @@ -478,9 +520,10 @@ fn fold_stmts(stmts: &mut Vec, changed: &mut bool) { idx += 1; continue; }; + let observable = scope.observes(&name, &stmts[idx]); // Empty literals only — see `fold_module_stmt_run`. let gap = if existing_len == 0 { - fold_gap_len(&name, |k| stmts.get(idx + 1 + k)) + fold_gap_len(&name, observable, visible, |k| stmts.get(idx + 1 + k)) } else { 0 }; @@ -491,7 +534,7 @@ fn fold_stmts(stmts: &mut Vec, changed: &mut bool) { let Some((key, value)) = assign_to_name_key(follower, &name) else { break; }; - if !fold_key_ok(&key, &keys) || !value_is_fold_safe(value, &name) { + if !fold_key_ok(&key, &keys) || !value_is_fold_safe(value, &name, observable, visible) { break; } if existing_len + appended.len() >= MAX_FOLDED_PROPS { @@ -521,7 +564,7 @@ fn fold_stmts(stmts: &mut Vec, changed: &mut bool) { idx += 1; } for s in stmts.iter_mut() { - walk_stmt(s, changed); + walk_stmt(s, changed, scope, visible); } } @@ -586,11 +629,18 @@ fn assign_to_name_key<'a>( /// the first statement that is an assignment to `name` — that is where the /// fold proper takes over — and at the first statement the declaration may /// not move below. -fn fold_gap_len<'a>(name: &str, at: impl Fn(usize) -> Option<&'a ast::Stmt>) -> usize { +fn fold_gap_len<'a>( + name: &str, + observable: bool, + visible: &Visible<'_>, + at: impl Fn(usize) -> Option<&'a ast::Stmt>, +) -> usize { let mut gap = 0usize; while gap < MAX_FOLD_GAP_STMTS { let Some(s) = at(gap) else { break }; - if assign_to_name_key(s, name).is_some() || !gap_stmt_is_hoistable(s, name) { + if assign_to_name_key(s, name).is_some() + || !gap_stmt_is_hoistable(s, name, observable, visible) + { break; } gap += 1; @@ -613,26 +663,29 @@ fn fold_gap_len<'a>(name: &str, at: impl Fn(usize) -> Option<&'a ast::Stmt>) -> /// reach a hoisted `function peek() { return o; }` that names the binding /// WITHOUT the statement naming it, turning a successful read into a TDZ /// `ReferenceError`. Reusing `value_is_fold_safe` for initializers and -/// expression statements buys exactly that test, and deliberately shares -/// its precision: that predicate admits the implicit conversions (`a + b`, -/// a template substitution) that can still reach a user `valueOf`, which -/// is a pre-existing imprecision of the value side, tracked separately — -/// the two sides must not drift apart here. +/// expression statements buys exactly that test — including its #10357 +/// rule for implicit conversions, which take the same builder's +/// `observable` answer so the two sides cannot drift apart. /// /// Destructuring patterns are excluded: the binding itself performs property /// reads, which can run a getter. Type-only declarations are erased before /// codegen, so they carry no runtime effect at all and always qualify — /// `enum` and `namespace` do emit code and do not. -fn gap_stmt_is_hoistable(s: &ast::Stmt, name: &str) -> bool { +fn gap_stmt_is_hoistable( + s: &ast::Stmt, + name: &str, + observable: bool, + visible: &Visible<'_>, +) -> bool { match s { ast::Stmt::Empty(_) => true, ast::Stmt::Decl(ast::Decl::TsInterface(_) | ast::Decl::TsTypeAlias(_)) => true, - ast::Stmt::Expr(es) => value_is_fold_safe(&es.expr, name), + ast::Stmt::Expr(es) => value_is_fold_safe(&es.expr, name, observable, visible), ast::Stmt::Decl(ast::Decl::Var(var)) => var.decls.iter().all(|d| { matches!(&d.name, ast::Pat::Ident(bi) if bi.id.sym.as_ref() != name) && d.init .as_deref() - .is_none_or(|init| value_is_fold_safe(init, name)) + .is_none_or(|init| value_is_fold_safe(init, name, observable, visible)) }), _ => false, } @@ -707,55 +760,78 @@ fn append_props(s: &mut ast::Stmt, appended: Vec<(ast::PropName, Box) } /// May this VALUE expression fold into a literal that now evaluates it -/// BEFORE the builder binding is initialized? Only expressions that -/// provably cannot execute user code qualify — a call, `new`, member read -/// (getters), optional chain, tagged template, spread (iterator -/// protocols), `in`/`instanceof` (traps / `Symbol.hasInstance`), -/// `await`/`yield`, or any function-bearing form could reach the binding -/// through a closure or trap WITHOUT naming it (e.g. a hoisted -/// `function f() { return o.a; }` observed via `o.b = f()` — folding +/// BEFORE the builder binding is initialized? +/// +/// A call, `new`, member read (getters), optional chain, tagged template, +/// spread (iterator protocols), `in`/`instanceof` (traps / +/// `Symbol.hasInstance`), `await`/`yield`, or any function-bearing form can +/// run arbitrary user code, so none of them ever qualifies — that code could +/// reach the binding through a closure WITHOUT the value naming it (e.g. a +/// hoisted `function f() { return o.a; }` observed via `o.b = f()` — folding /// would turn the original's successful read into a TDZ ReferenceError). /// Reading OTHER identifiers is safe (identical evaluation either side of /// the allocation); reading the builder's own name is excluded directly. -fn value_is_fold_safe(e: &ast::Expr, name: &str) -> bool { +/// +/// #10357: an implicit conversion runs user code too. `"" + w`, `-w`, `w < 1` +/// and `` `${w}` `` call `w`'s `valueOf`/`toString`/`Symbol.toPrimitive`. That +/// is only a problem when such code can read the binding at all, which is +/// what `observable` answers (`FoldScope::observes`): when nothing in scope +/// can, the conversion is unobservable and folds as before; when something +/// can, a converting operator qualifies only on operands that are primitive +/// by construction (`is_primitive_valued`), where no user code runs. +/// +/// A bare identifier read is the same hazard in disguise: one that resolves +/// to no binding reads a property of the global object, and that property +/// can be an accessor whose getter reads the builder. On an observable +/// builder a read therefore qualifies only when `visible` proves it resolves +/// to a declarative binding. +fn value_is_fold_safe(e: &ast::Expr, name: &str, observable: bool, visible: &Visible<'_>) -> bool { use ast::Expr as E; + let safe = |x: &ast::Expr| value_is_fold_safe(x, name, observable, visible); + let read_ok = |sym: &str| sym != name && (!observable || visible.resolves(sym)); match e { E::Lit(_) | E::This(_) => true, - E::Ident(i) => i.sym.as_ref() != name, - E::Paren(p) => value_is_fold_safe(&p.expr, name), - E::Tpl(t) => t.exprs.iter().all(|x| value_is_fold_safe(x, name)), - E::Unary(u) => u.op != ast::UnaryOp::Delete && value_is_fold_safe(&u.arg, name), + E::Ident(i) => read_ok(i.sym.as_ref()), + E::Paren(p) => safe(&p.expr), + E::Tpl(t) => t + .exprs + .iter() + .all(|x| safe(x) && (!observable || is_primitive_valued(x))), + E::Unary(u) => { + u.op != ast::UnaryOp::Delete + && safe(&u.arg) + && (!observable || !unary_converts(u.op) || is_primitive_valued(&u.arg)) + } E::Bin(b) => { !matches!(b.op, ast::BinaryOp::In | ast::BinaryOp::InstanceOf) - && value_is_fold_safe(&b.left, name) - && value_is_fold_safe(&b.right, name) - } - E::Cond(c) => { - value_is_fold_safe(&c.test, name) - && value_is_fold_safe(&c.cons, name) - && value_is_fold_safe(&c.alt, name) + && safe(&b.left) + && safe(&b.right) + && (!observable + || !binary_converts(b.op) + || (is_primitive_valued(&b.left) && is_primitive_valued(&b.right))) } - E::Seq(sq) => sq.exprs.iter().all(|x| value_is_fold_safe(x, name)), + E::Cond(c) => safe(&c.test) && safe(&c.cons) && safe(&c.alt), + E::Seq(sq) => sq.exprs.iter().all(|x| safe(x)), E::Array(a) => a.elems.iter().all(|el| match el { None => true, - Some(el) => el.spread.is_none() && value_is_fold_safe(&el.expr, name), + Some(el) => el.spread.is_none() && safe(&el.expr), }), E::Object(o) => o.props.iter().all(|p| match p { ast::PropOrSpread::Spread(_) => false, ast::PropOrSpread::Prop(prop) => match &**prop { ast::Prop::KeyValue(kv) => { matches!(kv.key, ast::PropName::Ident(_) | ast::PropName::Str(_)) - && value_is_fold_safe(&kv.value, name) + && safe(&kv.value) } - ast::Prop::Shorthand(i) => i.sym.as_ref() != name, + ast::Prop::Shorthand(i) => read_ok(i.sym.as_ref()), _ => false, }, }), - E::TsAs(t) => value_is_fold_safe(&t.expr, name), - E::TsNonNull(t) => value_is_fold_safe(&t.expr, name), - E::TsTypeAssertion(t) => value_is_fold_safe(&t.expr, name), - E::TsSatisfies(t) => value_is_fold_safe(&t.expr, name), - E::TsConstAssertion(t) => value_is_fold_safe(&t.expr, name), + E::TsAs(t) => safe(&t.expr), + E::TsNonNull(t) => safe(&t.expr), + E::TsTypeAssertion(t) => safe(&t.expr), + E::TsSatisfies(t) => safe(&t.expr), + E::TsConstAssertion(t) => safe(&t.expr), // Everything else — calls, news, member/optional access, tagged // templates, await/yield, updates, assignments, function-bearing // forms, unknown variants — may execute user code: unsafe to hoist @@ -764,51 +840,488 @@ fn value_is_fold_safe(e: &ast::Expr, name: &str) -> bool { } } -fn walk_decl(d: &mut ast::Decl, changed: &mut bool) { - if let ast::Decl::Fn(f) = d { - if let Some(body) = &mut f.function.body { - fold_stmts(&mut body.stmts, changed); +/// Does this unary operator convert its operand (`ToNumeric`)? `!` is +/// `ToBoolean`, `typeof`/`void` inspect without converting — none of those +/// can call user code. +fn unary_converts(op: ast::UnaryOp) -> bool { + matches!( + op, + ast::UnaryOp::Minus | ast::UnaryOp::Plus | ast::UnaryOp::Tilde + ) +} + +/// Does this binary operator convert its operands? Only strict equality and +/// the short-circuiting operators (`ToBoolean`, or no conversion at all) +/// cannot reach `valueOf`/`toString`. Loose equality can — an object +/// compared with a primitive goes through `ToPrimitive`. +fn binary_converts(op: ast::BinaryOp) -> bool { + !matches!( + op, + ast::BinaryOp::EqEqEq + | ast::BinaryOp::NotEqEq + | ast::BinaryOp::LogicalAnd + | ast::BinaryOp::LogicalOr + | ast::BinaryOp::NullishCoalescing + ) +} + +/// Is this expression's value a primitive by construction, whatever its +/// operands are? A conversion of a primitive runs no user code. Deliberately +/// syntactic: identifiers are never primitive here (no type facts at this +/// stage, and a `: number` annotation is not a proof), and a regex literal is +/// an object whose `toString` is user-patchable. +fn is_primitive_valued(e: &ast::Expr) -> bool { + use ast::Expr as E; + match e { + E::Lit(lit) => !matches!(lit, ast::Lit::Regex(_) | ast::Lit::JSXText(_)), + // A template is a string and every unary operator yields a + // primitive, whatever their operands. + E::Tpl(_) | E::Unary(_) => true, + E::Bin(b) => match b.op { + // These return one of their operands. + ast::BinaryOp::LogicalAnd + | ast::BinaryOp::LogicalOr + | ast::BinaryOp::NullishCoalescing => { + is_primitive_valued(&b.left) && is_primitive_valued(&b.right) + } + _ => true, + }, + E::Cond(c) => is_primitive_valued(&c.cons) && is_primitive_valued(&c.alt), + E::Seq(sq) => sq.exprs.last().is_some_and(|x| is_primitive_valued(x)), + E::Paren(p) => is_primitive_valued(&p.expr), + E::TsAs(t) => is_primitive_valued(&t.expr), + E::TsNonNull(t) => is_primitive_valued(&t.expr), + E::TsTypeAssertion(t) => is_primitive_valued(&t.expr), + E::TsSatisfies(t) => is_primitive_valued(&t.expr), + E::TsConstAssertion(t) => is_primitive_valued(&t.expr), + _ => false, + } +} + +/// Which builders in one var scope (a function-like body, or the module) +/// could be read by user code before their folded literal initializes them +/// (#10357). +/// +/// A binding can only be read by code that NAMES it, so the user code a +/// conversion reaches must be a function-like nested in the binding's own +/// scope whose body mentions the name. Three refinements keep that precise, +/// because every false positive costs a fold: +/// +/// - **Position.** A `let`/`const` binding is fresh on every pass through an +/// enclosing loop, and execution within one pass only moves forward, so a +/// function-like CREATED after the declaration cannot run before it. A +/// hoisted function declaration exists from scope entry and always counts. +/// A `var` binding is shared by every pass, so a closure made later in one +/// pass can run during the next pass's fold: for a `var`, any mention +/// counts. +/// - **Shadowing.** A nested function-like that re-binds the name — as a +/// parameter, or a declaration at the top level of its body — reads its own +/// binding, not the builder. Body declarations do not shadow parameter +/// defaults, which are evaluated in a scope of their own; block-level +/// re-declarations are not tracked and so conservatively still count. +/// - **Escapes the name scan cannot see** make every builder observable: +/// `eval` anywhere in the scope (Perry compiles a literal `eval("o")` into a +/// closure that reads `o`), `with` (identifier resolution becomes property +/// lookup), an export of the name (an importer reads the live binding), and +/// a module-level `var` (a global-object property). +/// +/// The scope is the whole enclosing function (or module), not the statement +/// list being folded: a `var` is function-scoped, and a `let` in one `case` +/// is visible to every other case of its `switch`. +#[derive(Default)] +struct FoldScope { + observers: Vec, + /// `eval` or `with` inside the scope. + opaque: bool, + /// Module top level. + top_level: bool, +} + +/// The function-likes that name one builder. +struct Observer { + name: String, + /// Named from code that exists before the scope runs anything: a hoisted + /// function declaration, or an export. + always: bool, + /// Where the earliest other function-like naming it starts. + first_lo: Option, +} + +impl FoldScope { + fn of_module(module: &ast::Module) -> Self { + let mut scope = Self::scan(|v| module.visit_with(v)); + scope.top_level = true; + scope + } + + fn of_body(stmts: &[ast::Stmt]) -> Self { + Self::scan(|v| stmts.visit_with(v)) + } + + /// Two passes, and the second only when it can change an answer: the + /// scope must bind an object literal (a builder candidate) at its own + /// depth. + fn scan(visit: impl Fn(&mut dyn Visit)) -> Self { + let mut own = OwnScopeScan::default(); + visit(&mut own); + if own.builders.is_empty() { + return Self::default(); + } + let mut observers = ObserverScan::new(&own.builders); + visit(&mut observers); + Self { + observers: observers.observers, + opaque: observers.opaque, + top_level: false, } } - if let ast::Decl::Class(c) = d { - walk_class(&mut c.class, changed); + + /// Is the builder declared by `decl` (bound to `name`) observable? + fn observes(&self, name: &str, decl: &ast::Stmt) -> bool { + let is_var = matches!( + decl, + ast::Stmt::Decl(ast::Decl::Var(v)) if v.kind == ast::VarDeclKind::Var + ); + if self.opaque || (self.top_level && is_var) { + return true; + } + let Some(observer) = self.observers.iter().find(|o| o.name == name) else { + return false; + }; + observer.always || is_var || observer.first_lo.is_some_and(|lo| lo < decl.span().hi) } - if let ast::Decl::Var(v) = d { - for decl in &mut v.decls { - if let Some(init) = &mut decl.init { - walk_expr(init, changed); +} + +/// Pass 1: the scope's OWN depth — never enters a nested function-like. +/// Collects object-literal binding names. +#[derive(Default)] +struct OwnScopeScan { + builders: Vec, +} + +impl Visit for OwnScopeScan { + fn visit_var_declarator(&mut self, d: &ast::VarDeclarator) { + if let (ast::Pat::Ident(bi), Some(init)) = (&d.name, d.init.as_deref()) { + if matches!(init, ast::Expr::Object(_)) { + let name = bi.id.sym.to_string(); + if !self.builders.contains(&name) { + self.builders.push(name); + } } } + d.visit_children_with(self); } + // Nested function-likes are separate scopes with their own scan. + fn visit_function(&mut self, _: &ast::Function) {} + fn visit_arrow_expr(&mut self, _: &ast::ArrowExpr) {} + fn visit_constructor(&mut self, _: &ast::Constructor) {} + fn visit_getter_prop(&mut self, _: &ast::GetterProp) {} + fn visit_setter_prop(&mut self, _: &ast::SetterProp) {} + fn visit_class_prop(&mut self, _: &ast::ClassProp) {} + fn visit_private_prop(&mut self, _: &ast::PrivateProp) {} + fn visit_auto_accessor(&mut self, _: &ast::AutoAccessor) {} + fn visit_static_block(&mut self, _: &ast::StaticBlock) {} +} + +/// Pass 2: every depth. Records where each builder name is mentioned by a +/// nested function-like or an export, and any `eval`/`with`. +struct ObserverScan<'b> { + builders: &'b [String], + depth: u32, + in_export: bool, + /// Set by a function declaration for the function it declares. + declared: bool, + /// The outermost nested function-like being walked. + frame_hoisted: bool, + frame_lo: BytePos, + /// Builder names re-bound by an enclosing nested function-like. + shadowed: Vec, + observers: Vec, + opaque: bool, } -fn walk_class(class: &mut ast::Class, changed: &mut bool) { +impl<'b> ObserverScan<'b> { + fn new(builders: &'b [String]) -> Self { + Self { + builders, + depth: 0, + in_export: false, + declared: false, + frame_hoisted: false, + frame_lo: BytePos::DUMMY, + shadowed: Vec::new(), + observers: Vec::new(), + opaque: false, + } + } + + fn enter(&mut self, lo: BytePos) -> usize { + let declared = std::mem::take(&mut self.declared); + if self.depth == 0 { + self.frame_hoisted = declared; + self.frame_lo = lo; + } + self.depth += 1; + self.shadowed.len() + } + + fn leave(&mut self, mark: usize) { + self.depth -= 1; + self.shadowed.truncate(mark); + } + + fn bind(&mut self, sym: &str) { + if self.builders.iter().any(|b| b == sym) { + self.shadowed.push(sym.to_string()); + } + } + + fn bind_pat(&mut self, pat: &ast::Pat) { + if let ast::Pat::Ident(bi) = pat { + self.bind(bi.id.sym.as_ref()); + } + } + + /// Declarations at the top level of a function-like body bind for the + /// whole body. + fn bind_body(&mut self, stmts: &[ast::Stmt]) { + for stmt in stmts { + match stmt { + ast::Stmt::Decl(ast::Decl::Var(v)) => { + for d in &v.decls { + self.bind_pat(&d.name); + } + } + ast::Stmt::Decl(ast::Decl::Fn(f)) => self.bind(f.ident.sym.as_ref()), + ast::Stmt::Decl(ast::Decl::Class(c)) => self.bind(c.ident.sym.as_ref()), + _ => {} + } + } + } + + fn record(&mut self, sym: &str) { + if !self.builders.iter().any(|b| b == sym) || self.shadowed.iter().any(|s| s == sym) { + return; + } + let always = self.in_export || self.frame_hoisted; + let lo = self.frame_lo; + match self.observers.iter_mut().find(|o| o.name == sym) { + Some(observer) => { + observer.always |= always; + if !always { + observer.first_lo = Some(observer.first_lo.map_or(lo, |first| first.min(lo))); + } + } + None => self.observers.push(Observer { + name: sym.to_string(), + always, + first_lo: (!always).then_some(lo), + }), + } + } +} + +impl Visit for ObserverScan<'_> { + fn visit_ident(&mut self, i: &ast::Ident) { + let sym = i.sym.as_ref(); + if sym == "eval" { + self.opaque = true; + } + if self.depth > 0 || self.in_export { + self.record(sym); + } + } + fn visit_with_stmt(&mut self, w: &ast::WithStmt) { + self.opaque = true; + w.visit_children_with(self); + } + fn visit_module_decl(&mut self, d: &ast::ModuleDecl) { + let outer = std::mem::replace(&mut self.in_export, true); + d.visit_children_with(self); + self.in_export = outer; + } + fn visit_fn_decl(&mut self, f: &ast::FnDecl) { + // The declared name is a binding, not a read. + self.declared = true; + f.function.visit_with(self); + } + fn visit_fn_expr(&mut self, f: &ast::FnExpr) { + // A named function expression binds its own name inside itself. + let mark = self.shadowed.len(); + if let Some(ident) = &f.ident { + self.bind(ident.sym.as_ref()); + } + f.function.visit_with(self); + self.shadowed.truncate(mark); + } + fn visit_function(&mut self, f: &ast::Function) { + let mark = self.enter(f.span.lo); + for param in &f.params { + self.bind_pat(¶m.pat); + } + f.decorators.visit_with(self); + f.params.visit_with(self); + if let Some(body) = &f.body { + self.bind_body(&body.stmts); + body.visit_with(self); + } + self.leave(mark); + } + fn visit_arrow_expr(&mut self, a: &ast::ArrowExpr) { + let mark = self.enter(a.span.lo); + for param in &a.params { + self.bind_pat(param); + } + a.params.visit_with(self); + match &*a.body { + ast::BlockStmtOrExpr::BlockStmt(body) => { + self.bind_body(&body.stmts); + body.visit_with(self); + } + ast::BlockStmtOrExpr::Expr(body) => body.visit_with(self), + } + self.leave(mark); + } + fn visit_constructor(&mut self, c: &ast::Constructor) { + let mark = self.enter(c.span.lo); + for param in &c.params { + match param { + ast::ParamOrTsParamProp::Param(p) => self.bind_pat(&p.pat), + ast::ParamOrTsParamProp::TsParamProp(p) => { + if let ast::TsParamPropParam::Ident(bi) = &p.param { + self.bind(bi.id.sym.as_ref()); + } + } + } + } + c.key.visit_with(self); + c.params.visit_with(self); + if let Some(body) = &c.body { + self.bind_body(&body.stmts); + body.visit_with(self); + } + self.leave(mark); + } + fn visit_getter_prop(&mut self, g: &ast::GetterProp) { + let mark = self.enter(g.span.lo); + g.key.visit_with(self); + if let Some(body) = &g.body { + self.bind_body(&body.stmts); + body.visit_with(self); + } + self.leave(mark); + } + fn visit_setter_prop(&mut self, st: &ast::SetterProp) { + let mark = self.enter(st.span.lo); + self.bind_pat(&st.param); + st.key.visit_with(self); + st.param.visit_with(self); + if let Some(body) = &st.body { + self.bind_body(&body.stmts); + body.visit_with(self); + } + self.leave(mark); + } + fn visit_class_prop(&mut self, p: &ast::ClassProp) { + let mark = self.enter(p.span.lo); + p.visit_children_with(self); + self.leave(mark); + } + fn visit_private_prop(&mut self, p: &ast::PrivateProp) { + let mark = self.enter(p.span.lo); + p.visit_children_with(self); + self.leave(mark); + } + fn visit_auto_accessor(&mut self, a: &ast::AutoAccessor) { + let mark = self.enter(a.span.lo); + a.visit_children_with(self); + self.leave(mark); + } + fn visit_static_block(&mut self, b: &ast::StaticBlock) { + let mark = self.enter(b.span.lo); + self.bind_body(&b.body.stmts); + b.body.visit_with(self); + self.leave(mark); + } +} + +fn walk_decl(d: &mut ast::Decl, changed: &mut bool, visible: &Visible<'_>) { + match d { + ast::Decl::Fn(f) => { + let params = param_names(f.function.params.iter().map(|p| &p.pat)); + if let Some(body) = &mut f.function.body { + fold_body(&mut body.stmts, changed, params, visible); + } + } + ast::Decl::Class(c) => walk_class(&mut c.class, changed, visible), + ast::Decl::Var(v) => { + for decl in &mut v.decls { + if let Some(init) = &mut decl.init { + walk_expr(init, changed, visible); + } + } + } + _ => {} + } +} + +/// Fold a function-like body: its own var scope, so its own `FoldScope`, and +/// its parameters and function-scoped `var`s as bindings for all of it. +fn fold_body( + stmts: &mut Vec, + changed: &mut bool, + mut bindings: Vec, + parent: &Visible<'_>, +) { + let scope = FoldScope::of_body(stmts); + bindings.extend(var_names(stmts)); + let visible = parent.child(bindings); + fold_stmts(stmts, changed, &scope, &visible); +} + +fn walk_class(class: &mut ast::Class, changed: &mut bool, visible: &Visible<'_>) { for member in &mut class.body { match member { ast::ClassMember::Method(m) => { + let params = param_names(m.function.params.iter().map(|p| &p.pat)); if let Some(body) = &mut m.function.body { - fold_stmts(&mut body.stmts, changed); + fold_body(&mut body.stmts, changed, params, visible); } } ast::ClassMember::PrivateMethod(m) => { + let params = param_names(m.function.params.iter().map(|p| &p.pat)); if let Some(body) = &mut m.function.body { - fold_stmts(&mut body.stmts, changed); + fold_body(&mut body.stmts, changed, params, visible); } } ast::ClassMember::Constructor(c) => { + let mut params = Vec::new(); + for param in &c.params { + match param { + ast::ParamOrTsParamProp::Param(p) => collect_pat_names(&p.pat, &mut params), + ast::ParamOrTsParamProp::TsParamProp(p) => match &p.param { + ast::TsParamPropParam::Ident(bi) => params.push(bi.id.sym.to_string()), + ast::TsParamPropParam::Assign(a) => { + collect_pat_names(&a.left, &mut params) + } + }, + } + } if let Some(body) = &mut c.body { - fold_stmts(&mut body.stmts, changed); + fold_body(&mut body.stmts, changed, params, visible); } } - ast::ClassMember::StaticBlock(b) => fold_stmts(&mut b.body.stmts, changed), + ast::ClassMember::StaticBlock(b) => { + fold_body(&mut b.body.stmts, changed, Vec::new(), visible) + } ast::ClassMember::ClassProp(prop) => { if let Some(v) = &mut prop.value { - walk_expr(v, changed); + walk_expr(v, changed, visible); } } ast::ClassMember::PrivateProp(prop) => { if let Some(v) = &mut prop.value { - walk_expr(v, changed); + walk_expr(v, changed, visible); } } _ => {} @@ -816,104 +1329,136 @@ fn walk_class(class: &mut ast::Class, changed: &mut bool) { } } -fn walk_stmt(s: &mut ast::Stmt, changed: &mut bool) { +fn walk_stmt(s: &mut ast::Stmt, changed: &mut bool, scope: &FoldScope, visible: &Visible<'_>) { match s { - ast::Stmt::Block(b) => fold_stmts(&mut b.stmts, changed), + ast::Stmt::Block(b) => fold_stmts(&mut b.stmts, changed, scope, visible), ast::Stmt::If(i) => { - walk_stmt(&mut i.cons, changed); + walk_stmt(&mut i.cons, changed, scope, visible); if let Some(alt) = &mut i.alt { - walk_stmt(alt, changed); + walk_stmt(alt, changed, scope, visible); } - walk_expr(&mut i.test, changed); + walk_expr(&mut i.test, changed, visible); } ast::Stmt::While(w) => { - walk_expr(&mut w.test, changed); - walk_stmt(&mut w.body, changed); + walk_expr(&mut w.test, changed, visible); + walk_stmt(&mut w.body, changed, scope, visible); } ast::Stmt::DoWhile(d) => { - walk_stmt(&mut d.body, changed); - walk_expr(&mut d.test, changed); + walk_stmt(&mut d.body, changed, scope, visible); + walk_expr(&mut d.test, changed, visible); } ast::Stmt::For(f) => { + let mut head = Vec::new(); + if let Some(ast::VarDeclOrExpr::VarDecl(v)) = &f.init { + collect_var_decl_names(v, &mut head); + } + let visible = &visible.child(head); if let Some(ast::VarDeclOrExpr::Expr(e)) = &mut f.init { - walk_expr(e, changed); + walk_expr(e, changed, visible); } if let Some(t) = &mut f.test { - walk_expr(t, changed); + walk_expr(t, changed, visible); } if let Some(u) = &mut f.update { - walk_expr(u, changed); + walk_expr(u, changed, visible); } - walk_stmt(&mut f.body, changed); + walk_stmt(&mut f.body, changed, scope, visible); + } + ast::Stmt::ForIn(f) => { + let visible = &visible.child(for_head_names(&f.left)); + walk_stmt(&mut f.body, changed, scope, visible); + } + ast::Stmt::ForOf(f) => { + let visible = &visible.child(for_head_names(&f.left)); + walk_stmt(&mut f.body, changed, scope, visible); } - ast::Stmt::ForIn(f) => walk_stmt(&mut f.body, changed), - ast::Stmt::ForOf(f) => walk_stmt(&mut f.body, changed), - ast::Stmt::Labeled(l) => walk_stmt(&mut l.body, changed), + ast::Stmt::Labeled(l) => walk_stmt(&mut l.body, changed, scope, visible), ast::Stmt::Try(t) => { - fold_stmts(&mut t.block.stmts, changed); + fold_stmts(&mut t.block.stmts, changed, scope, visible); if let Some(h) = &mut t.handler { - fold_stmts(&mut h.body.stmts, changed); + let mut param = Vec::new(); + if let Some(p) = &h.param { + collect_pat_names(p, &mut param); + } + fold_stmts(&mut h.body.stmts, changed, scope, &visible.child(param)); } if let Some(f) = &mut t.finalizer { - fold_stmts(&mut f.stmts, changed); + fold_stmts(&mut f.stmts, changed, scope, visible); } } ast::Stmt::Switch(sw) => { - walk_expr(&mut sw.discriminant, changed); + walk_expr(&mut sw.discriminant, changed, visible); + // A declaration in any case is scoped to the whole switch block. + let mut declared = Vec::new(); + for case in &sw.cases { + declared.extend(declared_names(&case.cons)); + } + let visible = &visible.child(declared); for case in &mut sw.cases { - fold_stmts(&mut case.cons, changed); + fold_stmts(&mut case.cons, changed, scope, visible); } } - ast::Stmt::Decl(d) => walk_decl(d, changed), - ast::Stmt::Expr(es) => walk_expr(&mut es.expr, changed), + ast::Stmt::Decl(d) => walk_decl(d, changed, visible), + ast::Stmt::Expr(es) => walk_expr(&mut es.expr, changed, visible), ast::Stmt::Return(r) => { if let Some(e) = &mut r.arg { - walk_expr(e, changed); + walk_expr(e, changed, visible); } } - ast::Stmt::Throw(t) => walk_expr(&mut t.arg, changed), + ast::Stmt::Throw(t) => walk_expr(&mut t.arg, changed, visible), _ => {} } } /// Recurse into expressions only far enough to find nested function bodies. -fn walk_expr(e: &mut ast::Expr, changed: &mut bool) { +fn walk_expr(e: &mut ast::Expr, changed: &mut bool, visible: &Visible<'_>) { use ast::Expr as E; match e { E::Fn(f) => { + let mut bindings = param_names(f.function.params.iter().map(|p| &p.pat)); + if let Some(ident) = &f.ident { + bindings.push(ident.sym.to_string()); + } if let Some(body) = &mut f.function.body { - fold_stmts(&mut body.stmts, changed); + fold_body(&mut body.stmts, changed, bindings, visible); } } - E::Arrow(a) => match &mut *a.body { - ast::BlockStmtOrExpr::BlockStmt(b) => fold_stmts(&mut b.stmts, changed), - ast::BlockStmtOrExpr::Expr(e) => walk_expr(e, changed), - }, - E::Class(c) => walk_class(&mut c.class, changed), + E::Arrow(a) => { + let params = param_names(a.params.iter()); + match &mut *a.body { + ast::BlockStmtOrExpr::BlockStmt(b) => { + fold_body(&mut b.stmts, changed, params, visible) + } + ast::BlockStmtOrExpr::Expr(e) => walk_expr(e, changed, &visible.child(params)), + } + } + E::Class(c) => walk_class(&mut c.class, changed, visible), E::Array(a) => { for el in a.elems.iter_mut().flatten() { - walk_expr(&mut el.expr, changed); + walk_expr(&mut el.expr, changed, visible); } } E::Object(o) => { for p in &mut o.props { match p { - ast::PropOrSpread::Spread(sp) => walk_expr(&mut sp.expr, changed), + ast::PropOrSpread::Spread(sp) => walk_expr(&mut sp.expr, changed, visible), ast::PropOrSpread::Prop(prop) => match &mut **prop { - ast::Prop::KeyValue(kv) => walk_expr(&mut kv.value, changed), + ast::Prop::KeyValue(kv) => walk_expr(&mut kv.value, changed, visible), ast::Prop::Method(m) => { + let params = param_names(m.function.params.iter().map(|p| &p.pat)); if let Some(body) = &mut m.function.body { - fold_stmts(&mut body.stmts, changed); + fold_body(&mut body.stmts, changed, params, visible); } } ast::Prop::Getter(g) => { if let Some(body) = &mut g.body { - fold_stmts(&mut body.stmts, changed); + fold_body(&mut body.stmts, changed, Vec::new(), visible); } } ast::Prop::Setter(sst) => { + let params = param_names(std::iter::once(&*sst.param)); if let Some(body) = &mut sst.body { - fold_stmts(&mut body.stmts, changed); + fold_body(&mut body.stmts, changed, params, visible); } } _ => {} @@ -921,59 +1466,276 @@ fn walk_expr(e: &mut ast::Expr, changed: &mut bool) { } } } - E::Unary(u) => walk_expr(&mut u.arg, changed), - E::Update(u) => walk_expr(&mut u.arg, changed), + E::Unary(u) => walk_expr(&mut u.arg, changed, visible), + E::Update(u) => walk_expr(&mut u.arg, changed, visible), E::Bin(b) => { - walk_expr(&mut b.left, changed); - walk_expr(&mut b.right, changed); + walk_expr(&mut b.left, changed, visible); + walk_expr(&mut b.right, changed, visible); } - E::Assign(a) => walk_expr(&mut a.right, changed), - E::Member(m) => walk_expr(&mut m.obj, changed), + E::Assign(a) => walk_expr(&mut a.right, changed, visible), + E::Member(m) => walk_expr(&mut m.obj, changed, visible), E::Cond(c) => { - walk_expr(&mut c.test, changed); - walk_expr(&mut c.cons, changed); - walk_expr(&mut c.alt, changed); + walk_expr(&mut c.test, changed, visible); + walk_expr(&mut c.cons, changed, visible); + walk_expr(&mut c.alt, changed, visible); } E::Call(c) => { if let ast::Callee::Expr(e) = &mut c.callee { - walk_expr(e, changed); + walk_expr(e, changed, visible); } for a in &mut c.args { - walk_expr(&mut a.expr, changed); + walk_expr(&mut a.expr, changed, visible); } } E::New(n) => { - walk_expr(&mut n.callee, changed); + walk_expr(&mut n.callee, changed, visible); if let Some(args) = &mut n.args { for a in args { - walk_expr(&mut a.expr, changed); + walk_expr(&mut a.expr, changed, visible); } } } E::Seq(s) => { for e in &mut s.exprs { - walk_expr(e, changed); + walk_expr(e, changed, visible); } } E::Tpl(t) => { for e in &mut t.exprs { - walk_expr(e, changed); + walk_expr(e, changed, visible); } } - E::Paren(p) => walk_expr(&mut p.expr, changed), - E::Await(a) => walk_expr(&mut a.arg, changed), + E::Paren(p) => walk_expr(&mut p.expr, changed, visible), + E::Await(a) => walk_expr(&mut a.arg, changed, visible), E::Yield(y) => { if let Some(a) = &mut y.arg { - walk_expr(a, changed); + walk_expr(a, changed, visible); + } + } + E::TsAs(t) => walk_expr(&mut t.expr, changed, visible), + E::TsNonNull(t) => walk_expr(&mut t.expr, changed, visible), + E::TsSatisfies(t) => walk_expr(&mut t.expr, changed, visible), + _ => {} + } +} + +/// Names that resolve to a declarative binding at a point in the source, +/// innermost scope first (#10357). A read of any other name may resolve to a +/// property of the global object, and that property may be an accessor. +/// +/// Built only from declarations that cover the WHOLE region they are attached +/// to: a statement list's own declarations, a function's parameters and +/// function-scoped `var`s, a loop head, a catch parameter, and the module's +/// imports and declarations. A declaration in a sibling block is not in the +/// chain, so a same-named read outside it correctly stays unresolved. An +/// ambient `declare` binds nothing at run time and is never collected. A +/// `with` statement anywhere in the module makes every read unresolvable — +/// its object is consulted, through `has`/`get`, before any binding. +struct Visible<'p> { + names: Vec, + parent: Option<&'p Visible<'p>>, + /// Nothing resolves (`with` in the module). + opaque: bool, +} + +impl Visible<'static> { + /// Resolves only the global constants; for predicates that are never + /// consulted on an observable builder. + const ROOT: Visible<'static> = Visible { + names: Vec::new(), + parent: None, + opaque: false, + }; + + fn of_module(module: &ast::Module) -> Self { + let mut names = Vec::new(); + let mut opaque = false; + for item in &module.body { + match item { + ast::ModuleItem::ModuleDecl(ast::ModuleDecl::Import(import)) => { + for specifier in &import.specifiers { + let local = match specifier { + ast::ImportSpecifier::Named(s) => &s.local, + ast::ImportSpecifier::Default(s) => &s.local, + ast::ImportSpecifier::Namespace(s) => &s.local, + }; + names.push(local.sym.to_string()); + } + } + ast::ModuleItem::ModuleDecl(ast::ModuleDecl::ExportDecl(e)) => { + collect_decl_names(&e.decl, &mut names); + } + ast::ModuleItem::Stmt(stmt) => collect_stmt_decl_names(stmt, &mut names), + _ => {} + } + } + let stmts: Vec<&ast::Stmt> = module + .body + .iter() + .filter_map(|item| match item { + ast::ModuleItem::Stmt(s) => Some(s), + _ => None, + }) + .collect(); + for stmt in stmts { + let mut vars = VarNames::default(); + stmt.visit_with(&mut vars); + names.extend(vars.names); + } + struct WithFinder(bool); + impl Visit for WithFinder { + fn visit_with_stmt(&mut self, _: &ast::WithStmt) { + self.0 = true; } } - E::TsAs(t) => walk_expr(&mut t.expr, changed), - E::TsNonNull(t) => walk_expr(&mut t.expr, changed), - E::TsSatisfies(t) => walk_expr(&mut t.expr, changed), + let mut with = WithFinder(false); + module.visit_with(&mut with); + opaque |= with.0; + Visible { + names, + parent: None, + opaque, + } + } +} + +impl<'p> Visible<'p> { + fn child<'c>(&'c self, names: Vec) -> Visible<'c> { + Visible { + names, + parent: Some(self), + opaque: self.opaque, + } + } + + /// Does a read of `sym` here resolve to a declarative binding, or to a + /// global constant that cannot be an accessor? + fn resolves(&self, sym: &str) -> bool { + if self.opaque { + return false; + } + // Non-writable, non-configurable data properties of the global + // object: they can never become accessors. + if matches!(sym, "undefined" | "NaN" | "Infinity") { + return true; + } + let mut scope = Some(self); + while let Some(visible) = scope { + if visible.names.iter().any(|n| n == sym) { + return true; + } + scope = visible.parent; + } + false + } +} + +/// The declarations made directly by a statement list. +fn declared_names(stmts: &[ast::Stmt]) -> Vec { + let mut names = Vec::new(); + for stmt in stmts { + collect_stmt_decl_names(stmt, &mut names); + } + names +} + +fn collect_stmt_decl_names(stmt: &ast::Stmt, names: &mut Vec) { + if let ast::Stmt::Decl(decl) = stmt { + collect_decl_names(decl, names); + } +} + +fn collect_decl_names(decl: &ast::Decl, names: &mut Vec) { + match decl { + ast::Decl::Var(v) => collect_var_decl_names(v, names), + ast::Decl::Fn(f) if !f.declare => names.push(f.ident.sym.to_string()), + ast::Decl::Class(c) if !c.declare => names.push(c.ident.sym.to_string()), + ast::Decl::TsEnum(e) if !e.declare => names.push(e.id.sym.to_string()), _ => {} } } +fn collect_var_decl_names(v: &ast::VarDecl, names: &mut Vec) { + if v.declare { + return; + } + for d in &v.decls { + collect_pat_names(&d.name, names); + } +} + +fn for_head_names(head: &ast::ForHead) -> Vec { + let mut names = Vec::new(); + if let ast::ForHead::VarDecl(v) = head { + collect_var_decl_names(v, &mut names); + } + names +} + +fn param_names<'a>(pats: impl Iterator) -> Vec { + let mut names = Vec::new(); + for pat in pats { + collect_pat_names(pat, &mut names); + } + names +} + +/// Binding names of a pattern. Default values are not walked: a closure in a +/// default binds nothing in this scope. +fn collect_pat_names(pat: &ast::Pat, names: &mut Vec) { + match pat { + ast::Pat::Ident(bi) => names.push(bi.id.sym.to_string()), + ast::Pat::Array(a) => { + for elem in a.elems.iter().flatten() { + collect_pat_names(elem, names); + } + } + ast::Pat::Rest(r) => collect_pat_names(&r.arg, names), + ast::Pat::Object(o) => { + for prop in &o.props { + match prop { + ast::ObjectPatProp::KeyValue(kv) => collect_pat_names(&kv.value, names), + ast::ObjectPatProp::Assign(a) => names.push(a.key.id.sym.to_string()), + ast::ObjectPatProp::Rest(r) => collect_pat_names(&r.arg, names), + } + } + } + ast::Pat::Assign(a) => collect_pat_names(&a.left, names), + ast::Pat::Invalid(_) | ast::Pat::Expr(_) => {} + } +} + +/// Function-scoped `var` names anywhere in a body, not inside a nested +/// function-like. +fn var_names(stmts: &[ast::Stmt]) -> Vec { + let mut vars = VarNames::default(); + stmts.visit_with(&mut vars); + vars.names +} + +#[derive(Default)] +struct VarNames { + names: Vec, +} + +impl Visit for VarNames { + fn visit_var_decl(&mut self, v: &ast::VarDecl) { + if v.kind == ast::VarDeclKind::Var { + collect_var_decl_names(v, &mut self.names); + } + v.visit_children_with(self); + } + fn visit_function(&mut self, _: &ast::Function) {} + fn visit_arrow_expr(&mut self, _: &ast::ArrowExpr) {} + fn visit_constructor(&mut self, _: &ast::Constructor) {} + fn visit_getter_prop(&mut self, _: &ast::GetterProp) {} + fn visit_setter_prop(&mut self, _: &ast::SetterProp) {} + fn visit_class_prop(&mut self, _: &ast::ClassProp) {} + fn visit_private_prop(&mut self, _: &ast::PrivateProp) {} + fn visit_auto_accessor(&mut self, _: &ast::AutoAccessor) {} + fn visit_static_block(&mut self, _: &ast::StaticBlock) {} +} + // --------------------------------------------------------------------------- // #6812 (w16): compile-time builder WIDTH scan. // diff --git a/crates/perry-hir/tests/builder_fold_conversion.rs b/crates/perry-hir/tests/builder_fold_conversion.rs new file mode 100644 index 0000000000..dc41f2e9a2 --- /dev/null +++ b/crates/perry-hir/tests/builder_fold_conversion.rs @@ -0,0 +1,561 @@ +//! #10357: the builder fold evaluates a value BEFORE the builder binding is +//! initialized, and an implicit conversion (`"" + w`, `-w`, `` `${w}` ``) can +//! call a user `valueOf`/`toString`. When that user code can read the binding +//! the fold turns a successful read into a TDZ `ReferenceError`, so such a +//! value must not fold. When nothing can read the binding before the literal +//! initializes it, the conversion is unobservable and must keep folding — +//! these tests pin both directions. + +use perry_diagnostics::SourceCache; +use perry_hir::{lower_module, Expr, Stmt}; +use perry_parser::parse_typescript_with_cache; + +fn lower_src(src: &str) -> perry_hir::Module { + let mut cache = SourceCache::new(); + let parsed = parse_typescript_with_cache(src, "builder_fold_conversion.ts", &mut cache) + .expect("parse should succeed"); + lower_module(&parsed.module, "test", "builder_fold_conversion.ts") + .expect("lower should succeed") +} + +/// Number of constructor args on the `__AnonShape_…` allocation bound to +/// `name`, i.e. how many properties the literal carries after folding. +fn anon_shape_arity(stmts: &[Stmt], name: &str) -> Option { + stmts.iter().find_map(|stmt| match stmt { + Stmt::Let { + name: binding, + init: Some(Expr::New { + class_name, args, .. + }), + .. + } if binding == name && class_name.starts_with("__AnonShape_") => Some(args.len()), + _ => None, + }) +} + +/// Every statement list in the module, depth-first: the builder under test +/// may sit in module init, a function body, or a block nested in either. +fn all_stmt_lists(module: &perry_hir::Module) -> Vec<&[Stmt]> { + fn nested<'a>(stmts: &'a [Stmt], out: &mut Vec<&'a [Stmt]>) { + out.push(stmts); + for stmt in stmts { + match stmt { + Stmt::If { + then_branch, + else_branch, + .. + } => { + nested(then_branch, out); + if let Some(else_branch) = else_branch { + nested(else_branch, out); + } + } + Stmt::Switch { cases, .. } => { + for case in cases { + nested(&case.body, out); + } + } + Stmt::While { body, .. } | Stmt::DoWhile { body, .. } | Stmt::For { body, .. } => { + nested(body, out) + } + Stmt::Try { + body, + catch, + finally, + } => { + nested(body, out); + if let Some(catch) = catch { + nested(&catch.body, out); + } + if let Some(finally) = finally { + nested(finally, out); + } + } + _ => {} + } + } + } + let mut out = Vec::new(); + nested(&module.init, &mut out); + for function in &module.functions { + nested(&function.body, &mut out); + } + out +} + +fn arity_anywhere(module: &perry_hir::Module, name: &str) -> Option { + all_stmt_lists(module) + .into_iter() + .find_map(|stmts| anon_shape_arity(stmts, name)) +} + +#[test] +fn conversions_keep_folding_when_nothing_can_observe_the_builder() { + // The #6812 motivating shape: no closure anywhere names `o`, so no user + // code a conversion reaches can read it early. + let module = lower_src( + r#" + export function build(r: number, i: number): any { + const o: any = {}; + o.a = i; o.b = r + i; o.c = `k${i}`; o.d = -r; + return o; + } + "#, + ); + assert_eq!( + arity_anywhere(&module, "o"), + Some(4), + "every conversion should still fold: {:?}", + module.functions + ); +} + +#[test] +fn a_conversion_does_not_fold_when_a_closure_can_read_the_builder() { + // The #10357 repro: `weird.valueOf()` reads `o`. + let module = lower_src( + r#" + export function run(): string { + const weird = { valueOf(): any { return o; } }; + const o: any = {}; + o.a = "" + weird; + return typeof o.a; + } + "#, + ); + assert_eq!( + arity_anywhere(&module, "o"), + Some(0), + "the conversion must stay after the allocation: {:?}", + module.functions + ); +} + +#[test] +fn conversion_free_values_still_fold_on_an_observable_builder() { + let module = lower_src( + r#" + export function run(w: any): any { + const peek = () => o; + const o: any = {}; + o.a = 1 + 2; o.b = typeof w; o.c = !w; o.d = w === 1; o.e = `lit`; o.f = w; + return peek; + } + "#, + ); + assert_eq!( + arity_anywhere(&module, "o"), + Some(6), + "values that run no conversion on an unknown operand should fold: {:?}", + module.functions + ); +} + +#[test] +fn each_converting_form_stops_the_fold_on_an_observable_builder() { + for value in [ + "-w", + "+w", + "~w", + "`${w}`", + "w + 1", + "w < 1", + "w == 1", + "(w && 1) + 1", + ] { + let module = lower_src(&format!( + r#" + export function run(w: any): any {{ + const peek = () => o; + const o: any = {{}}; + o.a = {value}; + return peek; + }} + "# + )); + assert_eq!( + arity_anywhere(&module, "o"), + Some(0), + "`o.a = {value}` must not fold when a closure reads `o`: {:?}", + module.functions + ); + } +} + +#[test] +fn a_gap_statement_with_a_conversion_does_not_sink_an_observable_allocation() { + // #10355's gap test shares the predicate: `"" + w` below would run + // `w.valueOf()` before `o` exists. + let module = lower_src( + r#" + export function run(w: any): any { + const peek = () => o; + const o: any = {}; + const s = "" + w; + o.a = s; + return peek; + } + "#, + ); + assert_eq!( + arity_anywhere(&module, "o"), + Some(0), + "the allocation must not sink below the conversion: {:?}", + module.functions + ); +} + +#[test] +fn a_closure_in_another_switch_case_observes_the_builder() { + // A `const` in a case clause is scoped to the whole switch, so a closure + // made in an earlier case can fall through into the builder and read it: + // the scan must cover the enclosing function, not just the statement + // list being folded. + let module = lower_src( + r#" + export function run(k: number, w: any): any { + let f: any; + switch (k) { + case 2: + f = () => o; + case 1: + const o: any = {}; + o.a = "" + w; + break; + } + return f; + } + "#, + ); + assert_eq!( + arity_anywhere(&module, "o"), + Some(0), + "a sibling-case closure must count: {:?}", + module.functions + ); +} + +#[test] +fn a_block_var_builder_is_observed_by_a_function_level_closure() { + let module = lower_src( + r#" + export function run(w: any): any { + if (w) { + var o: any = {}; + o.a = "" + w; + } + return () => o; + } + "#, + ); + assert_eq!( + arity_anywhere(&module, "o"), + Some(0), + "a `var` is function-scoped, so the closure outside the block counts: {:?}", + module.functions + ); +} + +#[test] +fn an_exported_top_level_builder_is_observable() { + let module = lower_src( + r#" + declare const w: any; + const o: any = {}; + o.a = "" + w; + export { o }; + "#, + ); + assert_eq!( + anon_shape_arity(&module.init, "o"), + Some(0), + "an importer can read the live binding: {:?}", + module.init + ); +} + +#[test] +fn a_top_level_var_builder_is_observable() { + let module = lower_src( + r#" + declare const w: any; + var o: any = {}; + o.a = "" + w; + "#, + ); + assert_eq!( + anon_shape_arity(&module.init, "o"), + Some(0), + "a top-level `var` is a global-object property: {:?}", + module.init + ); +} + +#[test] +fn an_unobserved_top_level_const_builder_still_folds() { + let module = lower_src( + r#" + declare const w: any; + const o: any = {}; + o.a = "" + w; + console.log(o.a); + "#, + ); + assert_eq!( + anon_shape_arity(&module.init, "o"), + Some(1), + "nothing can read `o` before the literal: {:?}", + module.init + ); +} + +#[test] +fn eval_makes_every_builder_in_its_scope_observable() { + let module = lower_src( + r#" + export function run(w: any): any { + const o: any = {}; + o.a = "" + w; + return eval("o"); + } + "#, + ); + assert_eq!( + arity_anywhere(&module, "o"), + Some(0), + "name-based reasoning is void once `eval` is in scope: {:?}", + module.functions + ); +} + +#[test] +fn a_closure_created_after_a_const_builder_does_not_observe_it() { + // It cannot exist yet while the folded values run. + let module = lower_src( + r#" + export function build(r: number, i: number): any { + const o: any = {}; + o.a = i; o.b = r + i; o.c = -r; + const peek = () => o; + return peek(); + } + "#, + ); + assert_eq!( + arity_anywhere(&module, "o"), + Some(3), + "a later closure is no observer: {:?}", + module.functions + ); +} + +#[test] +fn a_hoisted_function_declared_after_the_builder_observes_it() { + let module = lower_src( + r#" + export function run(w: any): any { + const o: any = {}; + o.a = "" + w; + return peek; + function peek(): any { return o; } + } + "#, + ); + assert_eq!( + arity_anywhere(&module, "o"), + Some(0), + "a function declaration exists from scope entry: {:?}", + module.functions + ); +} + +#[test] +fn a_var_builder_is_observed_by_a_closure_from_an_earlier_loop_pass() { + // The `var` binding is shared by every pass, so the closure pushed after + // the builder in pass 0 can run during pass 1's fold. + let module = lower_src( + r#" + export function run(w: any): any { + const fns: any[] = []; + for (let k = 0; k < 2; k++) { + var o: any = {}; + o.a = "" + w; + fns.push(() => o); + } + return fns; + } + "#, + ); + assert_eq!( + arity_anywhere(&module, "o"), + Some(0), + "position says nothing for a `var`: {:?}", + module.functions + ); +} + +#[test] +fn a_nested_function_with_its_own_binding_is_not_an_observer() { + let module = lower_src( + r#" + export function outer(x: number): any { + function inner(y: number): any { + const o: any = {}; + o.a = y + 1; + return o; + } + const peek = (o: any) => o; + const o: any = {}; + o.a = x + 1; + o.n = inner(x); + return peek(o); + } + "#, + ); + let folded: Vec = all_stmt_lists(&module) + .into_iter() + .filter_map(|stmts| anon_shape_arity(stmts, "o")) + .collect(); + assert!( + folded.contains(&1) && !folded.contains(&0), + "both builders should fold, since `inner` and `peek` bind their own `o`: {folded:?} {:?}", + module.functions + ); +} + +#[test] +fn a_body_declaration_does_not_shadow_a_parameter_default() { + // The default is evaluated in its own scope and reads the OUTER `o`. + let module = lower_src( + r#" + export function run(w: any): any { + const peek = (a = o) => { var o; return a; }; + const o: any = {}; + o.a = "" + w; + return peek; + } + "#, + ); + assert_eq!( + arity_anywhere(&module, "o"), + Some(0), + "the default parameter observes the builder: {:?}", + module.functions + ); +} + +#[test] +fn a_block_level_redeclaration_does_not_shadow() { + let module = lower_src( + r#" + export function run(w: any): any { + const peek = () => { { const o = 1; } return o; }; + const o: any = {}; + o.a = "" + w; + return peek; + } + "#, + ); + assert_eq!( + arity_anywhere(&module, "o"), + Some(0), + "only function-level bindings shadow: {:?}", + module.functions + ); +} + +#[test] +fn a_global_read_does_not_fold_on_an_observable_builder() { + // `gObs` is no declared binding, so it may be an accessor on the global + // object — a getter that reads `o` before it is initialized. + let module = lower_src( + r#" + export function run(): any { + const peek = () => o; + const o: any = {}; + o.a = gObs; + return peek; + } + "#, + ); + assert_eq!( + arity_anywhere(&module, "o"), + Some(0), + "an undeclared identifier may run a global getter: {:?}", + module.functions + ); +} + +#[test] +fn reads_of_declared_bindings_still_fold_on_an_observable_builder() { + let module = lower_src( + r#" + const top = 1; + export function run(w: any): any { + var fnVar = 2; + const peek = () => o; + for (let i = 0; i < 1; i++) { + try { + throw 0; + } catch (err) { + const o: any = {}; + o.a = w; o.b = top; o.c = fnVar; o.d = i; o.e = err; o.f = undefined; + return peek; + } + } + } + "#, + ); + assert_eq!( + arity_anywhere(&module, "o"), + Some(6), + "parameters, outer and loop-head bindings, catch parameters and `undefined` run no code: {:?}", + module.functions + ); +} + +#[test] +fn an_ambient_declaration_is_not_a_binding() { + // `declare` binds nothing at run time: the read still goes to the global + // object (#10363 tracks perry materializing it today). + let module = lower_src( + r#" + declare const gAmb: any; + export function run(): any { + const peek = () => o; + const o: any = {}; + o.a = gAmb; + return peek; + } + "#, + ); + assert_eq!( + arity_anywhere(&module, "o"), + Some(0), + "`declare const` must not count as a binding: {:?}", + module.functions + ); +} + +#[test] +fn a_declaration_in_a_sibling_block_does_not_resolve_the_read() { + let module = lower_src( + r#" + export function run(): any { + const peek = () => o; + { + const gSib = 1; + } + const o: any = {}; + o.a = gSib; + return peek; + } + "#, + ); + assert_eq!( + arity_anywhere(&module, "o"), + Some(0), + "`gSib` here is the global, not the block's binding: {:?}", + module.functions + ); +} diff --git a/crates/perry/tests/builder_fold_conversion_semantics.rs b/crates/perry/tests/builder_fold_conversion_semantics.rs new file mode 100644 index 0000000000..76528a6586 --- /dev/null +++ b/crates/perry/tests/builder_fold_conversion_semantics.rs @@ -0,0 +1,195 @@ +//! #10357: an implicit conversion in a folded builder value can call user +//! code. These are the observable consequences — the HIR tests in +//! `perry-hir/tests/builder_fold_conversion.rs` pin which shapes fold, this +//! file pins that the program behaves like node either way. + +use std::path::PathBuf; +use std::process::Command; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +fn compile_and_run(source: &str) -> String { + let dir = tempfile::tempdir().expect("tempdir"); + let entry = dir.path().join("main.ts"); + let output = dir.path().join("main_bin"); + std::fs::write(&entry, source).expect("write entry"); + + let compile = Command::new(perry_bin()) + .current_dir(dir.path()) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .arg("--no-cache") + .output() + .expect("run perry compile"); + assert!( + compile.status.success(), + "perry compile failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + + let run = Command::new(&output) + .current_dir(dir.path()) + .output() + .expect("run compiled binary"); + assert!( + run.status.success(), + "compiled binary failed (exit {:?})\nstdout:\n{}\nstderr:\n{}", + run.status.code(), + String::from_utf8_lossy(&run.stdout), + String::from_utf8_lossy(&run.stderr) + ); + String::from_utf8_lossy(&run.stdout).into_owned() +} + +#[test] +fn a_value_of_that_reads_the_builder_sees_the_allocated_object() { + // The #10357 repro: node prints `string:str`; the fold used to move + // `weird.valueOf()` into `o`'s TDZ. + let stdout = compile_and_run( + r#" +function run(): string { + const weird = { valueOf(): any { return o; } }; + const o: any = {}; + o.a = "" + weird; + return typeof o.a + ":" + (o.a === "[object Object]" ? "str" : "other"); +} +try { console.log(run()); } catch (e: any) { console.log("threw " + e.name); } +"#, + ); + assert_eq!(stdout, "string:str\n"); +} + +#[test] +fn every_converting_form_sees_the_allocated_object() { + let stdout = compile_and_run( + r#" +function probe(kind: string): string { + const seen: string[] = []; + const w = { + valueOf(): any { seen.push(typeof o); return 1; }, + toString(): string { seen.push(typeof o); return "w"; }, + }; + const o: any = {}; + if (kind === "neg") o.a = -(w as any); + else if (kind === "lt") o.a = (w as any) < 2; + else if (kind === "tpl") o.a = `${w}`; + else o.a = "" + w; + return kind + "=" + seen.join(","); +} +for (const kind of ["plus", "neg", "lt", "tpl"]) { + try { console.log(probe(kind)); } catch (e: any) { console.log(kind + " threw " + e.name); } +} +"#, + ); + assert_eq!(stdout, "plus=object\nneg=object\nlt=object\ntpl=object\n"); +} + +#[test] +fn a_conversion_in_a_gap_statement_sees_the_allocated_object() { + let stdout = compile_and_run( + r#" +function run(): string { + const w = { toString(): string { return typeof o; } }; + const o: any = {}; + const s = `${w}`; + o.a = s; + return o.a; +} +try { console.log(run()); } catch (e: any) { console.log("threw " + e.name); } +"#, + ); + assert_eq!(stdout, "object\n"); +} + +#[test] +fn unobserved_conversions_still_build_the_same_object() { + let stdout = compile_and_run( + r#" +function build(r: number, i: number): any { + const o: any = {}; + o.a = i; o.b = r + i; o.c = `k${i}`; o.d = -r; o.e = r < i; + return o; +} +console.log(JSON.stringify(build(3, 4)), Object.keys(build(1, 2)).join(",")); +"#, + ); + assert_eq!( + stdout, + "{\"a\":4,\"b\":7,\"c\":\"k4\",\"d\":-3,\"e\":true} a,b,c,d,e\n" + ); +} + +#[test] +fn loop_passes_see_the_right_binding() { + // `var`: the closure from pass 0 reads the SHARED binding, which the + // unfolded pass 1 has already pointed at a fresh object. `let`: each pass + // has its own binding, so pass 0's closure reads pass 0's object either + // way. + let stdout = compile_and_run( + r#" +function viaVar(): string { + const fns: Array<() => any> = []; + const seen: string[] = []; + for (let k = 0; k < 2; k++) { + const w = { valueOf(): any { if (fns.length) seen.push(fns[0]().v === undefined ? "fresh" : "old"); return k; } }; + var o: any = {}; + o.v = -(w as any); + fns.push(() => o); + } + return seen.join(","); +} +function viaLet(): string { + const fns: Array<() => any> = []; + const seen: string[] = []; + for (let k = 0; k < 2; k++) { + const w = { valueOf(): any { if (fns.length) seen.push(fns[0]().v === undefined ? "fresh" : "old"); return k; } }; + let o: any = {}; + o.v = -(w as any); + fns.push(() => o); + } + return seen.join(","); +} +try { console.log("var=" + viaVar()); } catch (e: any) { console.log("var threw " + e.name); } +try { console.log("let=" + viaLet()); } catch (e: any) { console.log("let threw " + e.name); } +"#, + ); + assert_eq!(stdout, "var=fresh\nlet=old\n"); +} + +#[test] +fn a_closure_created_after_the_builder_still_reads_the_built_object() { + let stdout = compile_and_run( + r#" +function build(r: number, i: number): any { + const o: any = {}; + o.a = i; o.b = r + i; o.c = -r; + const peek = () => o; + return peek(); +} +console.log(JSON.stringify(build(2, 5))); +"#, + ); + assert_eq!(stdout, "{\"a\":5,\"b\":7,\"c\":-2}\n"); +} + +#[test] +fn a_global_getter_that_reads_the_builder_sees_the_allocated_object() { + let stdout = compile_and_run( + r#" +// @ts-nocheck +function run() { + Object.defineProperty(globalThis, "gObs", { get() { return typeof o; }, configurable: true }); + const o = {}; + o.a = gObs; + return String(o.a); +} +try { console.log(run()); } catch (e) { console.log("threw " + e.name); } +"#, + ); + assert_eq!(stdout, "object\n"); +}