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/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 936b51ee74..549c205573 100644 --- a/crates/perry-hir/src/lower/builder_fold.rs +++ b/crates/perry-hir/src/lower/builder_fold.rs @@ -15,9 +15,36 @@ //! - 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. +//! - #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. @@ -40,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}; @@ -47,15 +75,23 @@ 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 { if !module_has_candidate(module) || module_mutates_object_prototype_descriptors(module) { return None; } + let scope = FoldScope::of_module(module); + let visible = Visible::of_module(module); let mut folded = module.clone(); let mut changed = false; - process_module_items(&mut folded.body, &mut changed); + process_module_items(&mut folded.body, &mut changed, &scope, &visible); changed.then_some(folded) } @@ -155,17 +191,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(), 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; } } module.body.iter().any(|item| match item { @@ -177,11 +221,18 @@ 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(), 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()) + { + return true; } } stmts.iter().any(scan_stmt) @@ -320,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() { @@ -331,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; } @@ -349,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)) = ({ @@ -368,17 +429,38 @@ 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, observable, visible, |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; }; 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 { @@ -392,21 +474,40 @@ 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; } } -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]) { @@ -419,13 +520,21 @@ 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, observable, visible, |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; }; - 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 { @@ -437,13 +546,25 @@ 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; } for s in stmts.iter_mut() { - walk_stmt(s, changed); + walk_stmt(s, changed, scope, visible); } } @@ -500,6 +621,76 @@ 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, + 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, observable, visible) + { + 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 — 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, + 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, 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, observable, visible)) + }), + _ => 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 { @@ -569,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 @@ -626,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); } } _ => {} @@ -678,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::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::ForOf(f) => { + let visible = &visible.child(for_head_names(&f.left)); + walk_stmt(&mut f.body, changed, scope, visible); + } + 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); } } _ => {} @@ -783,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-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_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"); +} 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"); +}