From b242a2d09ad4fe704bf09de79d37f2abfb641264 Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Mon, 26 Oct 2026 16:45:40 -0500 Subject: [PATCH] Address review feedback --- .../shape-inference/static-objects.ts | 45 ++++++++++++++++++- .../cases/static-objects-order/js-main.cjs | 2 + .../npm/cases/static-objects-order/main.ts | 2 + .../static-objects-order/coerce-user.d.ts | 1 + .../static-objects-order/coerce-user.js | 6 +++ .../static-objects-order/coerce.js | 17 +++++++ tests/harness/static-objects-order.test.ts | 6 +++ 7 files changed, 77 insertions(+), 2 deletions(-) create mode 100644 tests/fixtures/npm/cases/static-objects-order/node_modules/static-objects-order/coerce-user.d.ts create mode 100644 tests/fixtures/npm/cases/static-objects-order/node_modules/static-objects-order/coerce-user.js create mode 100644 tests/fixtures/npm/cases/static-objects-order/node_modules/static-objects-order/coerce.js diff --git a/packages/compiler/src/frontend/shape-inference/static-objects.ts b/packages/compiler/src/frontend/shape-inference/static-objects.ts index bf8835b19..25c0206c9 100644 --- a/packages/compiler/src/frontend/shape-inference/static-objects.ts +++ b/packages/compiler/src/frontend/shape-inference/static-objects.ts @@ -864,10 +864,46 @@ export function planStaticObjectRewrites( if (recv.size !== 1 || !constantAtoms.has([...recv][0]!)) return false; return pureReceiver(target.expression, target); }; + /** Whether evaluating `n` itself may run code of the program: converting + * an operand to a primitive or a property key (an arithmetic, relational or + * loose-equality operator, a compound assignment, `++`, a template + * substitution, a computed key) calls its valueOf, toString or + * Symbol.toPrimitive; `in`, `instanceof` and `delete` run traps and + * Symbol.hasInstance; spreading and destructuring run iterators and + * getters; a class runs its computed keys, static initializers and blocks. */ + const runsImplicitly = (n: Node): boolean => { + if (ts.isBinaryExpression(n)) { + const op = n.operatorToken.kind; + if (op === ts.SyntaxKind.EqualsToken) + return ts.isObjectLiteralExpression(n.left) || ts.isArrayLiteralExpression(n.left); + return ( + op !== ts.SyntaxKind.EqualsEqualsEqualsToken && + op !== ts.SyntaxKind.ExclamationEqualsEqualsToken && + op !== ts.SyntaxKind.AmpersandAmpersandToken && + op !== ts.SyntaxKind.BarBarToken && + op !== ts.SyntaxKind.QuestionQuestionToken && + op !== ts.SyntaxKind.CommaToken + ); + } + if (ts.isPrefixUnaryExpression(n)) return n.operator !== ts.SyntaxKind.ExclamationToken; + return ( + ts.isPostfixUnaryExpression(n) || + ts.isDeleteExpression(n) || + ts.isAwaitExpression(n) || + ts.isYieldExpression(n) || + ts.isTemplateExpression(n) || + ts.isSpreadElement(n) || + ts.isSpreadAssignment(n) || + ts.isComputedPropertyName(n) || + ts.isObjectBindingPattern(n) || + ts.isArrayBindingPattern(n) || + ts.isClassExpression(n) + ); + }; /** Whether a statement only defines things: function declarations, and * declarations and stores whose evaluation runs no code outside function * bodies (no call, no read that could run a getter, no store that could run - * a setter). */ + * a setter, no implicit conversion or other implicit call). */ const definitionOnly = (stmt: ts.Statement): boolean => { if (ts.isFunctionDeclaration(stmt)) return true; if (!ts.isExpressionStatement(stmt) && !ts.isVariableStatement(stmt)) return false; @@ -875,7 +911,12 @@ export function planStaticObjectRewrites( const visit = (n: Node): void => { if (runs) return; if (ts.isFunctionLike(n)) return; - if (ts.isCallExpression(n) || ts.isNewExpression(n) || ts.isTaggedTemplateExpression(n)) + if ( + ts.isCallExpression(n) || + ts.isNewExpression(n) || + ts.isTaggedTemplateExpression(n) || + runsImplicitly(n) + ) runs = true; if (ts.isPropertyAccessExpression(n) || ts.isElementAccessExpression(n)) { const p = n.parent; 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 c18877ab5..56da826ad 100644 --- a/tests/fixtures/npm/cases/static-objects-order/js-main.cjs +++ b/tests/fixtures/npm/cases/static-objects-order/js-main.cjs @@ -4,6 +4,7 @@ const receivers = require("static-objects-order/receivers.js"); const lazy = require("static-objects-order/lazy-user.js"); const vec = require("static-objects-order/vec-user.js"); const early = require("static-objects-order/early-user.js"); +const coerce = require("static-objects-order/coerce-user.js"); console.log("start"); console.log(receivers.ns.run("add"), receivers.ns.run("sub"), receivers.ns.reads()); @@ -12,3 +13,4 @@ console.log(receivers.ns.early()); console.log(lazy.greet("a"), lazy.greet("b")); console.log(vec.len2(3, 4)); console.log(early.early()); +console.log(coerce.area(3)); diff --git a/tests/fixtures/npm/cases/static-objects-order/main.ts b/tests/fixtures/npm/cases/static-objects-order/main.ts index 55bd19d7e..fb0e64e76 100644 --- a/tests/fixtures/npm/cases/static-objects-order/main.ts +++ b/tests/fixtures/npm/cases/static-objects-order/main.ts @@ -4,6 +4,7 @@ import { ns } from "static-objects-order/receivers.js"; import { greet } from "static-objects-order/lazy-user.js"; import { len2 } from "static-objects-order/vec-user.js"; import { early } from "static-objects-order/early-user.js"; +import { area } from "static-objects-order/coerce-user.js"; console.log("start"); console.log(ns.run("add"), ns.run("sub"), ns.reads()); @@ -12,3 +13,4 @@ console.log(ns.early()); console.log(greet("a"), greet("b")); console.log(len2(3, 4)); console.log(early()); +console.log(area(3)); diff --git a/tests/fixtures/npm/cases/static-objects-order/node_modules/static-objects-order/coerce-user.d.ts b/tests/fixtures/npm/cases/static-objects-order/node_modules/static-objects-order/coerce-user.d.ts new file mode 100644 index 000000000..947b775d5 --- /dev/null +++ b/tests/fixtures/npm/cases/static-objects-order/node_modules/static-objects-order/coerce-user.d.ts @@ -0,0 +1 @@ +export declare function area(w: number): string; diff --git a/tests/fixtures/npm/cases/static-objects-order/node_modules/static-objects-order/coerce-user.js b/tests/fixtures/npm/cases/static-objects-order/node_modules/static-objects-order/coerce-user.js new file mode 100644 index 000000000..7cf3c473c --- /dev/null +++ b/tests/fixtures/npm/cases/static-objects-order/node_modules/static-objects-order/coerce-user.js @@ -0,0 +1,6 @@ +var coerce = require("./coerce"); +module.exports = { + area: function (w) { + return coerce.seen + " " + new coerce.shapes.Box(w).area(); + }, +}; diff --git a/tests/fixtures/npm/cases/static-objects-order/node_modules/static-objects-order/coerce.js b/tests/fixtures/npm/cases/static-objects-order/node_modules/static-objects-order/coerce.js new file mode 100644 index 000000000..347c67de5 --- /dev/null +++ b/tests/fixtures/npm/cases/static-objects-order/node_modules/static-objects-order/coerce.js @@ -0,0 +1,17 @@ +// A conversion between the constructor function and its method runs code +// (Probe's toString) that reads the prototype before the method is stored. +var shapes = {}; +class Probe { + toString() { + return shapes.Box.prototype.area === undefined ? "no area yet" : "area"; + } +} +var probe = new Probe(); +shapes.Box = function (w) { + this.w = w; +}; +var seen = "" + probe; +shapes.Box.prototype.area = function () { + return this.w * this.w; +}; +module.exports = { shapes: shapes, seen: seen }; diff --git a/tests/harness/static-objects-order.test.ts b/tests/harness/static-objects-order.test.ts index d732d06a3..18fc58830 100644 --- a/tests/harness/static-objects-order.test.ts +++ b/tests/harness/static-objects-order.test.ts @@ -97,6 +97,12 @@ test( // early-user.js calls a function in the statement that loads lazy.js: // a binding after that statement is not initialized when it runs. expect(rewrittenIfAny(debug, join(pkg, "early-user.js")) ?? "").not.toContain("__ns_mod_"); + // coerce.js converts an object with a toString method between its + // constructor and the method: the conversion runs before the method exists. + expect(debug).toContain("code runs between the constructor and its methods"); + expect(rewrittenIfAny(debug, join(pkg, "coerce.js")) ?? "").not.toContain( + "class __ns_shapes_Box", + ); const vec = rewritten(debug, join(pkg, "vec.js")); expect(vec).toContain("class __ns_geo_Vec"); expect(vec).toContain("module.exports = { geo: geo };");