From dc4865574398434d768e964ff8bb127c06790705 Mon Sep 17 00:00:00 2001 From: Aiden Cline <63023139+rekram1-node@users.noreply.github.com> Date: Thu, 24 Sep 2026 00:36:41 -0500 Subject: [PATCH] fix(codemode): compare with == by IsLooselyEqual and stop deep-walking operands for opaque references (#50830) --- packages/codemode/interpreter-support.md | 15 ++++-- .../codemode/src/interpreter/interpreter.ts | 45 +++++++++------- packages/codemode/src/stdlib/object.ts | 11 +--- packages/codemode/test/errors.test.ts | 4 +- packages/codemode/test/parity.test.ts | 53 +++++++++++++++++++ packages/codemode/test/stdlib.test.ts | 4 +- packages/codemode/test/test262/README.md | 2 +- packages/codemode/test/test262/manifest.json | 11 +++- packages/codemode/test/test262/skipped.txt | 9 +++- 9 files changed, 111 insertions(+), 43 deletions(-) diff --git a/packages/codemode/interpreter-support.md b/packages/codemode/interpreter-support.md index b3c818298cd..e0232019334 100644 --- a/packages/codemode/interpreter-support.md +++ b/packages/codemode/interpreter-support.md @@ -207,11 +207,16 @@ ultimate source of truth. Upstream test262 files run verbatim from `test/test262 non-reference (`delete 0`, `delete f()`) evaluates the operand and is `true`; `delete x` on a variable throws. - [x] Coercion helpers and template interpolation accept functions and namespaces: `String(fn)` and `${fn}` give `"[object Function]"` rather than the source text, `isNaN(fn)` is `true`. -- [ ] Operators other than `===`/`!==`, `switch` discriminants and cases, and `Object.is` applied to a function, - promise, generator, tool reference, or any object holding one anywhere inside; JavaScript compares by identity or - coerces (`fn == null` is `false`, `fn + ""` is its source text), the interpreter throws - `TypeError: Binary operators require data values.` The check walks both operands' whole object graphs, so - `rows == null` on a large array is slow where `rows === null` is not. +- [x] `==` and `!=` follow IsLooselyEqual: objects (including functions and tool references) compare by identity, a + nullish operand never coerces the other side, and a data object facing a primitive coerces through its built-in + primitive form (`fn == null` is `false`, `fn == fn` is `true`, `[1] == 1` and `[1, 2] == "1,2"` are `true`). + `switch` matches cases with `===`, so `switch (fn) { case fn: }` selects, and `Object.is` compares any two + values. Operators inspect only their direct operands, so `rows == null` on a large array costs the same as + `rows === null`, and an object merely holding a function inside (`[fn] + ""`, `-[fn]`) coerces like any other + data object (`"[object Function]"`, `NaN`). +- [ ] Coercing a function, promise, generator, or tool reference itself: `fn + ""`, `-fn`, `fn++`, and `fn == 1` + throw `TypeError: Binary operators require data values.` (or the unary/update form) where JavaScript would use + the source text or `NaN`. - [ ] ToPrimitive on program objects: operators, `Number`/`String`, `Error(message)`, `parseInt` radix, multi-argument `Date` construction and `Date.UTC`, and numeric built-in arguments (`Math.max`, `at`, `indexOf` start) should call the object's own `valueOf`/`toString` in spec order and surface their throws. Today they use the built-in form diff --git a/packages/codemode/src/interpreter/interpreter.ts b/packages/codemode/src/interpreter/interpreter.ts index e3c99a97b11..ac3d886ec14 100644 --- a/packages/codemode/src/interpreter/interpreter.ts +++ b/packages/codemode/src/interpreter/interpreter.ts @@ -95,7 +95,7 @@ import { } from "./objects.js" import { preserveConsumerError } from "./callback.js" import { Pending, resolvePromise, resolvePromiseValue } from "./promises.js" -import { containsOpaqueReference, describeValue, rejectCircularInsertion, typeofValue } from "./references.js" +import { describeValue, isOpaque, rejectCircularInsertion, typeofValue } from "./references.js" import { ScopeStack } from "./scope.js" import { constructRegExp } from "../stdlib/regexp.js" import { enumerableSource } from "../stdlib/object.js" @@ -540,9 +540,6 @@ class Frame { const self = this return Effect.gen(function* () { const discriminant = yield* self.evaluateExpression(node.discriminant) - if (containsOpaqueReference(discriminant)) { - throw invalidData("Switch discriminants must be data values.", node) - } self.scopes.push() return yield* Effect.gen(function* () { const cases = node.cases @@ -557,11 +554,7 @@ class Frame { defaultIndex = index continue } - const candidate = yield* self.evaluateExpression(test) - if (containsOpaqueReference(candidate)) { - throw invalidData("Switch case values must be data values.", test) - } - if (candidate === discriminant) { + if ((yield* self.evaluateExpression(test)) === discriminant) { selected = index break } @@ -1358,16 +1351,17 @@ class Frame { private applyBinaryOperator(operator: string, lhs: Value, rhs: Value, node: AstNode): Value { if (operator === "===") return lhs === rhs if (operator === "!==") return lhs !== rhs - if (operator === "in" && rhs instanceof Obj && !containsOpaqueReference(lhs)) { + if (operator === "==") return this.looselyEqual(lhs, rhs, node) + if (operator === "!=") return !this.looselyEqual(lhs, rhs, node) + if (operator === "in" && rhs instanceof Obj && !isOpaque(lhs)) { return has(rhs, lhs !== null && typeof lhs === "object" ? coerceToString(lhs) : (lhs as PropertyKey)) } - if (containsOpaqueReference(lhs) || containsOpaqueReference(rhs)) { + if (isOpaque(lhs) || isOpaque(rhs)) { throw invalidData("Binary operators require data values.", node) } - // Addition and loose equality use the default hint; every other operator asks for a number. - const hint = operator === "+" || operator === "==" || operator === "!=" ? "default" : "number" + // Addition uses the default hint; every other operator asks for a number. + const hint = operator === "+" ? "default" : "number" const coerceOperand = (operand: Value) => (operand instanceof Obj ? operand.toPrimitive(hint) : operand) - const bothObjects = lhs !== null && typeof lhs === "object" && rhs !== null && typeof rhs === "object" const l = coerceOperand(lhs) const r = coerceOperand(rhs) switch (operator) { @@ -1386,10 +1380,6 @@ class Frame { return (l as number) % (r as number) case "**": return (l as number) ** (r as number) - case "==": - return bothObjects ? lhs === rhs : l == r - case "!=": - return bothObjects ? lhs !== rhs : l != r case "<": return (l as string) < (r as string) case "<=": @@ -1420,6 +1410,21 @@ class Frame { } } + // IsLooselyEqual: objects (including functions and tool references) compare by identity, and only a + // data object facing a non-nullish primitive needs to coerce, so an opaque value is rejected only there. + private looselyEqual(lhs: Value, rhs: Value, node: AstNode): boolean { + const lhsObject = lhs !== null && typeof lhs === "object" + const rhsObject = rhs !== null && typeof rhs === "object" + if (lhsObject === rhsObject) return lhsObject ? lhs === rhs : lhs == rhs + const object = lhsObject ? lhs : rhs + const primitive = lhsObject ? rhs : lhs + if (primitive === null || primitive === undefined) return false + if (!(object instanceof Obj) || isOpaque(object)) { + throw invalidData("Binary operators require data values.", node) + } + return object.toPrimitive("default") == primitive + } + private evaluateLogicalExpression(node: LogicalExpression): Effect.Effect { const operator = node.operator return Effect.flatMap(this.evaluateExpression(node.left), (left) => { @@ -1443,7 +1448,7 @@ class Frame { if (operator === "typeof") return typeofValue(value) if (operator === "!") return !value if (operator === "void") return undefined - if (containsOpaqueReference(value)) { + if (isOpaque(value)) { throw invalidData("Unary operators require data values.", node) } const operand = value instanceof Obj ? value.toPrimitive("number") : value @@ -1546,7 +1551,7 @@ class Frame { // CodeMode numeric coercion, not host Number(): null-prototype data objects would make // the host throw during ToPrimitive, and opaque runtime references must reject clearly. const operand = (current: Value): number => { - if (containsOpaqueReference(current)) { + if (isOpaque(current)) { throw invalidData(`'${operator}' requires a data value.`, argument) } return coerceToNumber(current) diff --git a/packages/codemode/src/stdlib/object.ts b/packages/codemode/src/stdlib/object.ts index 427bf43141e..335a23a3744 100644 --- a/packages/codemode/src/stdlib/object.ts +++ b/packages/codemode/src/stdlib/object.ts @@ -154,16 +154,7 @@ export const objectGlobal = (ctx: Interpreter) => { ), ], ["hasOwn", 2, (_, args) => hasOwn(enumerableSource(ctx, "Object.hasOwn(...)", args[0]), propertyKey(args[1]))], - [ - "is", - 2, - (_, args) => { - if (containsOpaqueReference(args[0]) || containsOpaqueReference(args[1])) { - throw invalidData("Object.is requires data values.") - } - return Object.is(args[0], args[1]) - }, - ], + ["is", 2, (_, args) => Object.is(args[0], args[1])], ["assign", 2, (_, args) => objectAssign(ctx, args)], ["fromEntries", 1, (_, args) => objectFromEntries(ctx, args[0])], ]) diff --git a/packages/codemode/test/errors.test.ts b/packages/codemode/test/errors.test.ts index 30b06ed2133..b64efd96021 100644 --- a/packages/codemode/test/errors.test.ts +++ b/packages/codemode/test/errors.test.ts @@ -68,9 +68,9 @@ describe("error identity", () => { describe("rethrown interpreter failures", () => { test("keep their diagnostic kind and source location", async () => { - const failure = await error(`try { switch (Symbol) {} } catch (e) { throw e }`) + const failure = await error(`try { Symbol + 1 } catch (e) { throw e }`) expect(failure.kind).toBe("InvalidDataValue") - expect(failure.message).toStartWith("TypeError: Switch discriminants must be data values. (line ") + expect(failure.message).toStartWith("TypeError: Binary operators require data values. (line ") expect(failure.location).toBeDefined() }) diff --git a/packages/codemode/test/parity.test.ts b/packages/codemode/test/parity.test.ts index 6e9bd472d95..681ed386de6 100644 --- a/packages/codemode/test/parity.test.ts +++ b/packages/codemode/test/parity.test.ts @@ -1178,3 +1178,56 @@ describe("sloppy duplicate parameters and for...in targets", () => { ).toEqual([["p", "q"], "q", ["p", "q"], "a"]) }) }) + +describe("loose equality and operator gates on opaque references", () => { + test("== follows IsLooselyEqual for data values", async () => { + expect( + await value(` + return [null == undefined, "1" == 1, true == 1, "" == 0, [1] == 1, [1, 2] == "1,2", + ({}) == "[object Object]", NaN == NaN, ({}) == ({}), null == 0, new Date(0) == 0] + `), + ).toEqual([true, true, true, true, true, true, true, false, false, false, false]) + }) + + test("functions and tool references compare by identity and are never equal to nullish", async () => { + expect( + await value(` + const fn = () => 1, other = () => 2 + return [fn == null, fn != null, fn == undefined, fn == fn, fn == other, [fn] == null, ({ f: fn }) == null, + [fn] == [fn], tools == null, tools == tools] + `), + ).toEqual([false, true, false, true, false, false, false, false, false, true]) + }) + + test("coercing an opaque reference against a non-nullish primitive still rejects", async () => { + expect((await error(`const fn = () => 1; return fn == 1`)).message).toContain( + "Binary operators require data values", + ) + expect((await error(`const fn = () => 1; return fn + ""`)).message).toContain( + "Binary operators require data values", + ) + expect((await error(`const fn = () => 1; return -fn`)).message).toContain("Unary operators require data values") + expect((await error(`let fn = () => 1; fn++`)).message).toContain("'++' requires a data value") + }) + + test("switch and Object.is match opaque references by identity", async () => { + expect( + await value(` + const fn = () => 1, other = () => 2 + const pick = (v) => { switch (v) { case fn: return "fn"; case other: return "other"; default: return "none" } } + return [pick(fn), pick(other), pick(1), Object.is(fn, fn), Object.is(fn, other), Object.is(NaN, NaN), Object.is(0, -0)] + `), + ).toEqual(["fn", "other", "none", true, false, true, false]) + }) + + test("operators look only at their direct operands, so nested functions coerce like other data", async () => { + expect( + await value(` + const fn = () => 1 + let x = [fn] + x++ + return [[1, [2]] + "", ({ a: 1 }) * 2, [fn] + "", Number.isNaN(-[fn]), Number.isNaN(x), typeof fn, !fn] + `), + ).toEqual(["1,2", null, "[object Function]", true, true, "function", false]) + }) +}) diff --git a/packages/codemode/test/stdlib.test.ts b/packages/codemode/test/stdlib.test.ts index d7e25e64f07..9e02b0428c1 100644 --- a/packages/codemode/test/stdlib.test.ts +++ b/packages/codemode/test/stdlib.test.ts @@ -1535,8 +1535,8 @@ describe("stdlib integration", () => { ).toEqual([true, false, true, false]) }) - test("Object.is rejects opaque runtime references", async () => { - expect((await error(`return Object.is(Math.max, Math.max)`)).kind).toBe("InvalidDataValue") + test("Object.is compares opaque runtime references by identity", async () => { + expect(await value(`return [Object.is(Math.max, Math.max), Object.is(Math.max, Math.min)]`)).toEqual([true, false]) }) test("Object values and entries accept arrays", async () => { diff --git a/packages/codemode/test/test262/README.md b/packages/codemode/test/test262/README.md index 582382943b5..9c4135cbada 100644 --- a/packages/codemode/test/test262/README.md +++ b/packages/codemode/test/test262/README.md @@ -24,7 +24,7 @@ Without them the runner registers no tests, so CI is unaffected. Licensed under `script/sync-test262.ts` skips a file when its frontmatter declares a `flags`, `features`, or `includes` value the manifest marks unsupported, or when its code matches one of the manifest's `boundaries` patterns. The sync checks the -checkout is at the pinned revision, so every machine runs the same 3993 files. Boundaries are +checkout is at the pinned revision, so every machine runs the same 4478 files. Boundaries are intentional limits of the interpreter, not compatibility work: classes, `this`, `arguments`, prototype objects, property descriptors, accessors, boxed primitives, sloppy mode, `eval`, `Symbol()`, and the `$262` host API. If one of those decisions changes, delete its entry and re-sync; the tests are upstream, not lost. diff --git a/packages/codemode/test/test262/manifest.json b/packages/codemode/test/test262/manifest.json index 26027df89f5..317731d8229 100644 --- a/packages/codemode/test/test262/manifest.json +++ b/packages/codemode/test/test262/manifest.json @@ -1,6 +1,15 @@ { "revision": "250f204f23a9249ff204be2baec29600faae7b75", - "directories": ["built-ins/Array/prototype", "built-ins/Iterator", "built-ins/String/raw", "language/expressions/tagged-template", "language/statements"], + "directories": [ + "built-ins/Array/prototype", + "built-ins/Iterator", + "built-ins/Object/is", + "built-ins/String/raw", + "language/expressions/does-not-equals", + "language/expressions/equals", + "language/expressions/tagged-template", + "language/statements" + ], "harness": ["assert.js", "sta.js", "compareArray.js", "doneprintHandle.js"], "flags": ["module", "raw", "noStrict"], "boundaries": { diff --git a/packages/codemode/test/test262/skipped.txt b/packages/codemode/test/test262/skipped.txt index 9e34665391f..e4629a834a9 100644 --- a/packages/codemode/test/test262/skipped.txt +++ b/packages/codemode/test/test262/skipped.txt @@ -198,6 +198,13 @@ built-ins/Iterator/prototype/drop/limit-tonumber-throws.js # Expected a Test262 built-ins/Iterator/prototype/drop/limit-tonumber.js # Iterator.prototype.drop expects a non-negative count, received NaN. built-ins/Iterator/prototype/take/limit-tonumber-throws.js # Expected a Test262Error but got a RangeError built-ins/Iterator/prototype/take/limit-tonumber.js # Iterator.prototype.take expects a non-negative count, received NaN. +built-ins/Object/is/not-same-value-x-y-object.js # Object(number) wrapper objects are not supported; use the primitive value directly. +built-ins/Object/is/same-value-x-y-object.js # Object(number) wrapper objects are not supported; use the primitive value directly. +language/expressions/does-not-equals/S11.9.2_A7.8.js # #1: (true != {valueOf: function() {return 1}}) === false +language/expressions/does-not-equals/S11.9.2_A7.9.js # #1: ({valueOf: function() {return 1}} != true) === false +language/expressions/equals/S11.9.1_A7.8.js # #1: ({valueOf: function() {return 1}} == true) === true +language/expressions/equals/S11.9.1_A7.9.js # #1: (true == {valueOf: function() {return 1}}) === true +language/expressions/equals/S9.1_A1_T3.js # #1: var object = {valueOf: function() {return 1}, toString: function() {return 0}}; object + "" === language/expressions/tagged-template/constructor-invocation.js # The called value cannot be constructed: user-defined constructors and classes are not supported. language/expressions/tagged-template/template-object-frozen-strict.js # Expected a TypeError to be thrown but no exception was thrown at all language/statements/async-generator/dstr/ary-init-iter-get-err-array-prototype.js # Expected a TypeError to be thrown but no exception was thrown at all @@ -265,7 +272,5 @@ language/statements/return/S12.9_A1_T6.js # expected SyntaxError but the progra language/statements/return/S12.9_A1_T7.js # expected SyntaxError but the program ran language/statements/return/S12.9_A1_T8.js # expected SyntaxError but the program ran language/statements/return/S12.9_A1_T9.js # expected SyntaxError but the program ran -language/statements/switch/S12.11_A1_T4.js # Switch discriminants must be data values. -language/statements/switch/scope-lex-open-dflt.js # Switch discriminants must be data values. language/statements/try/dstr/ary-init-iter-get-err-array-prototype.js # Expected a TypeError to be thrown but no exception was thrown at all language/statements/variable/dstr/ary-init-iter-get-err-array-prototype.js # Expected a TypeError to be thrown but no exception was thrown at all