From 342b770cf20a8b1ae124c5053b174ab18281177f Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Sun, 25 Oct 2026 03:25:37 -0500 Subject: [PATCH] Keep a Map.get result owned when its key can replace the map's holder The receiver of a borrowed Map.get is read before its key. When the key's evaluation replaces the field or local that holds the map (`this.m.get(this.key())` where key() assigns this.m), the read runs on the old map, which only the statement still holds, and the borrowed value outlives it. A borrow now also requires the read itself, receiver and key included, to preserve reference edges; otherwise the read keeps its own reference. Corpus 8181 and the emission test cover a key method that replaces the field. --- .../src/backend/llvm/map-read-lifetimes.ts | 7 +++++-- .../test/map-get-borrow-emission.test.ts | 17 ++++++++++++++++- tests/corpus/8181-map-get-borrows.ts | 19 ++++++++++++++++++- 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/packages/compiler/src/backend/llvm/map-read-lifetimes.ts b/packages/compiler/src/backend/llvm/map-read-lifetimes.ts index 3371a7f70..d5d286674 100644 --- a/packages/compiler/src/backend/llvm/map-read-lifetimes.ts +++ b/packages/compiler/src/backend/llvm/map-read-lifetimes.ts @@ -186,8 +186,10 @@ function heldReceiver(expr: IrExpr): boolean { * branches and joins them; any other statement that names the local is * judged whole. A later pass of an enclosing loop declares the local again, * so only the rest of its own statement list matters. The map must be held - * by a local or a field (heldReceiver), and locals declared from the local - * count as its uses. An update such as + * by a local or a field (heldReceiver), the read itself must preserve + * reference edges (a key whose evaluation replaces the receiver's field + * leaves the read on a map the statement releases), and locals declared + * from the local count as its uses. An update such as * `const c = m.get(k); if (c === undefined) m.set(k, v); else c.n++` keeps * the borrow: the write runs only on the branch that does not read `c`. */ export function findMapReadBorrows( @@ -264,6 +266,7 @@ export function findMapReadBorrows( stmt.kind === "varDecl" && stmt.init?.kind === "mapIntrinsic" && heldReceiver(stmt.init.receiver) && + effects.preserves(stmt.init) && candidate(stmt) && walk(list.slice(index + 1), new Set([stmt.localId]), false) !== null ) diff --git a/packages/compiler/test/map-get-borrow-emission.test.ts b/packages/compiler/test/map-get-borrow-emission.test.ts index eba430453..4e3c91766 100644 --- a/packages/compiler/test/map-get-borrow-emission.test.ts +++ b/packages/compiler/test/map-get-borrow-emission.test.ts @@ -49,9 +49,20 @@ function temporary(keys: number[]): number { } return total; } +class Holder { + m = new Map([[1, new Counter()]]); + key(): number { + this.m = new Map(); + return 1; + } + read(): number { + const held = this.m.get(this.key()); + return held === undefined ? 0 : held.num; + } +} const numbers = new Map(); count(numbers, [1, 2, 1]); -console.log(named(new Map([["a", new Counter()]]), "a"), replaced(numbers), aliased(numbers), temporary([1, 2])); +console.log(named(new Map([["a", new Counter()]]), "a"), replaced(numbers), aliased(numbers), temporary([1, 2]), new Holder().read()); `, ); const result = await compile(entry, { outDir: dir, outPath, outputKind: "ir" }); @@ -72,6 +83,10 @@ console.log(named(new Map([["a", new Counter()]]), "a"), replaced(numbers), alia // does a read from a map that is released after the declaration. for (const name of ["replaced", "aliased", "temporary"]) expect(fn(name)).toContain("@sc_retain_Counter"); + // A key whose evaluation replaces the receiver's field leaves the read + // on the old map, which the statement releases: the read keeps its own + // reference. + expect(fn("_x25_Holder_read")).toContain("@sc_retain_Counter"); } finally { await rm(dir, { recursive: true, force: true }); } diff --git a/tests/corpus/8181-map-get-borrows.ts b/tests/corpus/8181-map-get-borrows.ts index 5df7ad226..1a6c71667 100644 --- a/tests/corpus/8181-map-get-borrows.ts +++ b/tests/corpus/8181-map-get-borrows.ts @@ -2,7 +2,8 @@ // release the entry's value: an update on the absent branch, a write, delete // or clear before a later read, a call that may change the map, loops, // string keys, values that are stored, returned or compared, a local declared -// from the read, and a read from a temporary map. +// from the read, a read from a temporary map, and a key that replaces the +// field holding the map. class Counter { num: number; label: string; @@ -153,3 +154,19 @@ function fromTemporary(): string { return held === undefined ? "none" : `${held.num}:${held.label}:${filler.length}`; } console.log("temporary", fromTemporary()); + +// A key whose evaluation replaces the field that holds the map: the read +// uses the old map, which only the statement still holds. +class Holder { + m = new Map([[1, new Counter(9, "nine")]]); + key(): number { + this.m = new Map(); + return 1; + } + read(): string { + const held = this.m.get(this.key()); + const filler = [new Counter(10, "filler"), new Counter(11, "filler")]; + return held === undefined ? "none" : `${held.num}:${held.label}:${filler.length}:${this.m.size}`; + } +} +console.log("replaced by key", new Holder().read());