mirror of
https://github.com/vercel-labs/scriptc.git
synced 2026-11-04 08:04:37 +08:00
Check the elements a new Set takes from InlineArray field reads
`new Set([a, b])`, where `a` and `b` are reads of the same InlineArray field, gave a Set of size 2 where Node gives size 1: each read may give another array, and the source proof checked Set elements only when they were added with `add`. It now reports SC7009 when the seed of a new Set, or the values of a Map that become a Set (`new Set(map.values())`), may hold a field's array.
This commit is contained in:
@@ -175,6 +175,11 @@ const MODELLED: ReadonlySet<IrExpr["kind"]> = new Set<IrExpr["kind"]>([
|
||||
* identity. */
|
||||
const IDENTITY_SEARCHES: ReadonlySet<string> = new Set(["indexOf", "includes"]);
|
||||
|
||||
/** Each read of an inline field may give another array, where Node.js
|
||||
* gives the field's one array every time. */
|
||||
const IDENTITY_REASON =
|
||||
"an array that may be an InlineArray field's is compared by identity, and each read of the field may give another array (compare a copy made with slice(), or compare the elements)";
|
||||
|
||||
/** Map and Set methods whose first argument is a key (or a Set element). */
|
||||
const KEYED: ReadonlySet<string> = new Set(["get", "set", "has", "delete", "add"]);
|
||||
|
||||
@@ -1512,8 +1517,7 @@ class SourceAnalysis {
|
||||
reason: `an array that may be an InlineArray field's reaches ${what}, where writes are not checked`,
|
||||
});
|
||||
};
|
||||
// Each read of an inline field may give another array, where Node.js
|
||||
// gives the field's one array every time.
|
||||
// Comparisons by identity of an array a field read may give.
|
||||
const identity = (value: IrExpr, loc: SrcLoc): void => {
|
||||
if (check === null) return;
|
||||
const origin = this.origin(fn, value);
|
||||
@@ -1521,10 +1525,28 @@ class SourceAnalysis {
|
||||
check({
|
||||
loc,
|
||||
origin,
|
||||
reason:
|
||||
"an array that may be an InlineArray field's is compared by identity, and each read of the field may give another array (compare a copy made with slice(), or compare the elements)",
|
||||
reason: IDENTITY_REASON,
|
||||
});
|
||||
};
|
||||
// The same for the elements of an array or Set, or the values of a Map,
|
||||
// that become a new Set's elements.
|
||||
const elementIdentity = (container: IrExpr, loc: SrcLoc): void => {
|
||||
if (check === null) return;
|
||||
const t = container.type;
|
||||
const elem =
|
||||
t.kind === "array" || t.kind === "set" ? t.elem : t.kind === "map" ? t.value : null;
|
||||
if (elem === null || !this.holdsFieldArray(elem)) return;
|
||||
for (const slot of this.slotsOf(fn, container)) {
|
||||
const origin = this.pinned.get(`c:${slot}`);
|
||||
if (origin === undefined) continue;
|
||||
check({
|
||||
loc,
|
||||
origin,
|
||||
reason: IDENTITY_REASON,
|
||||
});
|
||||
return;
|
||||
}
|
||||
};
|
||||
const args = (call: IrExpr & { args: IrExpr[] }): void => {
|
||||
const callees = this.callees(call);
|
||||
for (const { callee, offset } of callees) {
|
||||
@@ -1575,8 +1597,10 @@ class SourceAnalysis {
|
||||
}
|
||||
case "mapIntrinsic":
|
||||
case "setIntrinsic":
|
||||
// Keys and Set elements are compared by identity.
|
||||
// Keys and Set elements are compared by identity, and so are a
|
||||
// Map's values when they become a Set's elements.
|
||||
if (e.args[0] !== undefined && KEYED.has(e.method)) identity(e.args[0], e.loc);
|
||||
if (e.method === "valueSet") elementIdentity(e.receiver, e.loc);
|
||||
if (e.method === "set" || e.method === "add")
|
||||
for (const a of e.args) store(e.receiver, a);
|
||||
else if (e.method === "clone" || e.method === "keySet" || e.method === "valueSet")
|
||||
@@ -1589,7 +1613,11 @@ class SourceAnalysis {
|
||||
});
|
||||
break;
|
||||
case "setNew":
|
||||
if (e.seed) spread(e.seed, this.site(fn, e));
|
||||
if (e.seed) {
|
||||
// The seed's elements become Set elements.
|
||||
elementIdentity(e.seed, e.loc);
|
||||
spread(e.seed, this.site(fn, e));
|
||||
}
|
||||
break;
|
||||
case "mapNew":
|
||||
for (const { key, value } of e.seed ?? []) {
|
||||
|
||||
@@ -519,13 +519,21 @@ ${use}
|
||||
"console.log([a].indexOf(b));",
|
||||
"console.log(new Set([a]).has(b));",
|
||||
"const seen = new Map<readonly Item[], number>();\nseen.set(a, 1);\nconsole.log(seen.get(b));",
|
||||
// Set elements from a seed.
|
||||
"console.log(new Set([a, b]).size);",
|
||||
"const all: (readonly Item[])[] = [];\nall.push(a);\nall.push(b);\nconsole.log(new Set(all).size);",
|
||||
"const entries: [readonly Item[], number][] = [[a, 1], [b, 2]];\nconsole.log(new Map(entries).size);",
|
||||
"const byId = new Map<number, readonly Item[]>([[1, a], [2, b]]);\nconsole.log(new Set(byId.values()).size);",
|
||||
]) {
|
||||
const result = await build(program(use));
|
||||
expect(result.ok, use).toBe(false);
|
||||
expect(result.messages.join("\n"), use).toContain("is compared by identity");
|
||||
}
|
||||
// Reading the elements and the length is not a comparison.
|
||||
const reads = await build(program("console.log(a.length === b.length, a[0] === b[0]);"));
|
||||
// Reading the elements and the length, or a Set of the elements, compares
|
||||
// no field arrays.
|
||||
const reads = await build(
|
||||
program("console.log(a.length === b.length, a[0] === b[0], new Set(a).size);"),
|
||||
);
|
||||
expect(reads.ok).toBe(true);
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user