From c75cc167dde84b6922ebba8cb623e875c736dc48 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Thu, 8 Oct 2026 23:27:46 -0700 Subject: [PATCH] Append runtime-optional push values in place `a.push(v)` where v may hold undefined at run time (an unchecked array read or a hole-observing for-of binding, as in tsc-ts's filterType loop) built a temporary one-element array, stored v or present undefined into it, and spread it onto the receiver: an allocation, a growth, a copy and two releases per append. Such pushes now evaluate every argument first, then append each present value through the ordinary push (and its inline dense fast path) and each missing one as a present-undefined slot at the current length, which is what Node stores. Pure receivers and arguments are re-read instead of copied into owned temporaries. Call arguments read from arrays of nullable references now take the inline dense lookup like other projections (the value is the element pointer itself, so the path is a few loads), and scr_arr_peek_ref answers dense in-range indices before the general lookup. compareSymbols' `declarations[0]!` arguments no longer leave compiled code. --- .../src/backend/llvm/local-array-reads.ts | 12 ++- .../lowering/containers/array-methods.ts | 75 ++++++++++++- .../test/ts7/baselines/order-parity.json | 6 ++ packages/runtime/src/scr_array.c | 9 ++ tests/corpus/4481-array-optional-push.ts | 101 ++++++++++++++++++ 5 files changed, 197 insertions(+), 6 deletions(-) create mode 100644 tests/corpus/4481-array-optional-push.ts diff --git a/packages/compiler/src/backend/llvm/local-array-reads.ts b/packages/compiler/src/backend/llvm/local-array-reads.ts index caf8a6f54..b95d1b85f 100644 --- a/packages/compiler/src/backend/llvm/local-array-reads.ts +++ b/packages/compiler/src/backend/llvm/local-array-reads.ts @@ -257,15 +257,17 @@ export function findCallArrayReads( export function emitCallArrayRead( host: LlvmEmitterContext, read: LocalArrayRead, - inline = read.borrow === true, + inline?: boolean, ): LlValue { const slot = host.B.slot(); host.B.entryAllocas.push(`${slot} = alloca ptr`); // Snapshot arguments already cross a runtime ownership boundary. Keep - // their lookup compact instead of duplicating the dense fast path at - // every call site; proven borrowed reads and immediate projections - // (tests, narrowing, comparisons) still expose that path to LLVM. - const owner = emitLocalArrayRead(host, read, slot, inline); + // boxed lookups compact instead of duplicating the dense fast path at + // every call site; proven borrowed reads, immediate projections (tests, + // narrowing, comparisons), and nullable references (the element pointer + // itself, so the dense path is a few loads) still expose it to LLVM. + const nullable = read.type.kind === "union" && host.nullableUnions.has(read.type.unionId); + const owner = emitLocalArrayRead(host, read, slot, inline ?? (read.borrow === true || nullable)); if (owner) host.ownSlot(owner.slot, owner.type); const value = host.B.tmp(); host.B.line(`${value} = load ptr, ptr ${slot}`); diff --git a/packages/compiler/src/frontend/lowering/containers/array-methods.ts b/packages/compiler/src/frontend/lowering/containers/array-methods.ts index ae0be75ad..20c791b36 100644 --- a/packages/compiler/src/frontend/lowering/containers/array-methods.ts +++ b/packages/compiler/src/frontend/lowering/containers/array-methods.ts @@ -23,6 +23,7 @@ import { } from "../../../ir/ir.js"; import { ARRAY_METHODS } from "../surfaces.js"; import { tryLowerExpression } from "../expressions/try-lower-expression.js"; +import { isSafeToRepeat } from "../expressions/evaluation-safety.js"; import { isJsSourceFile, locOf } from "../../program.js"; import { islandPrimitiveExit, lowerDynDispatchMethodCall } from "../lower-calls.js"; import { @@ -59,6 +60,69 @@ import { requireProducedArrayElement, } from "./callback-arguments.js"; +/** `a.push(v, ...)` where some argument is a runtime-optional read of the + * element type (an unchecked array read or a hole-observing iteration + * binding). Every argument evaluates before the first append; a present + * value takes the ordinary push (and its inline dense fast path), while a + * missing one appends a present-undefined slot at the current length, + * exactly what Node stores. The result is the final length. Pure + * receivers and arguments are repeated instead of copied into temporaries: + * appends run no user code, so repeated reads observe the same values. */ +function lowerOptionalArrayPush( + lowerer: Lowerer, + receiver: IrExpr, + values: IrExpr[], + elem: IrType, + loc: SrcLoc, +): IrExpr { + const stmts: IrStmt[] = []; + const pure = isSafeToRepeat(receiver) && values.every(isSafeToRepeat); + const stable = (value: IrExpr, name: string): IrExpr => { + if (pure) return value; + const temp = lowerer.declareHiddenLocal(name, value.type); + stmts.push({ kind: "varDecl", localId: temp.id, init: value, loc }); + return varRef(temp.id, value.type, loc); + }; + const target = stable(receiver, "%pushTarget"); + const args = values.map((value) => stable(value, "%pushValue")); + const length = (): IrExpr => ({ + kind: "arrIntrinsic", + method: "length", + receiver: target, + args: [], + type: F64, + loc, + }); + const push = (value: IrExpr): IrStmt => ({ + kind: "exprStmt", + expr: { kind: "arrIntrinsic", method: "push", receiver: target, args: [value], type: F64, loc }, + loc, + }); + for (const value of args) { + if (value.type.kind === "union" && lowerer.runtimeOptionalWidening(value.type, elem) !== null) { + const unionId = value.type.unionId; + stmts.push({ + kind: "if", + cond: { + kind: "unionIsTag", + unionId, + tag: lowerer.armTag(unionId, UNDEFINED_T), + negated: false, + value, + type: BOOL, + loc, + }, + then: [{ kind: "arraySetUndefined", arr: target, index: length(), loc }], + else_: [push(lowerer.coerceToExpected(value, elem))], + loc, + }); + } else { + stmts.push(push(lowerer.coerceToExpected(value, elem))); + } + } + return { kind: "seqExpr", stmts, result: length(), type: F64, loc }; +} + function primitivePositionType(lowerer: Lowerer, type: IrType): boolean { if (type.kind === "union") { return ( @@ -670,6 +734,15 @@ export function lowerArrayMethodCall( (probe) => probe !== null && lowerer.runtimeOptionalWidening(probe.type, elem) !== null, ) ) { + if (name === "push") { + return lowerOptionalArrayPush( + lowerer, + receiver, + call.arguments.map((arg) => lowerer.lowerExpr(arg)), + elem, + loc, + ); + } const items = lowerArrayValueItems( lowerer, call.arguments.map((arg) => lowerer.lowerExpr(arg)), @@ -679,7 +752,7 @@ export function lowerArrayMethodCall( ); return { kind: "arrIntrinsic", - method: name === "push" ? "pushSpread" : "unshiftSpread", + method: "unshiftSpread", receiver, args: [items], type: F64, diff --git a/packages/compiler/test/ts7/baselines/order-parity.json b/packages/compiler/test/ts7/baselines/order-parity.json index 6fa10a6cb..f95d52619 100644 --- a/packages/compiler/test/ts7/baselines/order-parity.json +++ b/packages/compiler/test/ts7/baselines/order-parity.json @@ -15478,6 +15478,12 @@ "/tests/corpus/4478-string-large-index-reads.ts" ], "diags": [] + }, + "/tests/corpus/4481-array-optional-push.ts": { + "order": [ + "/tests/corpus/4481-array-optional-push.ts" + ], + "diags": [] } } } diff --git a/packages/runtime/src/scr_array.c b/packages/runtime/src/scr_array.c index 779e1279c..01d0556d6 100644 --- a/packages/runtime/src/scr_array.c +++ b/packages/runtime/src/scr_array.c @@ -1068,6 +1068,15 @@ static bool scr_arr_read_value(const ScrArr *a, double i, uint64_t *slot) { } void *scr_arr_peek_ref(const ScrArr *a, double i) { + /* Dense in-range canonical indices answer from the state byte without + * the general lookup; the range check precedes the cast. */ + if (i >= 0 && i < (double)a->cap) { + size_t idx = (size_t)i; + if ((double)idx == i) { + if (idx >= a->len || a->present[idx] != SCR_ARR_VALUE) return NULL; + return scr_slot_to_ptr(a->data[idx]); + } + } uint64_t slot; return scr_arr_read_value(a, i, &slot) ? scr_slot_to_ptr(slot) : NULL; } diff --git a/tests/corpus/4481-array-optional-push.ts b/tests/corpus/4481-array-optional-push.ts new file mode 100644 index 000000000..ba13cf155 --- /dev/null +++ b/tests/corpus/4481-array-optional-push.ts @@ -0,0 +1,101 @@ +// push() of a value that may be a hole or missing read appends directly to +// the receiver: present values take the ordinary append, missing ones a +// present-undefined slot. Holes, lengths, return values, evaluation order, +// and reference counts must match Node. + +class Item { + readonly id: number; + readonly label: string; + constructor(id: number, label: string) { + this.id = id; + this.label = label; + } +} + +function describe(items: readonly Item[]): string { + const parts: string[] = []; + for (let i = 0; i < items.length; i++) { + parts.push(i in items ? (items[i] === undefined ? "undef" : items[i]!.label) : "hole"); + } + return `${items.length}:[${parts.join(",")}]`; +} + +const a = new Item(1, "a"); +const b = new Item(2, "b"); +const c = new Item(3, "c"); + +// for-of observes holes as undefined; pushing them stores present undefined. +const sparse: Item[] = [a]; +sparse[3] = b; +sparse.push(c); +const copied: Item[] = []; +let lengths = 0; +for (const item of sparse) lengths += copied.push(item); +console.log("for-of copy", describe(copied), lengths, 1 in copied, 2 in copied); + +// The filterType shape: keep the constituents a predicate accepts. +function filterItems(items: Item[], keep: (item: Item) => boolean): Item[] { + const kept: Item[] = []; + for (const item of items) if (keep(item)) kept.push(item); + return kept; +} +const many: Item[] = []; +for (let i = 0; i < 40; i++) many.push(new Item(i, "i" + i)); +const evens = filterItems(many, (item) => item.id % 2 === 0); +console.log("filtered", evens.length, evens[0]!.label, evens[19]!.label); + +// Unchecked indexed reads past the end or into holes are missing values. +const source: Item[] = [a, b]; +source[4] = c; +const reads: Item[] = []; +for (let i = 0; i < 6; i++) reads.push(source[i]); +console.log("reads", describe(reads)); + +// Several arguments: all evaluate before the first append, left to right. +const multi: Item[] = [c]; +const pushed = multi.push(source[2], source[0], source[9], source[4]); +console.log("multi", pushed, describe(multi)); + +// Arguments that reassign or mutate the receiver: the receiver evaluated +// first keeps receiving the appends; the length is read after the values. +let target: Item[] = [a]; +const first = target; +const second: Item[] = []; +const ret = target.push(source[3], ((target = second), source[1])); +console.log("reassigned", ret, describe(first), describe(second), target === second); +const shrinking: Item[] = [a, b, c]; +const ret2 = shrinking.push(source[5], ((shrinking.length = 1), source[0])); +console.log("shrunk", ret2, describe(shrinking)); + +// Numbers and strings through the same path. +const nums: number[] = [1, 2]; +nums[5] = 6; +const numCopy: number[] = []; +for (const n of nums) numCopy.push(n); +console.log("nums", numCopy.length, JSON.stringify(numCopy), 3 in numCopy); +const words: string[] = ["x"]; +words[2] = "z"; +const wordCopy: string[] = []; +for (const w of words) wordCopy.push(w); +console.log("words", wordCopy.length, JSON.stringify(wordCopy), 1 in wordCopy); + +// Values pushed from a source that is dropped stay alive in the copy. +function detach(): Item[] { + const temp: Item[] = [new Item(7, "seven"), new Item(8, "eight")]; + const out: Item[] = []; + for (const item of temp) out.push(item); + temp.length = 0; + return out; +} +const detached = detach(); +console.log("detached", describe(detached), detached[1]!.id); + +// A field receiver and a field value. +class Holder { + list: Item[] = []; + pending: Item[] = [b]; +} +const holder = new Holder(); +holder.pending[2] = c; +for (let i = 0; i < 4; i++) holder.list.push(holder.pending[i]); +console.log("holder", describe(holder.list));