From 775a317edda511db525a18630b2ef3de6e8f97ed Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Mon, 26 Oct 2026 23:20:59 -0500 Subject: [PATCH] Keep element reads through `xs[xs.length - 1]!` borrowed A member read through a non-null asserted element read evaluates the element's presence and then reads it, so a missing element throws Node's TypeError. An index other than a local or a field read was held in a hidden local first, which turned the receiver into a sequence: the element was then read with a reference (retain and release) instead of borrowed, and the fallback `genes[genes.length - 1]!.sc` of a search made fasta's lookup loop 12% slower. An index computed only from locals, field reads, number literals and the lengths of arrays read through them (`xs.length - 1`, `i + 1`, `-i`) has no effect and yields the same value when evaluated again with nothing in between, so it is evaluated for the presence test and again for the read, and the element stays a borrowed read. Other indexes keep their hidden local. --- .../src/frontend/lowering/lower-exprs.ts | 26 +++++- .../non-null-element-computed-index.ts.json | 6 ++ .../corpus/non-null-element-computed-index.ts | 84 +++++++++++++++++++ .../non-null-element-computed-index.ts.json | 1 + 4 files changed, 116 insertions(+), 1 deletion(-) create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/non-null-element-computed-index.ts.json create mode 100644 tests/corpus/non-null-element-computed-index.ts create mode 100644 tests/harness/corpus-costs/non-null-element-computed-index.ts.json diff --git a/packages/compiler/src/frontend/lowering/lower-exprs.ts b/packages/compiler/src/frontend/lowering/lower-exprs.ts index 9eacfa2b2..89ea52bc5 100644 --- a/packages/compiler/src/frontend/lowering/lower-exprs.ts +++ b/packages/compiler/src/frontend/lowering/lower-exprs.ts @@ -1227,7 +1227,7 @@ export function dereferencedElementParts( return varRef(tmp.id, value.type, loc); }; const a = !mayAssign(index) && plain(arr) ? arr : held(arr, "%elementArray"); - const i = plain(index) ? index : held(index, "%elementIndex"); + const i = plain(index) || repeatableIndex(index) ? index : held(index, "%elementIndex"); return { stmts, present: { @@ -1454,6 +1454,30 @@ function plainProjection(e: IrExpr): boolean { return (e.kind === "fieldGet" || e.kind === "recordGet") && plainProjection(e.obj); } +/** A number computed from plain reads, number literals and the lengths of + * arrays read through them (`xs.length - 1`): evaluating it again, with + * nothing in between, has no effect and yields the same value. An element + * read through `xs[xs.length - 1]!` then evaluates its index twice instead + * of holding it in a hidden local, and stays a borrowed element read. */ +function repeatableIndex(e: IrExpr): boolean { + if (e.type.kind !== "f64") return false; + switch (e.kind) { + case "numLit": + case "varRef": + case "fieldGet": + case "recordGet": + return plainProjection(e); + case "bin": + return repeatableIndex(e.left) && repeatableIndex(e.right); + case "unary": + return repeatableIndex(e.operand); + case "arrIntrinsic": + return e.method === "length" && e.args.length === 0 && plainProjection(e.receiver); + default: + return false; + } +} + /** 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 diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/non-null-element-computed-index.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/non-null-element-computed-index.ts.json new file mode 100644 index 000000000..3030323af --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/non-null-element-computed-index.ts.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/corpus/non-null-element-computed-index.ts" + ], + "diags": [] +} diff --git a/tests/corpus/non-null-element-computed-index.ts b/tests/corpus/non-null-element-computed-index.ts new file mode 100644 index 000000000..4afa873f0 --- /dev/null +++ b/tests/corpus/non-null-element-computed-index.ts @@ -0,0 +1,84 @@ +// Member reads through non-null asserted element reads at computed indexes: +// `xs[xs.length - 1]!`, `xs[i + 1]!`, `xs[-i]!` and indexes read from +// fields. A missing element (past the end, before the start, a fraction, +// NaN, or an empty array) is undefined, so the member access throws Node's +// TypeError; a present one reads the member. + +interface Gene { + c: number; + sc: number; +} + +class Cell { + label: string; + constructor(label: string) { + this.label = label; + } + shout(): string { + return this.label.toUpperCase(); + } +} + +function attempt(label: string, f: () => unknown): void { + try { + console.log(label.padEnd(16), "ok", f()); + } catch (e) { + const error = e as Error; + console.log(label.padEnd(16), error instanceof TypeError, error.message); + } +} + +// The fallback of a cumulative search, as fasta writes it. +function pick(genes: Gene[], rand: number): number { + for (const gene of genes) { + if (rand < gene.c) return gene.sc; + } + return genes[genes.length - 1]!.sc; +} + +const genes: Gene[] = [ + { c: 0.25, sc: 97 }, + { c: 0.5, sc: 99 }, + { c: 0.75, sc: 103 }, +]; +const codes: number[] = []; +for (let k = 0; k < 8; k++) codes.push(pick(genes, k / 8)); +console.log("codes", codes.join(",")); +attempt("fallback", () => pick(genes, 2)); +attempt("empty fallback", () => pick([], 0.5)); + +const cells = [new Cell("a"), new Cell("b"), new Cell("c")]; +const none: Cell[] = []; +const holey: Cell[] = []; +holey[3] = new Cell("d"); +const box = { at: 1, size: 3 }; + +for (const i of [0, 1, 2, 3, -1, 0.5, NaN]) { + attempt(`last - ${i}`, () => cells[cells.length - 1 - i]!.label); + attempt(`next ${i}`, () => cells[i + 1]!.label); + attempt(`neg ${i}`, () => cells[-i]!.label); + attempt(`double ${i}`, () => cells[i * 2]!.shout()); +} +attempt("empty last", () => none[none.length - 1]!.label); +attempt("empty method", () => none[none.length - 1]!.shout()); +attempt("hole last", () => holey[holey.length - 1]!.label); +attempt("hole before", () => holey[holey.length - 2]!.label); +attempt("field index", () => cells[box.at - 1]!.label); +attempt("field past", () => cells[box.size + box.at]!.label); +attempt("other length", () => cells[holey.length - 2]!.label); + +const rows = [ + [1, 2], + [3, 4], +]; +attempt("row last", () => rows[rows.length - 1]![0]); +attempt("row past", () => rows[rows.length]![1]); +attempt("row length", () => rows[rows.length - 2]!.length); + +// The same read in a loop that runs past the end. +let total = 0; +try { + for (let i = 0; i < 5; i++) total += cells[cells.length - 1 - i]!.label.length; +} catch (e) { + console.log("loop", (e as Error).message, total); +} diff --git a/tests/harness/corpus-costs/non-null-element-computed-index.ts.json b/tests/harness/corpus-costs/non-null-element-computed-index.ts.json new file mode 100644 index 000000000..cd39a7ab4 --- /dev/null +++ b/tests/harness/corpus-costs/non-null-element-computed-index.ts.json @@ -0,0 +1 @@ +{ "plain": 3900, "san": 6300 }