From 5ad9c266dc62cfd1eab6c484845abee6cf44e74c Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Mon, 26 Oct 2026 06:40:02 -0500 Subject: [PATCH 1/4] Read missing elements through non-null asserted operands as Node does --- .../src/backend/llvm/dense-array-access.ts | 58 +++- .../src/backend/llvm/expr-containers.ts | 21 ++ .../src/backend/llvm/int32-kind-loops.ts | 2 + .../src/backend/llvm/integer-clones.ts | 19 +- packages/compiler/src/contracts/check.ts | 9 + .../src/frontend/lowering/lower-exprs.ts | 259 ++++++++++++++++-- .../src/frontend/lowering/nan-coded-reads.ts | 13 + packages/compiler/src/ir/int32-slots.ts | 45 ++- .../compiler/src/ir/parameter-versions.ts | 2 +- .../7393-non-null-element-operands.ts.json | 6 + .../7395-non-null-element-dereference.ts.json | 6 + .../corpus/7393-non-null-element-operands.ts | 81 ++++++ .../7395-non-null-element-dereference.ts | 71 +++++ .../7393-non-null-element-operands.ts.json | 1 + .../7395-non-null-element-dereference.ts.json | 1 + 15 files changed, 558 insertions(+), 36 deletions(-) create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7393-non-null-element-operands.ts.json create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7395-non-null-element-dereference.ts.json create mode 100644 tests/corpus/7393-non-null-element-operands.ts create mode 100644 tests/corpus/7395-non-null-element-dereference.ts create mode 100644 tests/harness/corpus-costs/7393-non-null-element-operands.ts.json create mode 100644 tests/harness/corpus-costs/7395-non-null-element-dereference.ts.json diff --git a/packages/compiler/src/backend/llvm/dense-array-access.ts b/packages/compiler/src/backend/llvm/dense-array-access.ts index e002ecd8a0..0991c344ca 100644 --- a/packages/compiler/src/backend/llvm/dense-array-access.ts +++ b/packages/compiler/src/backend/llvm/dense-array-access.ts @@ -19,6 +19,7 @@ * length, so reads never reach the state check for it; a write past the * bound tests the flag and, without states, appends at the length. */ +import { InternalCompilerError } from "../../errors.js"; import { isRefCounted, type IrExpr, type IrType } from "../../ir/ir.js"; import { f64Lit, @@ -368,7 +369,9 @@ export function emitDenseArrayGet( /** emitDenseArrayGet, with the int32 of an int32 array's element: the * slot itself on the dense path, the runtime getter's double converted - * (exactly: the element is an int32) on the other. */ + * (exactly: the element is an int32) on the other. With `inexact`, the + * getter's double may be something other than an int32 (NaN from + * `scr_arr_get_number`): the read branches there when it is. */ export function emitDenseArrayRead( host: LlvmEmitterContext, array: string, @@ -379,6 +382,7 @@ export function emitDenseArrayRead( getter: string, retain: boolean, proven = false, + inexact?: string, ): { value: string; int32: string | null } { const B = host.B; const ty = valueType(acc); @@ -460,22 +464,68 @@ export function emitDenseArrayRead( : READ_FALLBACK; B.line(`${slowValue} = call ${ty} @${getter}(ptr ${array}, double ${index.name})${attrs}`); let slowInt: string | null = null; - if (fastInt !== null) { + let slowLabel = slow; + if (fastInt !== null && inexact !== undefined) { + // An int32 that is not -0, or the inexact branch. + host.declare("declare i32 @llvm.fptosi.sat.i32.f64(double)"); + slowInt = B.tmp(); + const back = B.tmp(), + exact = B.tmp(), + bits = B.tmp(), + notNegativeZero = B.tmp(), + ok = B.tmp(); + B.line(`${slowInt} = call i32 @llvm.fptosi.sat.i32.f64(double ${slowValue})`); + B.line(`${back} = sitofp i32 ${slowInt} to double`); + B.line(`${exact} = fcmp oeq double ${back}, ${slowValue}`); + B.line(`${bits} = bitcast double ${slowValue} to i64`); + B.line(`${notNegativeZero} = icmp ne i64 ${bits}, -9223372036854775808`); + B.line(`${ok} = and i1 ${exact}, ${notNegativeZero}`); + slowLabel = B.newLabel("arr.get.int32"); + B.condBr(ok, slowLabel, inexact); + B.startBlock(slowLabel); + } else if (fastInt !== null) { slowInt = B.tmp(); B.line(`${slowInt} = fptosi double ${slowValue} to i32`); } B.br(join); B.startBlock(join); const result = B.tmp(); - B.line(`${result} = phi ${ty} [ ${fast}, %${fastLabel} ], [ ${slowValue}, %${slow} ]`); + B.line(`${result} = phi ${ty} [ ${fast}, %${fastLabel} ], [ ${slowValue}, %${slowLabel} ]`); let int32: string | null = null; if (fastInt !== null && slowInt !== null) { int32 = B.tmp(); - B.line(`${int32} = phi i32 [ ${fastInt}, %${fastLabel} ], [ ${slowInt}, %${slow} ]`); + B.line(`${int32} = phi i32 [ ${fastInt}, %${fastLabel} ], [ ${slowInt}, %${slowLabel} ]`); } return { value: result, int32 }; } +/** emitDenseArrayNumber of an int32 array, with the element's int32: a + * read that answers NaN (or another value that is not an int32) branches + * to `inexact`. */ +export function emitDenseArrayNumberInt32( + host: LlvmEmitterContext, + array: string, + index: LlValue, + indexExpr: IrExpr | undefined, + proven: boolean, + inexact: string, +): { value: string; int32: string } { + const read = emitDenseArrayRead( + host, + array, + index, + indexExpr, + "i32", + { kind: "f64" }, + "scr_arr_get_number", + false, + proven, + inexact, + ); + if (read.int32 === null) throw new InternalCompilerError("llvm emitter bug: int32 read"); + return { value: read.value, int32: read.int32 }; +} + /** `scr_arr_get_number`: a numeric element, or NaN for every missing, * hole, or present-undefined slot (answered by the runtime). */ export function emitDenseArrayNumber( diff --git a/packages/compiler/src/backend/llvm/expr-containers.ts b/packages/compiler/src/backend/llvm/expr-containers.ts index 7eec14e4e6..279afdcd05 100644 --- a/packages/compiler/src/backend/llvm/expr-containers.ts +++ b/packages/compiler/src/backend/llvm/expr-containers.ts @@ -62,12 +62,14 @@ import { import { exactInteger, widenInteger } from "./integer-values.js"; import { emitDenseArrayNumber, + emitDenseArrayNumberInt32, emitDenseArrayPop, emitDenseArrayPush, runtimeAccess, type ArrayAccess, } from "./dense-array-access.js"; import { INT32_ARRAY_METHODS } from "../../ir/int32-arrays.js"; +import { INT32_RANGE } from "../../ir/integer-ranges.js"; import { emitUnionWiden } from "./expr-records.js"; import { registerReplay } from "./emit-journal.js"; @@ -694,6 +696,25 @@ export function emitArrIntrinsic( )!; return { name: value, type: e.type }; } + // In a store statement of an int32 kind loop's fast copy, a read that + // is not an int32 (a missing element's NaN) leaves for the + // statement's resume point, so the element is at hand as an i32. + if (!table && elem.kind === "f64" && access === "i32" && host.int32KindResume !== null) { + const read = emitDenseArrayNumberInt32( + host, + r.name, + index, + e.args[0], + host.provenDenseIndex(e.receiver, e.args[0]!), + host.int32KindResume, + ); + return { + name: read.value, + type: e.type, + uint32: read.int32, + integer: { name: read.int32, type: "i32", signed: true, range: INT32_RANGE }, + }; + } if (!table && elem.kind === "f64") return { name: emitDenseArrayNumber( diff --git a/packages/compiler/src/backend/llvm/int32-kind-loops.ts b/packages/compiler/src/backend/llvm/int32-kind-loops.ts index 12e38c1f0e..d596a731db 100644 --- a/packages/compiler/src/backend/llvm/int32-kind-loops.ts +++ b/packages/compiler/src/backend/llvm/int32-kind-loops.ts @@ -57,6 +57,8 @@ function isPure(e: IrExpr): boolean { case "arrayGet": case "arrayHas": return e.arr.kind === "varRef" && isPure(e.index); + case "arrIntrinsic": + return e.method === "getNumber" && e.receiver.kind === "varRef" && isPure(e.args[0]!); default: return false; } diff --git a/packages/compiler/src/backend/llvm/integer-clones.ts b/packages/compiler/src/backend/llvm/integer-clones.ts index 314eae5eaa..3f3f86e220 100644 --- a/packages/compiler/src/backend/llvm/integer-clones.ts +++ b/packages/compiler/src/backend/llvm/integer-clones.ts @@ -191,6 +191,8 @@ function cloneShape(fn: IrFunction, borrowed?: BorrowedParameter): Set | case "ternary": return expr(e.cond, "bool") && expr(e.then, want) && expr(e.else_, want); case "arrIntrinsic": + if (e.method === "getNumber") + return want === "num" && arrayRef(e.receiver) && expr(e.args[0]!, "num"); return ( want === "num" && e.method === "length" && e.args.length === 0 && arrayRef(e.receiver) ); @@ -268,6 +270,9 @@ function callsMatch(fn: IrFunction, byName: ReadonlyMap): bo case "arrayGet": visit(e.index); return; + case "arrIntrinsic": + if (e.method === "getNumber") visit(e.args[0]!); + return; default: return; } @@ -432,6 +437,8 @@ function callsAnything(e: IrExpr): boolean { return callsAnything(e.cond) || callsAnything(e.then) || callsAnything(e.else_); case "arrayGet": return callsAnything(e.index); + case "arrIntrinsic": + return e.method === "getNumber" && callsAnything(e.args[0]!); default: return false; } @@ -1020,10 +1027,10 @@ class CloneEmitter { * keeps every slot below `filled` a present value) whose element is * exactly an int32. Everything else (holes, indices outside the prefix, * other numbers) bails: the ordinary body reads it. */ - private emitElement(e: Extract): string { + private emitElement(arr: IrExpr, indexExpr: IrExpr): string { const { B } = this; - const array = this.emitArray(e.arr); - const index = this.narrow(this.emitNum(e.index)); + const array = this.emitArray(arr); + const index = this.narrow(this.emitNum(indexExpr)); const filledField = B.tmp(); const filled = B.tmp(); const inside = B.tmp(); @@ -1157,9 +1164,13 @@ class CloneEmitter { if (this.inlines(e)) return { name: this.emitInlineCall(e), wide: false }; return { name: this.emitCall(e), wide: true }; case "arrIntrinsic": + // `getNumber` reads an element like `arrayGet`: a missing one bails, + // and the ordinary body reads it as NaN. + if (e.method === "getNumber") + return { name: this.emitElement(e.receiver, e.args[0]!), wide: false }; return { name: this.emitLength(this.emitArray(e.receiver)), wide: false }; case "arrayGet": - return { name: this.emitElement(e), wide: false }; + return { name: this.emitElement(e.arr, e.index), wide: false }; case "unary": { const operand = this.narrow(this.emitNum(e.operand)); const value = B.tmp(); diff --git a/packages/compiler/src/contracts/check.ts b/packages/compiler/src/contracts/check.ts index fe5dc171e7..1e17c8782a 100644 --- a/packages/compiler/src/contracts/check.ts +++ b/packages/compiler/src/contracts/check.ts @@ -1581,6 +1581,15 @@ class ImmutableContracts { for (let steps = 0; steps < 16; steps++) { if (root.kind === "fieldGet" || root.kind === "recordGet") root = root.obj; else if (root.kind === "arrayGet") root = root.arr; + else if (root.kind === "seqExpr") root = root.result; + // A receiver that is the element when it is present and otherwise + // throws (`xs[i]!.push(v)`) is that element read. + else if ( + root.kind === "ternary" && + root.else_.kind === "libCall" && + root.else_.fn === "error.nodeThrow" + ) + root = root.then; else if ( root.kind === "upcast" || root.kind === "downcast" || diff --git a/packages/compiler/src/frontend/lowering/lower-exprs.ts b/packages/compiler/src/frontend/lowering/lower-exprs.ts index c70826975a..91864a036f 100644 --- a/packages/compiler/src/frontend/lowering/lower-exprs.ts +++ b/packages/compiler/src/frontend/lowering/lower-exprs.ts @@ -412,6 +412,10 @@ function lowerExprInner(lowerer: Lowerer, expr: ts.Expression): IrExpr { const probe = lowerAbsenceProbe(lowerer, expr.expression); if (probe?.type.kind === "union") return probe; } + const dereferenced = dereferencedElementReceiver(lowerer, expr, loc); + if (dereferenced) return dereferenced; + const element = assertedElementOperand(lowerer, expr, loc); + if (element) return element; const inner = lowerer.lowerExpr(expr.expression); if (inner.type.kind === "union") { // `!` performs no runtime check: a missing array element stays @@ -1074,18 +1078,10 @@ function assertionOperand( loc: SrcLoc, ): IrExpr | null { if (inner.type.kind !== "union" || target === null || target.kind === "union") return null; - const arms = lowerer.unions.get(inner.type.unionId)?.arms; - if ( - !arms?.some(isUnitType) || - !arms.every( - (arm) => - arm.kind === "f64" || arm.kind === "string" || arm.kind === "bool" || isUnitType(arm), - ) - ) - return null; - // Array element reads keep their own handling of a missing element. A - // typed-array read is optional here only where its use tells undefined - // from NaN (assertedReadObservesUndefined). + if (!scalarUnion(lowerer, inner.type.unionId)) return null; + // Array element reads take assertedElementOperand or keep their own + // handling of a missing element. A typed-array read is optional here only + // where its use tells undefined from NaN (assertedReadObservesUndefined). let operand: ts.Expression = expr.expression; while (ts.isParenthesizedExpression(operand)) operand = operand.expression; if (ts.isElementAccessExpression(operand)) { @@ -1094,19 +1090,241 @@ function assertionOperand( } const use = convertingUseOf(lowerer, expr); if (use === null) return null; - if (use === "value") return inner; + if (use === "value" || use === "string") return inner; return lowerPrimitiveUnionNumber(lowerer, inner, expr, loc); } -/** How the converting use of `x!` (see assertionOperand) reads it: - * "number" where the use applies ToNumber, "value" where the consumer - * lowers the union itself (strings, equality, truthiness), null for other +/** `xs[i]!` on an array as the operand of a converting use (see + * assertionOperand). The read answers undefined for a missing element, as + * in Node, instead of trapping: a numeric use reads NaN for it, and other + * uses get the optional element value. Returns null for other reads and * uses. */ -function convertingUseOf(lowerer: Lowerer, expr: ts.NonNullExpression): "number" | "value" | null { +function assertedElementOperand( + lowerer: Lowerer, + expr: ts.NonNullExpression, + loc: SrcLoc, +): IrExpr | null { + let read: ts.Expression = expr.expression; + while (ts.isParenthesizedExpression(read)) read = read.expression; + if (!ts.isElementAccessExpression(read) || read.questionDotToken) return null; + // Plain arrays only: regex match results and other array subtypes keep + // their own element reads. + const receiverType = lowerer.typeOf(read.expression); + if ( + !lowerer.checker.isArrayType(receiverType) || + lowerer.mapTypeOf(receiverType)?.kind !== "array" + ) + return null; + // A counter below the length reads an existing index: the read keeps + // its plain element access. + if (lowerer.nanCodedReads?.counterIndexInBounds(read)) return null; + const target = lowerer.mapTypeOf(lowerer.typeOf(expr)); + if (!target || !isScalarArm(target)) return null; + const use = convertingUseOf(lowerer, expr); + if (use === null) return null; + if (use === "string" && target.kind === "string") + return stringElementOrUndefined(lowerer, read, loc); + const probe = lowerAbsenceProbe(lowerer, read); + if (probe?.type.kind !== "union" || !scalarUnion(lowerer, probe.type.unionId)) return null; + if (use === "value" || use === "string") return probe; + return lowerer.stripUndefinedArm(probe.type).kind === "f64" + ? lowerOptionalNumber(lowerer, probe, loc) + : lowerPrimitiveUnionNumber(lowerer, probe, expr, loc); +} + +/** `xs[i]!` on an array as the receiver of a member read or write + * (`xs[i]!.name`, `xs[i]!.size()`, `xs[i]![0]`, `xs[i]!.name = v`). `!` + * performs no runtime check, so a missing element is undefined there and + * the member access throws Node's TypeError ("Cannot read properties of + * undefined (reading 'name')") instead of the bounds trap. The receiver is + * the element when it is present. Returns null for other reads and uses. */ +function dereferencedElementReceiver( + lowerer: Lowerer, + expr: ts.NonNullExpression, + loc: SrcLoc, +): IrExpr | null { + let read: ts.Expression = expr.expression; + while (ts.isParenthesizedExpression(read)) read = read.expression; + if (!ts.isElementAccessExpression(read) || read.questionDotToken) return null; + const receiverType = lowerer.typeOf(read.expression); + if ( + !lowerer.checker.isArrayType(receiverType) || + lowerer.mapTypeOf(receiverType)?.kind !== "array" + ) + return null; + if (lowerer.nanCodedReads?.counterIndexInBounds(read)) return null; + const use = runtimeOptionalUseOf(expr); + if (!use || use.optional || use.complex || use.comma || use.kind === "call") return null; + const message = memberAccessMessage(lowerer, use.access); + if (message === null) return null; + if (lowerer.mapTypeOf(lowerer.typeOf(read.argumentExpression))?.kind !== "f64") return null; + // The element must already have the asserted type: an element union the + // assertion or a guard narrows keeps its checked narrow. + const target = lowerer.mapTypeOf(lowerer.typeOf(expr)); + const declared = lowerer.mapTypeOf(receiverType); + if ( + !target || + declared?.kind !== "array" || + declared.elem.kind === "void" || + !typeEquals(declared.elem, target) + ) + return null; + const arr = lowerer.lowerExpr(read.expression); + if (arr.type.kind !== "array" || !typeEquals(arr.type.elem, target)) return null; + const elem = arr.type.elem; + const index = lowerer.lowerExprExpecting(read.argumentExpression, F64); + // Plain reads (a local, a field of one) evaluate once more instead of + // through a hidden local: the receiver then stays a borrowed element + // read, like the plain `xs[i]` it replaces. + const stmts: IrStmt[] = []; + const stable = (value: IrExpr, name: string): IrExpr => { + if (isSafeToRepeat(value) || plainProjection(value)) return value; + const tmp = lowerer.declareHiddenLocal(name, value.type); + stmts.push({ kind: "varDecl", localId: tmp.id, init: value, loc }); + return varRef(tmp.id, value.type, loc); + }; + const a = stable(arr, "%elementArray"); + const i = stable(index, "%elementIndex"); + const value: IrExpr = { + kind: "ternary", + cond: { + kind: "bin", + op: "===", + left: { kind: "arrayState", arr: a, index: i, type: F64, loc }, + right: numLit(1, loc), + type: BOOL, + loc, + }, + then: { kind: "arrayGet", arr: a, index: i, type: elem, loc }, + else_: { + kind: "libCall", + fn: "error.nodeThrow", + args: [numLit(1, loc), strLit("", loc), message], + type: elem, + loc, + }, + type: elem, + loc, + }; + return stmts.length === 0 ? value : { kind: "seqExpr", stmts, result: value, type: elem, loc }; +} + +/** A local, a number literal, or a field or record member read through a + * local: reading it again has no effect and yields the same value. */ +function plainProjection(e: IrExpr): boolean { + if (e.kind === "varRef" || e.kind === "numLit") return true; + return (e.kind === "fieldGet" || e.kind === "recordGet") && plainProjection(e.obj); +} + +/** Node's TypeError message for a member access on undefined: reading the + * member, or setting it when the access is the target of `=`. Null when + * the key is not a name, a literal, or a plain read of a number or string + * (which the message reads a second time). */ +function memberAccessMessage( + lowerer: Lowerer, + access: ts.PropertyAccessExpression | ts.ElementAccessExpression | ts.CallExpression, +): IrExpr | null { + let outer: ts.Expression = access; + while (ts.isParenthesizedExpression(outer.parent)) outer = outer.parent; + const parent = outer.parent; + const setting = + ts.isBinaryExpression(parent) && + parent.left === outer && + parent.operatorToken.kind === ts.SyntaxKind.EqualsToken; + const head = setting + ? "Cannot set properties of undefined (setting '" + : "Cannot read properties of undefined (reading '"; + const loc = locOf(access); + if (ts.isPropertyAccessExpression(access)) { + if (ts.isPrivateIdentifier(access.name)) return null; + return strLit(`${head}${access.name.text}')`, loc); + } + if (!ts.isElementAccessExpression(access)) return null; + const key = runtimeOptionalElementKey(access.argumentExpression); + if (key !== null) return strLit(`${head}${key}')`, loc); + let keyNode = access.argumentExpression; + while (ts.isParenthesizedExpression(keyNode)) keyNode = keyNode.expression; + if (!ts.isIdentifier(keyNode)) return null; + const keyType = lowerer.mapTypeOf(lowerer.typeOf(keyNode))?.kind; + if (keyType !== "f64" && keyType !== "string") return null; + const keyValue = lowerer.lowerExpr(keyNode); + if (!isSafeToRepeat(keyValue) || keyValue.type.kind !== keyType) return null; + const text: IrExpr = + keyValue.type.kind === "string" + ? keyValue + : { kind: "toString", operand: keyValue, type: STRING, loc }; + const concat = (left: IrExpr, right: IrExpr): IrExpr => ({ + kind: "strConcat", + left, + right, + type: STRING, + loc, + }); + return concat(concat(strLit(head, loc), text), strLit("')", loc)); +} + +/** A string array's element converted to a string: the element, or + * "undefined" for a missing index or a hole. */ +function stringElementOrUndefined( + lowerer: Lowerer, + read: ts.ElementAccessExpression, + loc: SrcLoc, +): IrExpr | null { + if (lowerer.mapTypeOf(lowerer.typeOf(read.argumentExpression))?.kind !== "f64") return null; + const arr = lowerer.lowerExpr(read.expression); + if (arr.type.kind !== "array" || arr.type.elem.kind !== "string") return null; + const index = lowerer.lowerExprExpecting(read.argumentExpression, F64); + const stmts: IrStmt[] = []; + const stable = (value: IrExpr, name: string): IrExpr => { + if (isSafeToRepeat(value)) return value; + const tmp = lowerer.declareHiddenLocal(name, value.type); + stmts.push({ kind: "varDecl", localId: tmp.id, init: value, loc }); + return varRef(tmp.id, value.type, loc); + }; + const a = stable(arr, "%elementArray"); + const i = stable(index, "%elementIndex"); + const value: IrExpr = { + kind: "ternary", + cond: { + kind: "bin", + op: "===", + left: { kind: "arrayState", arr: a, index: i, type: F64, loc }, + right: numLit(1, loc), + type: BOOL, + loc, + }, + then: { kind: "arrayGet", arr: a, index: i, type: STRING, loc }, + else_: strLit("undefined", loc), + type: STRING, + loc, + }; + return stmts.length === 0 ? value : { kind: "seqExpr", stmts, result: value, type: STRING, loc }; +} + +function isScalarArm(arm: IrType): boolean { + return arm.kind === "f64" || arm.kind === "string" || arm.kind === "bool" || isUnitType(arm); +} + +/** A union of numbers, strings, booleans, null and undefined with at least + * one unit arm. */ +function scalarUnion(lowerer: Lowerer, unionId: string): boolean { + const arms = lowerer.unions.get(unionId)?.arms; + return !!arms?.some(isUnitType) && arms.every(isScalarArm); +} + +/** How the converting use of `x!` (see assertionOperand) reads it: + * "number" where the use applies ToNumber, "string" where it converts the + * value to a string (a template span, `+` with a string), "value" where the + * consumer lowers the union itself (equality, truthiness, a comparison with + * a string), null for other uses. */ +function convertingUseOf( + lowerer: Lowerer, + expr: ts.NonNullExpression, +): "number" | "string" | "value" | null { let outer: ts.Expression = expr; while (ts.isParenthesizedExpression(outer.parent)) outer = outer.parent; const parent = outer.parent; - if (ts.isTemplateSpan(parent)) return parent.expression === outer ? "value" : null; + if (ts.isTemplateSpan(parent)) return parent.expression === outer ? "string" : null; if (ts.isTypeOfExpression(parent)) return "value"; if (ts.isPrefixUnaryExpression(parent)) { if (parent.operator === ts.SyntaxKind.ExclamationToken) return "value"; @@ -1139,7 +1357,7 @@ function convertingUseOf(lowerer: Lowerer, expr: ts.NonNullExpression): "number" if (op === ts.SyntaxKind.PlusEqualsToken && parent.right !== outer) return null; let partner = other; while (ts.isParenthesizedExpression(partner)) partner = partner.expression; - if (isString(other)) return ts.isNonNullExpression(partner) ? null : "value"; + if (isString(other)) return ts.isNonNullExpression(partner) ? null : "string"; if (!numericPartner(lowerer.typeOf(other))) return "value"; return isString(expr) ? null : "number"; } @@ -1225,7 +1443,8 @@ export function assertedReadObservesUndefined( lowerer: Lowerer, node: ts.NonNullExpression, ): boolean { - if (convertingUseOf(lowerer, node) === "value") return true; + const use = convertingUseOf(lowerer, node); + if (use === "value" || use === "string") return true; let outer: ts.Expression = node; while (ts.isParenthesizedExpression(outer.parent)) outer = outer.parent; const parent = outer.parent; diff --git a/packages/compiler/src/frontend/lowering/nan-coded-reads.ts b/packages/compiler/src/frontend/lowering/nan-coded-reads.ts index 24c981bd31..9456cfa188 100644 --- a/packages/compiler/src/frontend/lowering/nan-coded-reads.ts +++ b/packages/compiler/src/frontend/lowering/nan-coded-reads.ts @@ -451,6 +451,19 @@ export class NanCodedReads { // ---- present reads ------------------------------------------------------ + /** `xs[i]` whose index is a non-negative integer counter of the reading + * function and that a dominating `i < xs.length` guard bounds by the + * length: the read names an existing index. */ + counterIndexInBounds(read: ts.ElementAccessExpression): boolean { + const index = peelTransparent(read.argumentExpression); + if (!ts.isIdentifier(index)) return false; + const symbol = this.symbolOf(index); + if (!symbol || !this.nonNegativeCounter(symbol)) return false; + const decl = this.host.declarationOf(symbol); + if (!decl || this.containingFunction(decl) !== this.containingFunction(read)) return false; + return indexReadInBounds(read); + } + /** `t[i]` names an existing element: `i` is a non-negative integer * counter and a dominating guard bounds it by `t.length` (or by the * length of a typed array that always has the same length). diff --git a/packages/compiler/src/ir/int32-slots.ts b/packages/compiler/src/ir/int32-slots.ts index 1334b9b997..be5f29768f 100644 --- a/packages/compiler/src/ir/int32-slots.ts +++ b/packages/compiler/src/ir/int32-slots.ts @@ -1484,6 +1484,39 @@ export function analyzeInt32Slots(mod: IrModule, inputs: Int32SlotInputs = {}): continue; } const int32 = (e: IrExpr): boolean => withinInt32(ranges.get(e)); + // The range of an exact integer that may also be NaN: an element read + // of a candidate through `getNumber` (`a[k]!` in arithmetic, NaN for a + // missing element), and `+`, `-` and `*` over such values and integers. + // A checked store widens the array at a NaN like at any value that is + // not an int32. + const integerOrNaN = (e: IrExpr): IntegerRange | null => { + const range = ranges.get(e); + if (range) return range; + if (e.kind === "numLit") + return Number.isInteger(e.value) ? { min: e.value, max: e.value } : null; + if (e.kind === "arrIntrinsic" && e.method === "getNumber" && e.receiver.kind === "varRef") + return slots.hasInt32ElementSlot(int32ArraySlot(int32ArrayKey(fn.name, e.receiver.localId))) + ? INT32_RANGE + : null; + if (e.kind !== "bin" || (e.op !== "+" && e.op !== "-" && e.op !== "*")) return null; + const left = integerOrNaN(e.left); + const right = left && integerOrNaN(e.right); + if (!left || !right) return null; + const bounds = + e.op === "+" + ? [left.min + right.min, left.max + right.max] + : e.op === "-" + ? [left.min - right.max, left.max - right.min] + : [ + left.min * right.min, + left.min * right.max, + left.max * right.min, + left.max * right.max, + ]; + const min = Math.min(...bounds); + const max = Math.max(...bounds); + return Math.abs(min) < 2 ** 53 && Math.abs(max) < 2 ** 53 ? { min, max } : null; + }; // Every element stored into an int32 array candidate must be an int32 // under the facts that assume the candidates' own elements are; a // failing store drops its array's element facts, and the function is @@ -1513,6 +1546,10 @@ export function analyzeInt32Slots(mod: IrModule, inputs: Int32SlotInputs = {}): for (const [id, values] of int32ArrayStores(fn, arrays)) { const key = int32ArrayKey(fn.name, id); if (!slots.hasInt32ElementSlot(int32ArraySlot(key)) || values.every(int32)) continue; + // Checked stores serve a lone local: its allocations are the only + // arrays it holds. Its stores are judged under the element facts + // the ranges were computed with. + const checkable = values.every((value) => int32(value) || integerOrNaN(value) !== null); // The array's group drops together: every function reading a // member is analyzed again without its facts. const group = arrayGroups.group.get(key) ?? [key]; @@ -1523,17 +1560,11 @@ export function analyzeInt32Slots(mod: IrModule, inputs: Int32SlotInputs = {}): if (names) for (const name of names) requeue(name); } else requeue(member.slice(0, member.indexOf("\0"))); } - // Checked stores serve a lone local: its allocations are the only - // arrays it holds. - const checkable = (value: IrExpr): boolean => { - const range = ranges.get(value); - return int32(value) || range != null; - }; if ( group.length === 1 && !key.startsWith("\0") && !params.has(id) && - values.every(checkable) && + checkable && checkableInt32Allocations(fn, id) ) slots.checkedInt32Arrays.add(key); diff --git a/packages/compiler/src/ir/parameter-versions.ts b/packages/compiler/src/ir/parameter-versions.ts index 5156d4bff1..ca29585853 100644 --- a/packages/compiler/src/ir/parameter-versions.ts +++ b/packages/compiler/src/ir/parameter-versions.ts @@ -122,7 +122,7 @@ function numericLoop(loop: IrStmt, functions: ReadonlyMap): const target = functions.get(e.callee); allowed = target !== undefined && simple(target); } else if (e.kind === "libCall") allowed = e.fn.startsWith("math."); - else if (e.kind === "arrIntrinsic") allowed = e.method === "length"; + else if (e.kind === "arrIntrinsic") allowed = e.method === "length" || e.method === "getNumber"; return allowed && everyExprChild(e, expr, stmt); }; const stmt = (s: IrStmt): boolean => diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7393-non-null-element-operands.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7393-non-null-element-operands.ts.json new file mode 100644 index 0000000000..e50ca92f4d --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7393-non-null-element-operands.ts.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/corpus/7393-non-null-element-operands.ts" + ], + "diags": [] +} diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7395-non-null-element-dereference.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7395-non-null-element-dereference.ts.json new file mode 100644 index 0000000000..17551e6266 --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7395-non-null-element-dereference.ts.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/corpus/7395-non-null-element-dereference.ts" + ], + "diags": [] +} diff --git a/tests/corpus/7393-non-null-element-operands.ts b/tests/corpus/7393-non-null-element-operands.ts new file mode 100644 index 0000000000..0fd7190a36 --- /dev/null +++ b/tests/corpus/7393-non-null-element-operands.ts @@ -0,0 +1,81 @@ +// Non-null assertions on array element reads whose index is out of bounds +// or names a hole, used as operands of operators that convert them. `!` +// performs no runtime check, so the read is undefined, as in Node: NaN in +// arithmetic, false in comparisons, "undefined" in strings. + +const values = [3, 1, 4, 1, 5]; +const words = ["alpha", "beta"]; +const flags = [true, false]; +const holey: number[] = []; +holey[3] = 7; + +function show(v: unknown): string { + return Object.is(v, -0) ? "-0" : String(v); +} + +function line(label: string, parts: unknown[]): void { + console.log(label.padEnd(10), parts.map(show).join(" ")); +} + +for (const k of [0, 4, 5, -1, 1.5, 100]) { + line(`k=${k}`, [ + values[k]! + 1, + values[k]! * 2, + -values[k]!, + values[k]! | 0, + values[k]! < 3, + values[k]! >= 1, + values[k]! === 1, + values[k]! !== 1, + `${values[k]!}`, + "v=" + values[k]!, + Math.max(values[k]!, 0), + words[k]! + "!", + `<${words[k]!}>`, + words[k]! < "b", + !flags[k]!, + ]); +} + +line("holes", [holey[0]! + 1, holey[3]! + 1, `${holey[1]!}`, holey[2]! === undefined, holey[9]! > 0]); + +// `+` with an untyped partner concatenates or adds when it runs. +const untypedText: any = "x"; +const untypedNumber: any = 5; +for (const k of [1, 9]) { + line(`untyped ${k}`, [ + values[k]! + untypedText, + untypedText + values[k]!, + values[k]! + untypedNumber, + words[k]! + untypedNumber, + words[k]! + untypedText, + ]); +} + +let sum = 0; +let text = ""; +for (let i = 0; i <= values.length; i++) { + sum += values[i]!; + text += words[i % 3]!; + text += ","; +} +line("compound", [sum, text]); + +// A counter array: an index past the end reads NaN, and the store widens +// the array to doubles. +function counter(n: number, m: number): number[] { + const a = new Array(n).fill(0) as number[]; + for (let i = 0; i < 6000; i++) a[i % m] = a[i % m]! + 1; + return [a[0]!, a[m - 1]!, a.length, a[n + 1]! + 0]; +} +line("counter", counter(5000, 5000)); +line("past", counter(5000, 5003)); +line("zero", counter(4, 0)); + +const grid = [ + [1, 2], + [3, 4], +]; +let total = 0; +for (let r = 0; r < 3; r++) for (let c = 0; c < 3; c++) total += grid[r % 2]![c]! * 1; +line("grid", [total]); diff --git a/tests/corpus/7395-non-null-element-dereference.ts b/tests/corpus/7395-non-null-element-dereference.ts new file mode 100644 index 0000000000..fdae23796c --- /dev/null +++ b/tests/corpus/7395-non-null-element-dereference.ts @@ -0,0 +1,71 @@ +// Member reads and writes through non-null asserted array element reads. +// `!` performs no runtime check: a missing element is undefined, so the +// member access throws Node's TypeError, which the program can catch. + +class Cell { + count = 0; + label: string; + constructor(label: string) { + this.label = label; + } + bump(): number { + return ++this.count; + } +} + +function cell(label: string): Cell { + return new Cell(label); +} + +const cells = [cell("a"), cell("b")]; +const words = ["alpha", "beta"]; +const grid = [ + [1, 2], + [3, 4], +]; +const maps = [new Map([["x", 1]])]; +const holey: Cell[] = []; +holey[2] = cell("z"); +let reads = 0; +function at(n: number): number { + reads++; + return n; +} + +function attempt(label: string, f: () => unknown): void { + try { + console.log(label.padEnd(12), "ok", f()); + } catch (e) { + const error = e as Error; + console.log(label.padEnd(12), error instanceof TypeError, error.message); + } +} + +for (const k of [0, 1, 2, -1]) { + attempt(`label ${k}`, () => cells[k]!.label); + attempt(`bump ${k}`, () => cells[k]!.bump()); + attempt(`set ${k}`, () => (cells[k]!.label = "set")); + attempt(`count++ ${k}`, () => cells[k]!.count++); + attempt(`length ${k}`, () => words[k]!.length); + attempt(`upper ${k}`, () => words[k]!.toUpperCase()); + attempt(`grid ${k}`, () => grid[k]![0]); + attempt(`row ${k}`, () => { + const j = 1; + return grid[k]![j]; + }); + attempt(`push ${k}`, () => grid[k]!.push(9)); + attempt(`map ${k}`, () => maps[k]!.get("x")); +} + +attempt("hole", () => holey[0]!.label); +attempt("hole value", () => holey[2]!.label); +attempt("order", () => cells[at(7)]!.count + at(1)); +console.log("reads", reads); + +let total = 0; +try { + for (let i = 0; i < 4; i++) total += grid[i]![0]!; +} catch (e) { + console.log("loop", (e as Error).message, total); +} +console.log(cells.map((c) => `${c.label}:${c.count}`).join(" ")); diff --git a/tests/harness/corpus-costs/7393-non-null-element-operands.ts.json b/tests/harness/corpus-costs/7393-non-null-element-operands.ts.json new file mode 100644 index 0000000000..e053c13c8a --- /dev/null +++ b/tests/harness/corpus-costs/7393-non-null-element-operands.ts.json @@ -0,0 +1 @@ +{ "plain": 3000, "san": 3800 } diff --git a/tests/harness/corpus-costs/7395-non-null-element-dereference.ts.json b/tests/harness/corpus-costs/7395-non-null-element-dereference.ts.json new file mode 100644 index 0000000000..e0d5a6a6d9 --- /dev/null +++ b/tests/harness/corpus-costs/7395-non-null-element-dereference.ts.json @@ -0,0 +1 @@ +{ "plain": 3585, "san": 5768 } From b926722ad3dfed5879cad9713146b776717a1ac0 Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Mon, 26 Oct 2026 10:16:54 -0500 Subject: [PATCH 2/4] Evaluate writes and indices through missing asserted elements in Node's order `xs[i]!.name = v` threw while it evaluated the receiver, so a missing element skipped the value's side effects and hid an error the value threw. Node evaluates the array, the index, the key and the value, then the write throws "Cannot set properties of undefined". Such a write, in statement or value position, now evaluates them in that order and then tests the element: a missing one throws Node's TypeError, a present one takes the write. The array and the index are held only when the key or the value can assign a binding or field, and a key or value whose evaluation is unobservable stays in the write. A key of any number or string value is named in the message, since the write holds it. An index that replaces the array binding or field (`xs[(xs = other, 0)]!`) ran before the member access and the string conversion read the array, so they read the replacement. The array is now held before such an index, as JavaScript reads it first; an index that cannot assign keeps the plain read. Corpus 7395 covers both orders. --- .../frontend/lowering/expressions/binary.ts | 3 + .../src/frontend/lowering/lower-exprs.ts | 287 +++++++++++++++--- .../src/frontend/lowering/lower-stmts.ts | 29 ++ .../7395-non-null-element-dereference.ts | 42 +++ 4 files changed, 320 insertions(+), 41 deletions(-) diff --git a/packages/compiler/src/frontend/lowering/expressions/binary.ts b/packages/compiler/src/frontend/lowering/expressions/binary.ts index 0d24561dee..f65b7f6ba5 100644 --- a/packages/compiler/src/frontend/lowering/expressions/binary.ts +++ b/packages/compiler/src/frontend/lowering/expressions/binary.ts @@ -37,6 +37,7 @@ import { dynCompoundValue, lowerAbsenceProbe, lowerDynMemberAssignment, + lowerDereferencedElementAssignment, lowerOptionalNumber, lowerPresenceOperand, nullishBranches, @@ -1029,6 +1030,8 @@ export function lowerBinary(lowerer: Lowerer, expr: ts.BinaryExpression): IrExpr if (prototypeWrite) return prototypeWrite; const callableWrite = lowerNativeFunctionAssignment(lowerer, expr); if (callableWrite) return callableWrite; + const dereferenced = lowerDereferencedElementAssignment(lowerer, expr); + if (dereferenced) return dereferenced; } if ( op === ts.SyntaxKind.QuestionQuestionEqualsToken || diff --git a/packages/compiler/src/frontend/lowering/lower-exprs.ts b/packages/compiler/src/frontend/lowering/lower-exprs.ts index 91864a036f..baa3cf9139 100644 --- a/packages/compiler/src/frontend/lowering/lower-exprs.ts +++ b/packages/compiler/src/frontend/lowering/lower-exprs.ts @@ -82,6 +82,7 @@ import { unitOnlyUnion } from "../type-mapper.js"; import { lowerYield } from "./lower-generators.js"; import { unionWideningTags } from "../../ir/analysis.js"; import { isSafeToDiscard, isSafeToRepeat } from "./expressions/evaluation-safety.js"; +import { everyExprChild } from "../../ir/traverse.js"; import { hasOptionalChainGuard, isOptionalChainTail } from "./expressions/optional-chains.js"; import { classToStringConversion, classUnionToString } from "./class-to-primitive.js"; import { lowerCjsTopLevelThis, lowerRequireCacheElement } from "./lower-node-module.js"; @@ -1132,17 +1133,50 @@ function assertedElementOperand( : lowerPrimitiveUnionNumber(lowerer, probe, expr, loc); } -/** `xs[i]!` on an array as the receiver of a member read or write - * (`xs[i]!.name`, `xs[i]!.size()`, `xs[i]![0]`, `xs[i]!.name = v`). `!` - * performs no runtime check, so a missing element is undefined there and - * the member access throws Node's TypeError ("Cannot read properties of - * undefined (reading 'name')") instead of the bounds trap. The receiver is - * the element when it is present. Returns null for other reads and uses. */ +/** `xs[i]!` on an array as the receiver of a member read + * (`xs[i]!.name`, `xs[i]!.size()`, `xs[i]![0]`). `!` performs no runtime + * check, so a missing element is undefined there and the member access + * throws Node's TypeError ("Cannot read properties of undefined (reading + * 'name')") instead of the bounds trap. The receiver is the element when it + * is present. A write (`xs[i]!.name = v`) is lowered as a statement by + * lowerDereferencedElementWrite, which evaluates the value before the + * receiver throws; elsewhere it keeps the bounds trap. Returns null for + * other reads and uses. */ function dereferencedElementReceiver( lowerer: Lowerer, expr: ts.NonNullExpression, loc: SrcLoc, ): IrExpr | null { + const use = runtimeOptionalUseOf(expr); + if (!use || isAssignmentTarget(use.access)) return null; + const parts = dereferencedElementParts(lowerer, expr, loc, false); + if (parts?.message == null) return null; + const message = parts.message; + const value: IrExpr = { + kind: "ternary", + cond: parts.present, + then: parts.element, + else_: nodeTypeError(message, parts.element.type, loc), + type: parts.element.type, + loc, + }; + return parts.stmts.length === 0 + ? value + : { kind: "seqExpr", stmts: parts.stmts, result: value, type: value.type, loc }; +} + +/** The parts of a member access through `xs[i]!` (see + * dereferencedElementReceiver): statements that evaluate the array and the + * index, whether the element is present, the element read, and Node's + * TypeError message for a missing one. `hold` keeps the array and the + * index in hidden locals, for a write whose value is evaluated before the + * element is read. */ +export function dereferencedElementParts( + lowerer: Lowerer, + expr: ts.NonNullExpression, + loc: SrcLoc, + hold: boolean, +): { stmts: IrStmt[]; present: IrExpr; element: IrExpr; message: IrExpr | null } | null { let read: ts.Expression = expr.expression; while (ts.isParenthesizedExpression(read)) read = read.expression; if (!ts.isElementAccessExpression(read) || read.questionDotToken) return null; @@ -1155,8 +1189,14 @@ function dereferencedElementReceiver( if (lowerer.nanCodedReads?.counterIndexInBounds(read)) return null; const use = runtimeOptionalUseOf(expr); if (!use || use.optional || use.complex || use.comma || use.kind === "call") return null; - const message = memberAccessMessage(lowerer, use.access); - if (message === null) return null; + // A write holds its key, so the message can name any number or string + // key (dereferencedElementWrite builds it). + const message = + memberAccessMessage(lowerer, use.access) ?? + (hold && ts.isElementAccessExpression(use.access) && namedKey(lowerer, use.access) + ? null + : undefined); + if (message === undefined) return null; if (lowerer.mapTypeOf(lowerer.typeOf(read.argumentExpression))?.kind !== "f64") return null; // The element must already have the asserted type: an element union the // assertion or a guard narrows keeps its checked narrow. @@ -1175,19 +1215,22 @@ function dereferencedElementReceiver( const index = lowerer.lowerExprExpecting(read.argumentExpression, F64); // Plain reads (a local, a field of one) evaluate once more instead of // through a hidden local: the receiver then stays a borrowed element - // read, like the plain `xs[i]` it replaces. + // read, like the plain `xs[i]` it replaces. An index that can assign + // can replace the array binding or field (`xs[(xs = other, 0)]!`), so the + // array is held before it, as JavaScript reads it first. const stmts: IrStmt[] = []; - const stable = (value: IrExpr, name: string): IrExpr => { - if (isSafeToRepeat(value) || plainProjection(value)) return value; + const plain = (value: IrExpr): boolean => + !hold && (isSafeToRepeat(value) || plainProjection(value)); + const held = (value: IrExpr, name: string): IrExpr => { const tmp = lowerer.declareHiddenLocal(name, value.type); stmts.push({ kind: "varDecl", localId: tmp.id, init: value, loc }); return varRef(tmp.id, value.type, loc); }; - const a = stable(arr, "%elementArray"); - const i = stable(index, "%elementIndex"); - const value: IrExpr = { - kind: "ternary", - cond: { + const a = !mayAssign(index) && plain(arr) ? arr : held(arr, "%elementArray"); + const i = plain(index) ? index : held(index, "%elementIndex"); + return { + stmts, + present: { kind: "bin", op: "===", left: { kind: "arrayState", arr: a, index: i, type: F64, loc }, @@ -1195,18 +1238,167 @@ function dereferencedElementReceiver( type: BOOL, loc, }, - then: { kind: "arrayGet", arr: a, index: i, type: elem, loc }, - else_: { - kind: "libCall", - fn: "error.nodeThrow", - args: [numLit(1, loc), strLit("", loc), message], - type: elem, + element: { kind: "arrayGet", arr: a, index: i, type: elem, loc }, + message, + }; +} + +/** A write through `xs[i]!` (`xs[i]!.name = v`, `xs[i]![k] = v`): as in + * Node, the array, the index, the key and the value evaluate in order (the + * statements), then a missing element makes the write throw Node's + * TypeError and a present one takes the write, which reads the element, + * the key and the value through `overrides`. Null for other targets. */ +export function dereferencedElementWrite( + lowerer: Lowerer, + expr: ts.BinaryExpression, +): { + stmts: IrStmt[]; + present: IrExpr; + message: IrExpr; + overrides: { node: ts.Expression; value: IrExpr }[]; +} | null { + if (expr.operatorToken.kind !== ts.SyntaxKind.EqualsToken) return null; + const target = expr.left; + if ( + !(ts.isPropertyAccessExpression(target) || ts.isElementAccessExpression(target)) || + target.questionDotToken || + lowerer.chainRecvByNode.has(target.expression) + ) + return null; + let receiver: ts.Expression = target.expression; + while (ts.isParenthesizedExpression(receiver)) receiver = receiver.expression; + if (!ts.isNonNullExpression(receiver)) return null; + const loc = locOf(expr); + // The key and the value evaluate before the element is read, so the + // array and the index are held when either can assign one of them. + const keyValue = ts.isElementAccessExpression(target) + ? lowerer.lowerExpr(target.argumentExpression) + : null; + const rightValue = lowerer.lowerExpr(expr.right); + const pure = !mayAssign(rightValue) && (keyValue === null || !mayAssign(keyValue)); + const parts = dereferencedElementParts(lowerer, receiver, loc, !pure); + if (parts === null) return null; + const stmts = [...parts.stmts]; + const held = ( + node: ts.Expression, + value: IrExpr, + name: string, + ): { node: ts.Expression; value: IrExpr } => { + // A key or value whose evaluation is unobservable may evaluate in the + // write itself, after the element is tested. + if (isSafeToRepeat(value) || unobservable(value)) return { node, value }; + // A call without a result evaluates to undefined. + if (value.type.kind === "void") { + stmts.push({ kind: "exprStmt", expr: value, loc }); + return { node, value: { kind: "unitLit", unit: "undefined", type: UNDEFINED_T, loc } }; + } + const tmp = lowerer.declareHiddenLocal(name, value.type); + stmts.push({ kind: "varDecl", localId: tmp.id, init: value, loc }); + return { node, value: varRef(tmp.id, value.type, loc) }; + }; + const overrides = [{ node: target.expression, value: parts.element }]; + let message = parts.message; + if (ts.isElementAccessExpression(target) && keyValue !== null) { + const key = held(target.argumentExpression, keyValue, "%elementWriteKey"); + overrides.push(key); + message ??= keyMessage(messageHead(target), key.value, loc); + } + overrides.push(held(expr.right, rightValue, "%elementWriteValue")); + if (message === null) return null; + return { stmts, present: parts.present, message, overrides }; +} + +/** dereferencedElementWrite in value position (`f(xs[i]!.name = v)`). */ +export function lowerDereferencedElementAssignment( + lowerer: Lowerer, + expr: ts.BinaryExpression, +): IrExpr | null { + const write = dereferencedElementWrite(lowerer, expr); + if (write === null) return null; + const loc = locOf(expr); + const value = lowerer.withExpressionOverrides(write.overrides, () => lowerer.lowerExpr(expr)); + return { + kind: "seqExpr", + stmts: write.stmts, + result: { + kind: "ternary", + cond: write.present, + then: value, + else_: nodeTypeError(write.message, value.type, loc), + type: value.type, loc, }, - type: elem, + type: value.type, loc, }; - return stmts.length === 0 ? value : { kind: "seqExpr", stmts, result: value, type: elem, loc }; +} + +/** Whether evaluating `e` has no effect and cannot throw: a discardable + * expression, or the compiler's element-or-undefined read of one. */ +function unobservable(e: IrExpr): boolean { + if (e.kind === "call" && e.callee.startsWith("%arr.idxOr.")) return e.args.every(unobservable); + return isSafeToDiscard(e); +} + +/** Whether evaluating `e` can assign a binding or a field: it contains an + * assignment, an increment, statements, a suspension, or a call that can + * run program code (a library call only through a function it is given). */ +function mayAssign(e: IrExpr): boolean { + switch (e.kind) { + case "call": + // The compiler's element-or-undefined read and union narrows run no + // program code. + if (!e.callee.startsWith("%arr.idxOr.") && !e.callee.startsWith("%union.narrow.")) + return true; + break; + case "assignExpr": + case "incDec": + case "seqExpr": + case "callValue": + case "virtualCall": + case "dynCall": + case "dynInvoke": + case "ffiCall": + case "new": + case "newValue": + case "awaitExpr": + case "awaitUnionExpr": + case "yieldExpr": + return true; + case "closure": + return false; + case "libCall": + if (e.args.some((arg) => arg.type.kind === "func")) return true; + break; + } + return !everyExprChild( + e, + (child) => !mayAssign(child), + () => false, + ); +} + +/** Node's TypeError, thrown in place of a value of `type`. */ +export function nodeTypeError(message: IrExpr, type: IrType, loc: SrcLoc): IrExpr { + return { + kind: "libCall", + fn: "error.nodeThrow", + args: [numLit(1, loc), strLit("", loc), message], + type, + loc, + }; +} + +/** Whether `access` is the target of `=`. */ +function isAssignmentTarget(access: ts.Expression): boolean { + let outer: ts.Expression = access; + while (ts.isParenthesizedExpression(outer.parent)) outer = outer.parent; + const parent = outer.parent; + return ( + ts.isBinaryExpression(parent) && + parent.left === outer && + parent.operatorToken.kind === ts.SyntaxKind.EqualsToken + ); } /** A local, a number literal, or a field or record member read through a @@ -1224,16 +1416,7 @@ function memberAccessMessage( lowerer: Lowerer, access: ts.PropertyAccessExpression | ts.ElementAccessExpression | ts.CallExpression, ): IrExpr | null { - let outer: ts.Expression = access; - while (ts.isParenthesizedExpression(outer.parent)) outer = outer.parent; - const parent = outer.parent; - const setting = - ts.isBinaryExpression(parent) && - parent.left === outer && - parent.operatorToken.kind === ts.SyntaxKind.EqualsToken; - const head = setting - ? "Cannot set properties of undefined (setting '" - : "Cannot read properties of undefined (reading '"; + const head = messageHead(access); const loc = locOf(access); if (ts.isPropertyAccessExpression(access)) { if (ts.isPrivateIdentifier(access.name)) return null; @@ -1249,10 +1432,31 @@ function memberAccessMessage( if (keyType !== "f64" && keyType !== "string") return null; const keyValue = lowerer.lowerExpr(keyNode); if (!isSafeToRepeat(keyValue) || keyValue.type.kind !== keyType) return null; + return keyMessage(head, keyValue, loc); +} + +/** The start of Node's TypeError message for a member access on undefined: + * setting the member when the access is the target of `=`, else reading + * it. */ +function messageHead(access: ts.Expression): string { + return isAssignmentTarget(access) + ? "Cannot set properties of undefined (setting '" + : "Cannot read properties of undefined (reading '"; +} + +/** Whether `access`'s key is typed a number or a string, which the message + * can name from its value. */ +function namedKey(lowerer: Lowerer, access: ts.ElementAccessExpression): boolean { + const kind = lowerer.mapTypeOf(lowerer.typeOf(access.argumentExpression))?.kind; + return kind === "f64" || kind === "string"; +} + +/** The message naming a key from its value (a number or a string), or null + * for another type. */ +function keyMessage(head: string, key: IrExpr, loc: SrcLoc): IrExpr | null { + if (key.type.kind !== "f64" && key.type.kind !== "string") return null; const text: IrExpr = - keyValue.type.kind === "string" - ? keyValue - : { kind: "toString", operand: keyValue, type: STRING, loc }; + key.type.kind === "string" ? key : { kind: "toString", operand: key, type: STRING, loc }; const concat = (left: IrExpr, right: IrExpr): IrExpr => ({ kind: "strConcat", left, @@ -1275,14 +1479,15 @@ function stringElementOrUndefined( if (arr.type.kind !== "array" || arr.type.elem.kind !== "string") return null; const index = lowerer.lowerExprExpecting(read.argumentExpression, F64); const stmts: IrStmt[] = []; - const stable = (value: IrExpr, name: string): IrExpr => { - if (isSafeToRepeat(value)) return value; + const held = (value: IrExpr, name: string): IrExpr => { const tmp = lowerer.declareHiddenLocal(name, value.type); stmts.push({ kind: "varDecl", localId: tmp.id, init: value, loc }); return varRef(tmp.id, value.type, loc); }; - const a = stable(arr, "%elementArray"); - const i = stable(index, "%elementIndex"); + // An index that can assign can replace the array binding or field, so + // the array is held before it, as JavaScript reads it first. + const a = !mayAssign(index) && isSafeToRepeat(arr) ? arr : held(arr, "%elementArray"); + const i = isSafeToRepeat(index) ? index : held(index, "%elementIndex"); const value: IrExpr = { kind: "ternary", cond: { diff --git a/packages/compiler/src/frontend/lowering/lower-stmts.ts b/packages/compiler/src/frontend/lowering/lower-stmts.ts index fc259bbd86..5af5531a78 100644 --- a/packages/compiler/src/frontend/lowering/lower-stmts.ts +++ b/packages/compiler/src/frontend/lowering/lower-stmts.ts @@ -240,10 +240,12 @@ import { globalSymbolKey } from "./expressions/global-symbols.js"; import { identifiersNamed } from "./identifier-uses.js"; import { lowerShortCircuitAssignment } from "./expressions/nullish-assignment.js"; import { + dereferencedElementWrite, jsHoldsDynObject, lowerEnvironmentKey, lowerNativeFunctionAssignment, lowerStrictEquality, + nodeTypeError, } from "./lower-exprs.js"; import { keptIdentityAssertion } from "./assertion-identity.js"; import { isNativeProxyInitializer } from "./expressions/native-proxy.js"; @@ -7944,6 +7946,29 @@ function lowerCjsSingleValueExport( * assignment, and `({ a, b } = e);` (destructuring ASSIGNMENT, which JS * requires parenthesized in statement position) lands in the * object-literal-target branch below. */ +/** dereferencedElementWrite in statement position. */ +function lowerDereferencedElementWrite(lowerer: Lowerer, expr: ts.BinaryExpression): IrStmt | null { + const write = dereferencedElementWrite(lowerer, expr); + if (write === null) return null; + const loc = locOf(expr); + return { + kind: "block", + body: [ + ...write.stmts, + { + kind: "if", + cond: write.present, + then: [ + lowerer.withExpressionOverrides(write.overrides, () => lowerExprStatement(lowerer, expr)), + ], + else_: [{ kind: "exprStmt", expr: nodeTypeError(write.message, VOID, loc), loc }], + loc, + }, + ], + loc, + }; +} + export function lowerExprStatement(lowerer: Lowerer, expr: ts.Expression): IrStmt { const stmtNode = expr.parent; while (ts.isParenthesizedExpression(expr)) expr = expr.expression; @@ -8206,6 +8231,10 @@ export function lowerExprStatement(lowerer: Lowerer, expr: ts.Expression): IrStm const ex = lowerExpandoAssignStmt(lowerer, expr); if (ex) return ex; } + { + const dereferenced = lowerDereferencedElementWrite(lowerer, expr); + if (dereferenced) return dereferenced; + } if (ts.isElementAccessExpression(expr.left)) return lowerer.lowerElementWrite(expr); if (ts.isPropertyAccessExpression(expr.left)) { fenceNodeModuleMutation(lowerer, expr.left, "assignment"); diff --git a/tests/corpus/7395-non-null-element-dereference.ts b/tests/corpus/7395-non-null-element-dereference.ts index fdae23796c..41eb6d1db8 100644 --- a/tests/corpus/7395-non-null-element-dereference.ts +++ b/tests/corpus/7395-non-null-element-dereference.ts @@ -69,3 +69,45 @@ try { console.log("loop", (e as Error).message, total); } console.log(cells.map((c) => `${c.label}:${c.count}`).join(" ")); + +// A write through a missing element evaluates the key and the value before +// it throws; an error the value throws is the one that propagates. +const events: string[] = []; +const note = (event: string, value: number): number => { + events.push(event); + return value; +}; +attempt("write order", () => { + cells[note("index", 7)]!.count = note("value", 1); +}); +attempt("write key order", () => { + grid[note("row", 9)]![note("column", 0)] = note("cell", 5); +}); +attempt("write literal key", () => { + grid[9]![0] = note("cell 0", 5); +}); +attempt("value throws", () => { + cells[7]!.count = (() => { + throw new Error("from the value"); + })(); +}); +attempt("present write", () => { + cells[0]!.count = note("present", 41) + 1; + return cells[0]!.count; +}); +console.log("events", events.join(",")); + +// The array is read before an index that replaces it. +let current = [new Cell("first")]; +const replace = (): number => { + current = []; + return 0; +}; +attempt("replaced read", () => current[replace()]!.label); +current = [new Cell("second")]; +attempt("replaced write", () => { + current[replace()]!.label = "written"; +}); +let names = ["kept"]; +attempt("replaced string", () => `${names[(names = [], 0)]!}`); +console.log(current.length, names.length); From 6d8f0558ff4bf52040c500250ff750d9350db5f3 Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Mon, 26 Oct 2026 15:45:29 -0500 Subject: [PATCH 3/4] Read int32-slot arrays at their width in integer clones --- .../src/backend/llvm/dense-array-access.ts | 3 ++ .../src/backend/llvm/integer-clones.ts | 33 ++++++++++++++- .../corpus/integer-clone-int32-slots.ts.json | 6 +++ tests/corpus/integer-clone-int32-slots.ts | 40 +++++++++++++++++++ .../integer-clone-int32-slots.ts.json | 1 + 5 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/integer-clone-int32-slots.ts.json create mode 100644 tests/corpus/integer-clone-int32-slots.ts create mode 100644 tests/harness/corpus-costs/integer-clone-int32-slots.ts.json diff --git a/packages/compiler/src/backend/llvm/dense-array-access.ts b/packages/compiler/src/backend/llvm/dense-array-access.ts index 0991c344ca..b1e65a23fc 100644 --- a/packages/compiler/src/backend/llvm/dense-array-access.ts +++ b/packages/compiler/src/backend/llvm/dense-array-access.ts @@ -301,6 +301,9 @@ function loadState(host: LlvmEmitterContext, guard: DenseGuard): string { return state; } +/** The ScrElemKind of double arrays (scr_array.h). */ +export const SCR_ELEM_F64 = 0; + /** The ScrElemKind of int32 arrays (scr_array.h). */ export const SCR_ELEM_I32 = 7; diff --git a/packages/compiler/src/backend/llvm/integer-clones.ts b/packages/compiler/src/backend/llvm/integer-clones.ts index 3f3f86e220..0ea3947b21 100644 --- a/packages/compiler/src/backend/llvm/integer-clones.ts +++ b/packages/compiler/src/backend/llvm/integer-clones.ts @@ -3,6 +3,7 @@ import type { IrExpr, IrFunction, IrLocal, IrModule, IrStmt, IrType } from "../. import { mangleFunction } from "../mangle.js"; import { BlockBuilder } from "./blocks.js"; import { SCR_ARR_FIELD } from "./common.js"; +import { SCR_ELEM_F64, SCR_ELEM_I32 } from "./dense-array-access.js"; /** Speculative integer clones of pure number-valued functions. * @@ -1051,9 +1052,39 @@ class CloneEmitter { ); B.line(`${data} = load ptr, ptr ${dataField}`); B.line(`${offset} = zext i32 ${index} to i64`); + // A number array keeps 8-byte doubles, or 4-byte int32 slots when it is + // proven to hold only int32s (SCR_ELEM_I32); any other storage bails. + const kindField = B.tmp(); + const kind = B.tmp(); + const isInt32 = B.tmp(); + B.line( + `${kindField} = getelementptr inbounds %ScrArr, ptr ${array}, i32 0, i32 ${SCR_ARR_FIELD.elem}`, + ); + B.line(`${kind} = load i8, ptr ${kindField}`); + B.line(`${isInt32} = icmp eq i8 ${kind}, ${SCR_ELEM_I32}`); + const int32Slots = B.newLabel("elem.i32"); + const doubleSlots = B.newLabel("elem.f64"); + const join = B.newLabel("elem.join"); + B.condBr(isInt32, int32Slots, doubleSlots); + B.startBlock(int32Slots); + const int32Slot = B.tmp(); + const int32 = B.tmp(); + B.line(`${int32Slot} = getelementptr inbounds i32, ptr ${data}, i64 ${offset}`); + B.line(`${int32} = load i32, ptr ${int32Slot}`); + B.br(join); + B.startBlock(doubleSlots); + const isDouble = B.tmp(); + B.line(`${isDouble} = icmp eq i8 ${kind}, ${SCR_ELEM_F64}`); + this.require(isDouble); B.line(`${slot} = getelementptr inbounds double, ptr ${data}, i64 ${offset}`); B.line(`${number} = load double, ptr ${slot}`); - return this.exactInt32(number); + const exact = this.exactInt32(number); + const doubleEnd = B.currentLabel(); + B.br(join); + B.startBlock(join); + const value = B.tmp(); + B.line(`${value} = phi i32 [ ${int32}, %${int32Slots} ], [ ${exact}, %${doubleEnd} ]`); + return value; } /** The int32 the double `number` equals bit for bit (never -0, NaN, a diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/integer-clone-int32-slots.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/integer-clone-int32-slots.ts.json new file mode 100644 index 0000000000..e26a5a727b --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/integer-clone-int32-slots.ts.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/corpus/integer-clone-int32-slots.ts" + ], + "diags": [] +} diff --git a/tests/corpus/integer-clone-int32-slots.ts b/tests/corpus/integer-clone-int32-slots.ts new file mode 100644 index 0000000000..5c34ada677 --- /dev/null +++ b/tests/corpus/integer-clone-int32-slots.ts @@ -0,0 +1,40 @@ +// A recursive function over a number array runs as an integer clone. +// sumFrom only receives arrays that hold int32s, which keep 4-byte int32 +// slots, and the clone reads them at that width, near their end too. +// sumWide receives an array that also holds a fraction, with 8-byte slots. +function sumFrom(xs: number[], i: number): number { + if (i >= xs.length) return 0; + return xs[i]! + sumFrom(xs, i + 1); +} + +function masked(n: number, start: number): number { + const a: number[] = []; + for (let i = 0; i < n; i++) a.push(i & 7); + return sumFrom(a, start); +} + +function halves(n: number, start: number): number { + const a: number[] = []; + for (let i = 0; i < n; i++) a.push(i < n / 2 ? 0 : 3); + return sumFrom(a, start); +} + +function sumWide(xs: number[], i: number): number { + if (i >= xs.length) return 0; + return xs[i]! + sumWide(xs, i + 1); +} + +function widened(n: number, start: number): number { + const a: number[] = []; + for (let i = 0; i < n; i++) a.push(i % 5); + const before = sumWide(a, start); + a.push(0.5); + return before * 10 + sumWide(a, start); +} + +const { n } = JSON.parse('{"n": 1000}') as { n: number }; +for (const start of [0, 1, n / 2, n - 9, n - 2, n - 1, n]) + console.log(start, masked(n | 0, start | 0), halves(n | 0, start | 0)); +for (const size of [63, 64, 65]) + console.log(size, halves(size, size - 24), halves(size, size - 1), masked(size, size - 3)); +console.log(widened(40, 0), widened(40, 37)); diff --git a/tests/harness/corpus-costs/integer-clone-int32-slots.ts.json b/tests/harness/corpus-costs/integer-clone-int32-slots.ts.json new file mode 100644 index 0000000000..91deb4042c --- /dev/null +++ b/tests/harness/corpus-costs/integer-clone-int32-slots.ts.json @@ -0,0 +1 @@ +{ "plain": 3474, "san": 38229 } From 19b0b40486ddd3c38b681f4c4fe1c76f6456b96e Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Mon, 26 Oct 2026 18:01:27 -0500 Subject: [PATCH 4/4] Address review feedback --- .../src/frontend/lowering/lower-exprs.ts | 99 ++++++++++++++----- .../7395-non-null-element-dereference.ts | 29 ++++++ 2 files changed, 101 insertions(+), 27 deletions(-) diff --git a/packages/compiler/src/frontend/lowering/lower-exprs.ts b/packages/compiler/src/frontend/lowering/lower-exprs.ts index baa3cf9139..76ef8a7042 100644 --- a/packages/compiler/src/frontend/lowering/lower-exprs.ts +++ b/packages/compiler/src/frontend/lowering/lower-exprs.ts @@ -83,6 +83,7 @@ import { lowerYield } from "./lower-generators.js"; import { unionWideningTags } from "../../ir/analysis.js"; import { isSafeToDiscard, isSafeToRepeat } from "./expressions/evaluation-safety.js"; import { everyExprChild } from "../../ir/traverse.js"; +import { declareContextLocal } from "./function-context.js"; import { hasOptionalChainGuard, isOptionalChainTail } from "./expressions/optional-chains.js"; import { classToStringConversion, classUnionToString } from "./class-to-primitive.js"; import { lowerCjsTopLevelThis, lowerRequireCacheElement } from "./lower-node-module.js"; @@ -1168,14 +1169,13 @@ function dereferencedElementReceiver( /** The parts of a member access through `xs[i]!` (see * dereferencedElementReceiver): statements that evaluate the array and the * index, whether the element is present, the element read, and Node's - * TypeError message for a missing one. `hold` keeps the array and the - * index in hidden locals, for a write whose value is evaluated before the - * element is read. */ + * TypeError message for a missing one. A `write` holds its key, so its + * message may be left for dereferencedElementWrite to build from the key. */ export function dereferencedElementParts( lowerer: Lowerer, expr: ts.NonNullExpression, loc: SrcLoc, - hold: boolean, + write: boolean, ): { stmts: IrStmt[]; present: IrExpr; element: IrExpr; message: IrExpr | null } | null { let read: ts.Expression = expr.expression; while (ts.isParenthesizedExpression(read)) read = read.expression; @@ -1193,7 +1193,7 @@ export function dereferencedElementParts( // key (dereferencedElementWrite builds it). const message = memberAccessMessage(lowerer, use.access) ?? - (hold && ts.isElementAccessExpression(use.access) && namedKey(lowerer, use.access) + (write && ts.isElementAccessExpression(use.access) && namedKey(lowerer, use.access) ? null : undefined); if (message === undefined) return null; @@ -1219,8 +1219,7 @@ export function dereferencedElementParts( // can replace the array binding or field (`xs[(xs = other, 0)]!`), so the // array is held before it, as JavaScript reads it first. const stmts: IrStmt[] = []; - const plain = (value: IrExpr): boolean => - !hold && (isSafeToRepeat(value) || plainProjection(value)); + const plain = (value: IrExpr): boolean => isSafeToRepeat(value) || plainProjection(value); const held = (value: IrExpr, name: string): IrExpr => { const tmp = lowerer.declareHiddenLocal(name, value.type); stmts.push({ kind: "varDecl", localId: tmp.id, init: value, loc }); @@ -1269,43 +1268,89 @@ export function dereferencedElementWrite( while (ts.isParenthesizedExpression(receiver)) receiver = receiver.expression; if (!ts.isNonNullExpression(receiver)) return null; const loc = locOf(expr); - // The key and the value evaluate before the element is read, so the - // array and the index are held when either can assign one of them. const keyValue = ts.isElementAccessExpression(target) ? lowerer.lowerExpr(target.argumentExpression) : null; const rightValue = lowerer.lowerExpr(expr.right); - const pure = !mayAssign(rightValue) && (keyValue === null || !mayAssign(keyValue)); - const parts = dereferencedElementParts(lowerer, receiver, loc, !pure); + const parts = dereferencedElementParts(lowerer, receiver, loc, true); if (parts === null) return null; const stmts = [...parts.stmts]; - const held = ( - node: ts.Expression, - value: IrExpr, - name: string, - ): { node: ts.Expression; value: IrExpr } => { - // A key or value whose evaluation is unobservable may evaluate in the - // write itself, after the element is tested. - if (isSafeToRepeat(value) || unobservable(value)) return { node, value }; + let present = parts.present; + let element = parts.element; + // JavaScript reads the element before the key and the value. When either + // has an effect, which can replace, remove or add the element, its + // presence and value are read first; otherwise the write tests and reads + // it after them. + if (!unobservable(rightValue) || (keyValue !== null && !unobservable(keyValue))) { + const presentLocal = lowerer.declareHiddenLocal("%elementPresent", BOOL); + // Assigned only when the element is present. + const elementLocal = declareContextLocal( + lowerer.ctx, + "%elementValue", + element.type, + true, + undefined, + undefined, + ); + stmts.push( + { kind: "varDecl", localId: presentLocal.id, init: present, loc }, + { kind: "varDecl", localId: elementLocal.id, init: null, loc }, + ); + present = varRef(presentLocal.id, BOOL, loc); + stmts.push({ + kind: "if", + cond: present, + then: [{ kind: "assign", localId: elementLocal.id, value: element, loc }], + else_: null, + loc, + }); + element = varRef(elementLocal.id, element.type, loc); + } + const overrides: { node: ts.Expression; value: IrExpr }[] = []; + // The write lowers the node with or without its parentheses. + const override = (node: ts.Expression, value: IrExpr): void => { + for (let n: ts.Expression = node; ; n = n.expression) { + overrides.push({ node: n, value }); + if (!ts.isParenthesizedExpression(n)) break; + } + }; + const held = (value: IrExpr, name: string, inline: boolean): IrExpr => { + if (inline) return value; // A call without a result evaluates to undefined. if (value.type.kind === "void") { stmts.push({ kind: "exprStmt", expr: value, loc }); - return { node, value: { kind: "unitLit", unit: "undefined", type: UNDEFINED_T, loc } }; + return { kind: "unitLit", unit: "undefined", type: UNDEFINED_T, loc }; } const tmp = lowerer.declareHiddenLocal(name, value.type); stmts.push({ kind: "varDecl", localId: tmp.id, init: value, loc }); - return { node, value: varRef(tmp.id, value.type, loc) }; + return varRef(tmp.id, value.type, loc); }; - const overrides = [{ node: target.expression, value: parts.element }]; + override(target.expression, element); let message = parts.message; if (ts.isElementAccessExpression(target) && keyValue !== null) { - const key = held(target.argumentExpression, keyValue, "%elementWriteKey"); - overrides.push(key); - message ??= keyMessage(messageHead(target), key.value, loc); + // The key evaluates before the value, which can assign the binding or + // field it reads; a literal, or an unobservable key the value cannot + // change, may evaluate in the write. + const key = held( + keyValue, + "%elementWriteKey", + keyValue.kind === "numLit" || + keyValue.kind === "strLit" || + (unobservable(keyValue) && !mayAssign(rightValue)), + ); + override(target.argumentExpression, key); + // The message names the key's value as the write evaluated it. + if (key !== keyValue || message === null) + message = keyMessage(messageHead(target), key, loc) ?? message; } - overrides.push(held(expr.right, rightValue, "%elementWriteValue")); + // A value whose evaluation is unobservable may evaluate in the write + // itself, after the element is tested. + override( + expr.right, + held(rightValue, "%elementWriteValue", isSafeToRepeat(rightValue) || unobservable(rightValue)), + ); if (message === null) return null; - return { stmts, present: parts.present, message, overrides }; + return { stmts, present, message, overrides }; } /** dereferencedElementWrite in value position (`f(xs[i]!.name = v)`). */ diff --git a/tests/corpus/7395-non-null-element-dereference.ts b/tests/corpus/7395-non-null-element-dereference.ts index 41eb6d1db8..67d9249d18 100644 --- a/tests/corpus/7395-non-null-element-dereference.ts +++ b/tests/corpus/7395-non-null-element-dereference.ts @@ -111,3 +111,32 @@ attempt("replaced write", () => { let names = ["kept"]; attempt("replaced string", () => `${names[(names = [], 0)]!}`); console.log(current.length, names.length); + +// The element is read before the key and the value: a value that replaces, +// removes or adds it writes to, or misses, the element the target read, and +// a key the value reassigns is the one read before it. +const slots = [new Cell("p"), new Cell("q")]; +const original = slots[0]!; +attempt("value replaces", () => { + slots[0]!.label = ((slots[0] = new Cell("r")), "set"); + return `${original.label} ${slots[0]!.label}`; +}); +const tail = slots[1]!; +attempt("value removes", () => { + slots[1]!.label = (slots.pop(), "gone"); + return `${tail.label} ${slots.length}`; +}); +attempt("value adds", () => { + slots[1]!.label = (slots.push(new Cell("s")), "added"); +}); +const table: number[][] = [[1, 2]]; +let column = 0; +attempt("value moves key", () => { + table[0]![column] = ((column = 1), 5); + return table[0]!.join(","); +}); +column = 0; +attempt("value moves missing key", () => { + table[4]![column] = ((column = 1), 5); +}); +console.log(slots.length, column);