mirror of
https://github.com/vercel-labs/scriptc.git
synced 2026-11-06 08:23:19 +08:00
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.
This commit is contained in:
@@ -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
|
||||
)
|
||||
|
||||
@@ -49,9 +49,20 @@ function temporary(keys: number[]): number {
|
||||
}
|
||||
return total;
|
||||
}
|
||||
class Holder {
|
||||
m = new Map<number, Counter>([[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<number, Counter>();
|
||||
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 });
|
||||
}
|
||||
|
||||
@@ -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<number, Counter>([[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());
|
||||
|
||||
Reference in New Issue
Block a user