From 52c6dccfa5279b8ea16c4a498cbc51fbe386f15c Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Wed, 28 Oct 2026 22:00:02 -0500 Subject: [PATCH] Read the callee of an untyped member call before its arguments A call through an untyped receiver now reads a class instance's field, a getter, or a property added to the instance before it evaluates the arguments, as JavaScript does. An argument that replaces the member no longer changes which function runs. --- .../lowering/class-dynamic-dispatch.ts | 156 ++++++++++++++++++ .../corpus/dynamic-call-callee-order.mjs.json | 6 + packages/runtime/src/scr_dyn.h | 6 + packages/runtime/src/scr_dyn_invoke.c | 8 + packages/runtime/src/scr_json.c | 46 ++++++ tests/corpus/dynamic-call-callee-order.mjs | 113 +++++++++++++ 6 files changed, 335 insertions(+) create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/dynamic-call-callee-order.mjs.json create mode 100644 tests/corpus/dynamic-call-callee-order.mjs diff --git a/packages/compiler/src/frontend/lowering/class-dynamic-dispatch.ts b/packages/compiler/src/frontend/lowering/class-dynamic-dispatch.ts index c19763e8d..1870f87ce 100644 --- a/packages/compiler/src/frontend/lowering/class-dynamic-dispatch.ts +++ b/packages/compiler/src/frontend/lowering/class-dynamic-dispatch.ts @@ -93,6 +93,96 @@ import { protocolArm, protocolFieldArm } from "./class-protocol-dispatch.js"; import { STREAM_OPERATORS, operatorSignalOptions, reduceSignalSetup } from "./lower-stream.js"; import { deletedPropertyNames, fixedFieldFamily } from "./js-class-fields.js"; +/** Methods that builtin prototypes define (arrays, strings, collections, + * promises, functions, regular expressions, iterators, Object.prototype): + * a call of one of these names through an untyped receiver keeps the + * runtime's method dispatch, which answers them, even when a class + * declares a getter of the same name. */ +const BUILTIN_METHOD_NAMES: ReadonlySet = new Set([ + "add", + "apply", + "at", + "bind", + "call", + "catch", + "charAt", + "charCodeAt", + "clear", + "codePointAt", + "concat", + "copyWithin", + "delete", + "endsWith", + "entries", + "every", + "exec", + "fill", + "filter", + "finally", + "find", + "findIndex", + "findLast", + "findLastIndex", + "flat", + "flatMap", + "forEach", + "get", + "has", + "hasOwnProperty", + "includes", + "indexOf", + "isPrototypeOf", + "join", + "keys", + "lastIndexOf", + "localeCompare", + "map", + "match", + "matchAll", + "next", + "normalize", + "padEnd", + "padStart", + "pop", + "propertyIsEnumerable", + "push", + "reduce", + "reduceRight", + "repeat", + "replace", + "replaceAll", + "return", + "reverse", + "search", + "set", + "shift", + "slice", + "some", + "sort", + "splice", + "split", + "startsWith", + "substr", + "substring", + "test", + "then", + "throw", + "toLocaleString", + "toLowerCase", + "toReversed", + "toSorted", + "toSpliced", + "toString", + "toUpperCase", + "trim", + "trimEnd", + "trimStart", + "unshift", + "valueOf", + "values", + "with", +]); + const STREAM_METHODS = new Set([ "on", "addListener", @@ -1897,6 +1987,64 @@ export class ClassDynamicDispatch { byMethod.set(method, matching); return matching; }; + // A name some class declares as a getter, and no class as a method: the + // call reads the member (running the getter) before it evaluates the + // arguments, as JavaScript reads the callee first, and then calls what + // it read. The runtime reads fields and other own properties of class + // instances that early itself (scr_dyn_prepare_method). + const getterReceiver = (method: string): boolean => + !BUILTIN_METHOD_NAMES.has(method) && + this.propertyReceivers(lowerer, true).some(({ info }) => + findMethodOn(lowerer, info, `get:${method}`), + ); + const getterCall = (fn: IrFunction, invoke: IrExpr & { kind: "dynInvoke" }): IrExpr => { + const loc = invoke.loc; + const receiverLocal = { + id: `%dispatch.receiver.${fn.locals.length}`, + name: "receiver", + type: DYN, + mutable: false, + }; + fn.locals.push(receiverLocal); + const calleeLocal = { + id: `%dispatch.callee.${fn.locals.length}`, + name: "callee", + type: DYN, + mutable: false, + }; + fn.locals.push(calleeLocal); + const receiver = varRef(receiverLocal.id, DYN, loc); + return { + kind: "seqExpr", + stmts: [ + { kind: "varDecl", localId: receiverLocal.id, init: invoke.recv, loc }, + { + kind: "varDecl", + localId: calleeLocal.id, + init: { + kind: "dynKeyGet", + key: { kind: "strLit", value: invoke.method, type: STRING, loc }, + value: receiver, + type: DYN, + loc, + }, + loc, + }, + ], + result: { + kind: "dynCall", + callee: varRef(calleeLocal.id, DYN, loc), + receiver, + calleeName: invoke.calleeName, + ...(invoke.calleeNameValue ? { calleeNameValue: invoke.calleeNameValue } : {}), + args: invoke.args, + type: DYN, + loc, + }, + type: DYN, + loc, + }; + }; const candidateSets = new Map>(); const candidateSet = (method: string): ReadonlySet => { let set = candidateSets.get(method); @@ -2964,6 +3112,14 @@ export class ClassDynamicDispatch { const info = lowerer.classes.get(name); return info && findStaticOn(lowerer, info, method)?.method; }); + if ( + invoke && + expr.kind === "dynInvoke" && + candidates(invoke.method).length === 0 && + !staticCandidates(invoke.method) && + getterReceiver(invoke.method) + ) + return getterCall(fn, expr); if ( !invoke || (candidates(invoke.method).length === 0 && diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/dynamic-call-callee-order.mjs.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/dynamic-call-callee-order.mjs.json new file mode 100644 index 000000000..1e72aaf98 --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/dynamic-call-callee-order.mjs.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/corpus/dynamic-call-callee-order.mjs" + ], + "diags": [] +} diff --git a/packages/runtime/src/scr_dyn.h b/packages/runtime/src/scr_dyn.h index 343930580..a995002d5 100644 --- a/packages/runtime/src/scr_dyn.h +++ b/packages/runtime/src/scr_dyn.h @@ -1411,6 +1411,12 @@ void scr_dyn_typed_ref_bind_bag_class(ScrDyn *d, const ScrDynTypedBagClassOps *o * exception pending, when the prototype hook threw. Never runs user * code. */ ScrDyn *scr_dyn_typed_ref_bag_method(const ScrDyn *d, const char *key, size_t key_len); +/* The own data property `key` of a class instance's capsule, +1, as its + * view holds it; NULL when the instance has no such own data property, or + * when `key` names a member of its class's prototype or of + * Object.prototype. NULL with an exception pending when building the view + * threw. */ +ScrDyn *scr_dyn_typed_ref_own_member(const ScrDyn *d, const char *key, size_t key_len); /* The constructor name of a class instance capsule's own class, for * util.inspect; NULL when the capsule is not a bound class instance, its * class is anonymous, or its prototype was replaced. */ diff --git a/packages/runtime/src/scr_dyn_invoke.c b/packages/runtime/src/scr_dyn_invoke.c index 7a9b1bc35..0279ccab1 100644 --- a/packages/runtime/src/scr_dyn_invoke.c +++ b/packages/runtime/src/scr_dyn_invoke.c @@ -787,6 +787,14 @@ ScrDyn *scr_dyn_prepare_method(ScrDyn *recv, const char *method) { scr_str_release(key); return result; } + /* A class instance's own function-valued property is read before the + * arguments are evaluated, as JavaScript reads the callee first; its + * methods keep the dispatch scr_dyn_invoke makes. */ + if (recv->kind == SCR_DYN_TYPED_REF) { + ScrDyn *own = scr_dyn_typed_ref_own_member(recv, method, strlen(method)); + if (own && scr_dyn_is_callable(own)) return own; + scr_dyn_release(own); + } return NULL; } diff --git a/packages/runtime/src/scr_json.c b/packages/runtime/src/scr_json.c index f3b2d3dc0..0b4ffb2a1 100644 --- a/packages/runtime/src/scr_json.c +++ b/packages/runtime/src/scr_json.c @@ -4761,6 +4761,52 @@ bool scr_dyn_typed_ref_class_has(const ScrDyn *d, const ScrStr *key) { return ops && scr_dyn_name_in_strs(key, ops->names, ops->count); } +ScrDyn *scr_dyn_typed_ref_own_member(const ScrDyn *d, const char *key, size_t key_len) { + if (d->kind != SCR_DYN_TYPED_REF || d->prototype) return NULL; + if (!d->v.typed_ref.bag_class) { + /* A class instance's view holds its own fields and the properties added + * to it, and its class's prototype members stay off the view. A name + * that is neither one of those members nor one of Object.prototype's + * can only be an own property, so the view is built (refreshed from the + * instance) for it. Other capsules answer through scr_dyn_invoke. */ + const ScrDynTypedClassOps *klass = scr_typed_ref_class_ops(d); + if (!klass) return NULL; + ScrStr *name = scr_str_new(key, key_len); + bool member = scr_dyn_name_in_strs(name, klass->names, klass->count) || scr_dyn_object_proto_name(name); + scr_str_release(name); + if (member) return NULL; + ScrDyn *view = scr_dyn_typed_ref_materialize(d); + if (scr_exc_pending()) { + scr_dyn_release(view); + return NULL; + } + ScrDyn *found = NULL; + if (view && view->kind == SCR_DYN_OBJ) { + if (scr_dyn_is_layout(view)) { + ScrDyn **slot = scr_dyn_layout_slot(view, key, key_len); + if (slot) found = scr_dyn_retain(*slot); + } else { + const ScrDynEntry *own = scr_dyn_find_entry(view, key, key_len); + if (own && !own->accessor) found = scr_dyn_retain(own->value); + } + } + scr_dyn_release(view); + return found; + } + const ScrDynTypedBagClassOps *ops = (const ScrDynTypedBagClassOps *)(const void *)d->v.typed_ref.array; + void *instance = d->v.typed_ref.ptr; + const ScrDyn *bag = ops->bag(instance); + if (!bag || bag->kind != SCR_DYN_OBJ) return NULL; + const ScrDynEntry *own = scr_dyn_find_entry(bag, key, key_len); + if (!own || own->accessor) return NULL; + for (size_t i = 0; i < ops->count; i++) { + const ScrStr *name = ops->names[i]; + if (name->len != key_len || memcmp(name->data, key, key_len) != 0) continue; + return own->writable ? ops->field(instance, i) : NULL; + } + return scr_dyn_retain(own->value); +} + static const ScrDynTypedRecordOps *scr_typed_ref_record_ops(const ScrDyn *d) { if (!d->v.typed_ref.record || d->v.typed_ref.materialized) return NULL; const ScrDynTypedRecordOps *ops = (const ScrDynTypedRecordOps *)(const void *)d->v.typed_ref.array; diff --git a/tests/corpus/dynamic-call-callee-order.mjs b/tests/corpus/dynamic-call-callee-order.mjs new file mode 100644 index 000000000..20190db84 --- /dev/null +++ b/tests/corpus/dynamic-call-callee-order.mjs @@ -0,0 +1,113 @@ +// A call through an untyped receiver reads its callee before it evaluates +// the arguments: an argument that replaces the member does not change which +// function the call runs. Class instance fields, properties added to an +// instance later, plain object properties, prototype methods, getters, own +// accessors, a field that does not hold a function, and array methods named +// like a class field or getter. +const log = []; +class Holder { + constructor() { + this.run = () => "field-init"; + } + get viaGetter() { + log.push("getter"); + return () => "from-getter"; + } +} +function callRun(target, replace) { + return target.run(replace()); +} +function callMethod(target, replace) { + return target.method(replace()); +} +function callGetter(target, replace) { + return target.viaGetter(replace()); +} +const holder = new Holder(); +holder.run = () => "field-old"; +console.log( + callRun(holder, () => { + holder.run = () => "field-new"; + return 0; + }), + holder.run(), +); +function callExtra(target, replace) { + return target.extra(replace()); +} +const later = new Holder(); +later.extra = () => "added-old"; +console.log( + callExtra(later, () => { + later.extra = () => "added-new"; + return 0; + }), + later.extra(), +); +const plain = { run: () => "plain-old" }; +console.log( + callRun(plain, () => { + plain.run = () => "plain-new"; + return 0; + }), + plain.run(), +); +class Swapped { + method() { + return "method-old"; + } +} +console.log( + callMethod(new Swapped(), () => { + Swapped.prototype.method = () => "method-new"; + return 0; + }), + new Swapped().method(), +); +console.log( + callGetter(new Holder(), () => { + log.push("argument"); + return 0; + }), + log.join(","), +); +log.length = 0; +const accessorTarget = { run: () => "plain" }; +Object.defineProperty(accessorTarget, "run", { + get() { + log.push("own-getter"); + return () => "own-accessor"; + }, + configurable: true, +}); +console.log( + callRun(accessorTarget, () => { + log.push("argument"); + return 0; + }), + log.join(","), +); +class Box { + constructor(value) { + this.run = value; + } +} +try { + callRun(new Box(5), () => 0); +} catch (e) { + console.log(e instanceof TypeError, e.message); +} +// A class field and a getter named like array methods do not change calls +// of those methods on arrays. +class Material { + constructor() { + this.map = null; + } + get filter() { + return () => "material-filter"; + } +} +function doubled(items) { + return items.map((x) => x * 2).filter((x) => x > 2); +} +console.log(new Material().map, new Material().filter(), doubled([1, 2, 3]).join(","));