From 98006639b79d166920ccf8a3192fa76d34f61d4c Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Wed, 28 Oct 2026 02:19:44 -0500 Subject: [PATCH] Read the receiver first when an element write's index can rebind it An element write of a `T | undefined` binding holds a computed index in a temporary ahead of its two stores, which then read the receiver after the index. When the index can rebind the receiver (`a[(a = b, 0)] = u`), the write now keeps reading the receiver first, so it stores into the array the receiver held before the index ran. The key conversion of a union index (a sequence that declares a temporary and may read the constant "undefined" key) no longer counts as running other code, so such an index is still held. --- .../src/frontend/lowering/array-values.ts | 19 ++++++++++++++++++- .../lowering/expressions/element-access.ts | 8 ++++++-- tests/corpus/union-local-copies.ts | 14 ++++++++++++++ 3 files changed, 38 insertions(+), 3 deletions(-) diff --git a/packages/compiler/src/frontend/lowering/array-values.ts b/packages/compiler/src/frontend/lowering/array-values.ts index a8c2d699b..f8c25a5fa 100644 --- a/packages/compiler/src/frontend/lowering/array-values.ts +++ b/packages/compiler/src/frontend/lowering/array-values.ts @@ -260,6 +260,9 @@ const PURE_KINDS = new Set([ "unionIsTag", "unionNarrow", "unionWrap", + // Its statements are checked on their own (declarations and assignments + // are recorded writes). + "seqExpr", ]); /** The locals evaluating `value` may assign, and whether it may run other @@ -273,7 +276,11 @@ function valueEffects(lowerer: Lowerer, value: IrExpr): { writes: Set; o everyExpr(value, { expr: (e) => { if (e.kind === "assignExpr" || e.kind === "incDec") writes.add(e.localId); - else if (e.kind === "call" ? !readers.has(e.callee) : !PURE_KINDS.has(e.kind)) opaque = true; + else if (e.kind === "libCall") { + // The "undefined" key of an undefined index is a constant. + if (e.fn !== "array.undefinedKey") opaque = true; + } else if (e.kind === "call" ? !readers.has(e.callee) : !PURE_KINDS.has(e.kind)) + opaque = true; return true; }, stmt: (s) => { @@ -314,6 +321,16 @@ function readsMutableLocal(lowerer: Lowerer, e: IrExpr): boolean { }); } +/** Whether `receiver` still reads the same value after `index` runs: the + * index assigns none of the bindings it reads, and runs no code that could + * assign them through a closure. */ +export function unaffectedByIndex(lowerer: Lowerer, receiver: IrExpr, index: IrExpr): boolean { + const effects = valueEffects(lowerer, index); + return ( + unaffectedBy(receiver, effects) && !(effects.opaque && readsMutableLocal(lowerer, receiver)) + ); +} + /** A number, string or boolean element type. Arrays of it hold a present * undefined in the element's state, so `(T | undefined)[]` stores T * elements (the type mapper) and a T[] seen as one stays the same array. */ diff --git a/packages/compiler/src/frontend/lowering/expressions/element-access.ts b/packages/compiler/src/frontend/lowering/expressions/element-access.ts index f8688e074..e40b9a00f 100644 --- a/packages/compiler/src/frontend/lowering/expressions/element-access.ts +++ b/packages/compiler/src/frontend/lowering/expressions/element-access.ts @@ -66,6 +66,7 @@ import { lowerSafeIndexRead, primitiveElement, unionArrayValueRead, + unaffectedByIndex, } from "../array-values.js"; import { lowerOptionalStringIndex, lowerUnionStringIndex } from "../string-index.js"; import { InternalCompilerError } from "../../../errors.js"; @@ -2479,7 +2480,9 @@ export function lowerElementWrite(lowerer: Lowerer, expr: ts.BinaryExpression): // A binding's `T | undefined` value is stored by two writes that both // read the index (arrayValueStore). A computed index, such as the key // conversion of a union index that declares a temporary, is held once - // ahead of them, after the receiver and before the value as in source order. + // ahead of them. The stores then read the receiver after the index, so + // an index that can rebind the receiver (`a[(a = b, 0)] = u`) keeps the + // stores as they are, reading the receiver first. if ( stored.kind === "varRef" && stored.type.kind === "union" && @@ -2487,7 +2490,8 @@ export function lowerElementWrite(lowerer: Lowerer, expr: ts.BinaryExpression): !typeEquals(stored.type, arr.type.elem) && arr.kind === "varRef" && index.kind !== "varRef" && - index.kind !== "numLit" + index.kind !== "numLit" && + unaffectedByIndex(lowerer, arr, index) ) { const loc = locOf(expr); const held = lowerer.declareHiddenLocal("%arrayIndex", index.type); diff --git a/tests/corpus/union-local-copies.ts b/tests/corpus/union-local-copies.ts index aa83b6e89..7dfc88ca9 100644 --- a/tests/corpus/union-local-copies.ts +++ b/tests/corpus/union-local-copies.ts @@ -179,3 +179,17 @@ function nested(grid: number[][]): number { return s; } console.log(nested([[1, 2], [3]])); + +// The index rebinds the receiver: the store goes to the array the receiver +// held before the index ran, as for any element assignment. +let target: number[] = [1, 2, 3]; +const original = target; +const other: number[] = [7, 8, 9]; +const keyed: number[] = [1]; +const missing = sparse[1]; +const kept = sparse[0]; +target[(target = other, keyed[0])] = missing; +console.log(original, target === other, 1 in original, other); +target = original; +target[(target = other, keyed[0])] = kept; +console.log(original, other);