diff --git a/packages/compiler/src/frontend/shape-inference/static-objects.ts b/packages/compiler/src/frontend/shape-inference/static-objects.ts index 7093df28d..bf58186dc 100644 --- a/packages/compiler/src/frontend/shape-inference/static-objects.ts +++ b/packages/compiler/src/frontend/shape-inference/static-objects.ts @@ -733,6 +733,67 @@ export function planStaticObjectRewrites( const call = requireValue(v); return call !== null && requiredFiles(sf, call).includes(target); }); + /** Whether `node` sits inside a function of its top-level statement. */ + const insideFunction = (node: Node): boolean => { + const top = topStatement(node); + for (let p: Node | undefined = node.parent; p !== undefined && p !== top; p = p.parent) + if (ts.isFunctionLike(p)) return true; + return false; + }; + /** The nodes whose statements must have run for an effect-free receiver + * (effectFree) to be the constant object it reads as: each member read's + * stores of that member, and a name's declaration. */ + const receiverDeps = (e: ts.Expression): Node[] => { + while (ts.isParenthesizedExpression(e)) e = e.expression; + if (ts.isIdentifier(e)) { + const d = declOf(e); + return d === undefined ? [] : [d]; + } + if (!ts.isPropertyAccessExpression(e) && !ts.isElementAccessExpression(e)) return []; + const op = readOps.get(e); + if (op === undefined || op.name === null) return []; + const objects = analysis.nodes[op.recv]!.atoms; + const stores = analysis.ops + .filter( + (w) => + w.kind === "write" && + w.name === op.name && + [...analysis.nodes[w.recv]!.atoms].some((a) => objects.has(a)), + ) + .map((w) => w.node); + return [...stores, ...receiverDeps(e.expression)]; + }; + /** Whether `read` can only run after the statements holding `deps`: what + * it reads exists by then, so a rewrite that no longer evaluates the read + * cannot drop the TypeError or ReferenceError of an earlier read. In + * another file, the defining module has finished loading unless it + * requires the reading file. In the same file, a top-level read comes + * after them. A read inside a function runs when the function is called: + * the statements from the one that creates the function (the first, for + * a hoisted declaration) to the last of them must only define things. */ + const createdBefore = (read: Node, deps: readonly Node[]): boolean => { + const sf = read.getSourceFile(); + const stmts = sf.statements.nodes; + let last = -1; + for (const d of deps) { + const file = d.getSourceFile(); + if (file !== sf) { + if (requiredClosure(file).has(sf)) return false; + continue; + } + const at = stmts.indexOf(topStatement(d)!); + if (at < 0) return false; + last = Math.max(last, at); + } + if (last < 0) return true; + const top = topStatement(read)!; + if (!insideFunction(read)) return stmts.indexOf(top) > last; + let fn: Node | undefined; + for (let p: Node | undefined = read.parent; p !== undefined && p !== top; p = p.parent) + if (ts.isFunctionLike(p)) fn = p; + const from = fn !== undefined && ts.isFunctionDeclaration(fn) ? 0 : stmts.indexOf(top); + return stmts.slice(from, last + 1).every(definitionOnly); + }; const moduleBindings = new Map(); const exportAdds = new Map>(); const crossFileReference = ( @@ -759,11 +820,7 @@ export function planStaticObjectRewrites( const at = stmts.findIndex((st) => requiresWhenRun(sf, st, defining)); if (at < 0) return null; const top = topStatement(node)!; - const inFunction = ((): boolean => { - for (let p: Node | undefined = node.parent; p !== undefined && p !== top; p = p.parent) - if (ts.isFunctionLike(p)) return true; - return false; - })(); + const inFunction = insideFunction(node); if (!inFunction && stmts.indexOf(top) <= at) return null; // A function may run before the binding below is initialized if the // statement that loads the module, or one before it, runs other code @@ -829,6 +886,10 @@ export function planStaticObjectRewrites( kept.push(`reference ${node.getText()}: the receiver may have effects`); continue; } + if (!createdBefore(node, [field.values[0]!.node!, ...receiverDeps(node.expression)])) { + kept.push(`reference ${node.getText()}: it may run before the member is stored`); + continue; + } if (node.getSourceFile() !== field.values[0]!.node!.getSourceFile()) { const text = crossFileReference(node, field.values[0]!.node!.getSourceFile(), name); if (text === null) continue; @@ -871,6 +932,10 @@ export function planStaticObjectRewrites( kept.push(`constant read ${node.getText()}: the receiver may have effects`); continue; } + if (!createdBefore(node, [o.site.node, ...receiverDeps(node.expression)])) { + kept.push(`constant read ${node.getText()}: it may run before the object is created`); + continue; + } callEdits.push({ sf: node.getSourceFile(), edit: { start: ts.tokenStartOfNode(node), end: node.end, text: `(${literal})` }, @@ -950,6 +1015,11 @@ export function planStaticObjectRewrites( kept.push(`${where}: a member is not a hoisted function of this file`); continue; } + const armNodes = o.fields.map((f) => f.values[0]!.node!); + if (!createdBefore(call, [o.site.node, ...armNodes, ...receiverDeps(node.expression)])) { + kept.push(`${where}: it may run before the table is created`); + continue; + } tableCalls.push({ sf: call.getSourceFile(), call, arms }); } for (const { sf, call, arms } of tableCalls) { diff --git a/tests/fixtures/npm/cases/static-objects-order/js-main.cjs b/tests/fixtures/npm/cases/static-objects-order/js-main.cjs index 5d10567c0..c18877ab5 100644 --- a/tests/fixtures/npm/cases/static-objects-order/js-main.cjs +++ b/tests/fixtures/npm/cases/static-objects-order/js-main.cjs @@ -8,6 +8,7 @@ const early = require("static-objects-order/early-user.js"); console.log("start"); console.log(receivers.ns.run("add"), receivers.ns.run("sub"), receivers.ns.reads()); console.log(receivers.ns.proto()); +console.log(receivers.ns.early()); console.log(lazy.greet("a"), lazy.greet("b")); console.log(vec.len2(3, 4)); console.log(early.early()); diff --git a/tests/fixtures/npm/cases/static-objects-order/main.ts b/tests/fixtures/npm/cases/static-objects-order/main.ts index 490407232..55bd19d7e 100644 --- a/tests/fixtures/npm/cases/static-objects-order/main.ts +++ b/tests/fixtures/npm/cases/static-objects-order/main.ts @@ -8,6 +8,7 @@ import { early } from "static-objects-order/early-user.js"; console.log("start"); console.log(ns.run("add"), ns.run("sub"), ns.reads()); console.log(ns.proto()); +console.log(ns.early()); console.log(greet("a"), greet("b")); console.log(len2(3, 4)); console.log(early()); diff --git a/tests/fixtures/npm/cases/static-objects-order/node_modules/static-objects-order/receivers.js b/tests/fixtures/npm/cases/static-objects-order/node_modules/static-objects-order/receivers.js index 4925d023b..816212f65 100644 --- a/tests/fixtures/npm/cases/static-objects-order/node_modules/static-objects-order/receivers.js +++ b/tests/fixtures/npm/cases/static-objects-order/node_modules/static-objects-order/receivers.js @@ -1,6 +1,28 @@ // Constant members, a function table and a namespace function read through // receivers with side effects: each receiver still runs. var ns = ns || {}; +// Reads that run before their members are created: Node throws. +var early = (function () { + var out = []; + try { + out.push(ns.Late.X); + } catch (e) { + out.push(e.constructor.name); + } + try { + out.push(ns.later()); + } catch (e) { + out.push(e.constructor.name); + } + return out.join(" "); +})(); +ns.Late = { X: 5 }; +ns.later = function () { + return "later"; +}; +ns.early = function () { + return early + " " + ns.Late.X + " " + ns.later(); +}; ns.Kind = { A: 1, B: 2 }; ns.ops = { add: function (a, b) { diff --git a/tests/harness/static-objects-order.test.ts b/tests/harness/static-objects-order.test.ts index 1c44aa0b4..d732d06a3 100644 --- a/tests/harness/static-objects-order.test.ts +++ b/tests/harness/static-objects-order.test.ts @@ -2,10 +2,11 @@ * rewrites keep what the program evaluates and when its modules load. With * the shape overlay on, the npm case static-objects-order (through its * JavaScript consumer, js-main.cjs) keeps receivers with effects as - * written, binds a module where the program loads it and only for reads - * that run after the binding, never adds a hoisted class to an exports - * literal that runs before the class's declaration, does not fold a - * literal's `__proto__`, and prints what Node prints. A module in a require cycle through another + * written, keeps reads that may run before what they read exists, binds a + * module where the program loads it and only for reads that run after the + * binding, never adds a hoisted class to an exports literal that runs + * before the class's declaration, does not fold a literal's `__proto__`, + * and prints what Node prints. A module in a require cycle through another * module is not read through its exports from the cycle. The rewritten * files are read from the overlay's debug output * (SCRIPTC_DEBUG_SHAPE_OVERLAY). */ @@ -83,6 +84,11 @@ test( "kept table call (reads++, ns.ops)[op]: the table expression may have effects", ); // vec.js assigns its exports literal before the constructor function. + // receivers.js reads ns.Late.X and calls ns.later() before they exist. + expect(debug).toContain( + "kept constant read ns.Late.X: it may run before the object is created", + ); + expect(debug).toContain("kept reference ns.later: it may run before the member is stored"); // lazy-user.js's first mention of lazy.js is a conditional require: the // binding follows the statement that loads the module each time it runs. expect(rewritten(debug, join(pkg, "lazy-user.js"))).toMatch(