diff --git a/packages/compiler/src/frontend/lowering/lower-calls.ts b/packages/compiler/src/frontend/lowering/lower-calls.ts index 8667621c8..bfb2ea656 100644 --- a/packages/compiler/src/frontend/lowering/lower-calls.ts +++ b/packages/compiler/src/frontend/lowering/lower-calls.ts @@ -4573,8 +4573,9 @@ export function lowerCall(L: Lowerer, expr: ts.CallExpression): IrExpr { // take the ordinary typed paths, but there is no static home for an // any-elemented array). Typed receivers keep their own lowerings. const recvTs = L.typeOf(access.expression); + const arrayReceiver = L.checker.isArrayType(recvTs); const anyArray = - L.checker.isArrayType(recvTs) && + arrayReceiver && ((L.checker.getTypeArguments(recvTs as ts.TypeReference)[0]?.flags ?? 0) & (ts.TypeFlags.Any | ts.TypeFlags.Unknown)) !== 0; let recv: IrExpr; @@ -4648,21 +4649,8 @@ export function lowerCall(L: Lowerer, expr: ts.CallExpression): IrExpr { // unimplemented methods throw a LOUD not-supported Error; names the // kind's prototype lacks throw Node's "x.y is not a function"; OBJ // receivers call the own member. - if (DYN_DISPATCH_METHODS.has(access.name.text) && !call.questionDotToken && !access.questionDotToken) { - if (call.arguments.some((a) => ts.isSpreadElement(a))) { - L.unsupported("SC1090", call, "spread arguments in calls through 'unknown' values"); - } - const args = call.arguments.map((a) => L.lowerExprExpecting(a, DYN)); - return { - kind: "dynInvoke", - recv, - method: access.name.text, - calleeName: access.getText(), - args, - type: DYN, - loc: locOf(call), - }; - } + const dispatched = lowerDynDispatchMethodCall(L, call, access, recv, arrayReceiver); + if (dispatched) return dispatched; // Names NO dyn-representable prototype declares: the member can only // be an OWN property, so "read the member, call it" IS Node's // semantics for every possible dyn value — `handlers.onDone(x)` on a @@ -4774,6 +4762,42 @@ export const DYN_DISPATCH_METHODS = new Set([ "additionalHeaders", "altsvc", "origin", ]); +export function lowerDynDispatchMethodCall( + L: Lowerer, + call: ts.CallExpression, + access: ts.PropertyAccessExpression, + recv: IrExpr, + arrayReceiver: boolean, +): IrExpr | null { + const method = access.name.text; + if (!DYN_DISPATCH_METHODS.has(method) || call.questionDotToken || access.questionDotToken) return null; + if (call.arguments.some((arg) => ts.isSpreadElement(arg))) { + L.unsupported("SC1090", call, "spread arguments in calls through 'unknown' values"); + } + const predicate = method === "filter" && call.arguments[0] + ? L.lowerExpr(call.arguments[0]) + : null; + if (arrayReceiver && predicate?.type.kind === "func" && predicate.type.ret.kind === "void") { + L.unsupported( + "SC1090", + call.arguments[0]!, + "'.filter()' with a void-returning predicate (the callback return value is erased before its truthiness can be tested)", + ); + } + const args = call.arguments.map((arg, i) => + i === 0 && predicate ? L.coerceInto(arg, predicate, DYN) : L.lowerExprExpecting(arg, DYN), + ); + return { + kind: "dynInvoke", + recv, + method, + calleeName: access.getText(), + args, + type: DYN, + loc: locOf(call), + }; +} + /** STR_METHODS ∪ the regex-form names, MINUS everything Array (or any * other dyn kind's prototype) also declares. */ const DYN_STRING_ONLY_METHODS = new Set([ diff --git a/packages/compiler/src/frontend/lowering/lower-containers.ts b/packages/compiler/src/frontend/lowering/lower-containers.ts index 6fd860a94..70855f1a6 100644 --- a/packages/compiler/src/frontend/lowering/lower-containers.ts +++ b/packages/compiler/src/frontend/lowering/lower-containers.ts @@ -9,7 +9,7 @@ import { ARRAY_METHODS, MAP_METHODS, SET_COMBINE_METHODS, SET_METHODS, STR_METHO import { droppableStatic, isRequireMainFilename, lowerDynObjectLiteral, probeLower, pureReemittable } from "./lower-exprs.js"; import { forOfVarTarget, lowerDestructuringAssign } from "./lower-stmts.js"; import { isJsSourceFile, locOf } from "../program.js"; -import { DYN_DISPATCH_METHODS, islandPrimitiveExit } from "./lower-calls.js"; +import { islandPrimitiveExit, lowerDynDispatchMethodCall } from "./lower-calls.js"; import { typeKey } from "../types.js"; import { dynUndefinedExpr, own, WidthLift } from "./lowerer.js"; @@ -155,7 +155,15 @@ function lowerSplitLimitArg(L: Lowerer, node: ts.Expression | undefined, loc: Sr { const receiver = L.lowerExpr(access.expression); if (receiver.type.kind === "jsval") { - const args = call.arguments.map((a) => L.jsvalIn(L.lowerExpr(a), a)); + const loweredArgs = call.arguments.map((a) => L.lowerExpr(a)); + if (name === "filter" && loweredArgs[0]?.type.kind === "func" && loweredArgs[0].type.ret.kind === "void") { + L.unsupported( + "SC1090", + call.arguments[0]!, + "'.filter()' with a void-returning predicate (the callback return value is erased before its truthiness can be tested)", + ); + } + const args = loweredArgs.map((arg, i) => L.jsvalIn(arg, call.arguments[i]!)); const result: IrExpr = { kind: "jsOp", op: "callMethod", name, args: [receiver, ...args], type: JSVAL, loc }; return islandPrimitiveExit(L, call, result); } @@ -168,21 +176,8 @@ function lowerSplitLimitArg(L: Lowerer, node: ts.Expression | undefined, loc: Sr // ICE). Consumers validate the dyn result where a static type is // required (dynCheck — the member-read discipline). if (receiver.type.kind === "dyn") { - if (DYN_DISPATCH_METHODS.has(name) && !call.questionDotToken && !access.questionDotToken) { - if (call.arguments.some((a) => ts.isSpreadElement(a))) { - L.unsupported("SC1090", call, "spread arguments in calls through 'unknown' values"); - } - const args = call.arguments.map((a) => L.lowerExprExpecting(a, DYN)); - return { - kind: "dynInvoke", - recv: receiver, - method: name, - calleeName: access.getText(), - args, - type: DYN, - loc, - }; - } + const dispatched = lowerDynDispatchMethodCall(L, call, access, receiver, true); + if (dispatched) return dispatched; L.noLowering( `.${name} on an array value held in a checked-dynamic binding`, call, @@ -668,13 +663,68 @@ function lowerSplitLimitArg(L: Lowerer, node: ts.Expression | undefined, loc: Sr "'.map()' with a callback returning 'unknown'-typed values (the result array has no static element type — annotate the callback's return)", ); } - if (method === "filter" && fnRet.kind !== "bool") L.badType(argNode, L.typeOf(argNode)); + // JS applies ToBoolean to whatever the predicate answers, so a non-bool + // result is not an error — the filter loop wraps the call in the same + // toBool an `if` statement would apply. The island/dyn shapes, whose + // truthiness needs the engine, and void, whose real answer the ABI has + // already discarded, keep the fence. No separate + // requireTruthyUnion call belongs here: its check (no dyn/caught arm) + // IS filterPredicateOk's union branch, so it could never speak. + if (method === "filter" && !filterPredicateOk(L, fnRet)) { + if (fnRet.kind === "void") { + L.unsupported( + "SC1090", + argNode, + "'.filter()' with a void-returning predicate (the callback return value is erased before its truthiness can be tested)", + ); + } + L.badType(argNode, L.typeOf(argNode)); + } const helper = arrayHofHelper(L, method, elem, fnRet, arity, loc); const resultType: IrType = method === "map" ? arrayOf(fnRet) : method === "filter" ? arrayOf(elem) : VOID; return { kind: "call", callee: helper, args: [receiver, fnArg], type: resultType, loc }; } +/** The predicate result kinds `.filter()` accepts. JS applies ToBoolean to + * whatever the callback answers, so a bool is not required: the scalars and + * the reference kinds have constant or by-value answers, and a union is fine + * when every arm does. void/dyn/jsval/caught stay out — see below. */ +function filterPredicateOk(L: Lowerer, ret: IrType): boolean { + if (ret.kind === "bool") return true; + // VOID is a TYPE erasure, not a runtime value: TS lets a value-returning + // function sit in a void-returning slot (`const p: (n: number) => void = + // (n) => n`), so the predicate's real answer can be truthy while the + // compiled ABI has already discarded it. Treating void as constantly + // falsy would silently answer [] where Node answers [1]. Fenced until + // the returned value can be preserved through the void ABI. + // dyn/jsval/caught stay out too: no native ToBoolean to compile against. + if (ret.kind === "void") return false; + if (ret.kind === "dyn" || ret.kind === "jsval" || ret.kind === "caught") return false; + if (ret.kind === "union") { + const def = L.unions.get(ret.unionId); + return def !== undefined && def.arms.every((a) => a.kind !== "dyn" && a.kind !== "caught"); + } + return true; +} + +/** The filter loop's condition: the predicate's result put through JS + * ToBoolean. A bool answer is already the condition; everything else takes + * the same `toBool` wrapper an `if` statement would apply (a union answer + * routes through its interned per-arm truthy helper). */ +function filterCond(call: IrExpr, fnRet: IrType, loc: SrcLoc): IrExpr { + if (fnRet.kind === "bool") return call; + // No constant-false arm here: void is fenced at the call site, and a + // bare undefinedT/nullT return cannot arise (mapType sends `undefined`/ + // `void` returns to void and a standalone `null` return to the unit-ONLY + // UNION, which routes through toBool below; ir/validate.ts rejects a + // bare unit return type outright). filterPredicateOk already rejected + // the arms with no native ToBoolean (dyn/caught) — which is exactly what + // requireTruthyUnion checks — and it did so at the call site, where a + // real node exists for the diagnostic. + return { kind: "toBool", operand: call, type: BOOL, loc }; +} + /** Interned synthetic loop function for one (method, elem, fnRet, arity) * combo. Named `%arr..` ('%' keeps it out of the user * namespace); rides `liftedFns` into the module like a lifted lambda (it @@ -1006,7 +1056,9 @@ function lowerSplitLimitArg(L: Lowerer, node: ts.Expression | undefined, loc: Sr { kind: "varDecl", localId: "v.0", init: getElem, loc }, { kind: "if", - cond: callF(ref("v.0", elem)), + // ToBoolean over the predicate's answer — inert when it already + // returned bool, the per-union helper when it returned a union. + cond: filterCond(callF(ref("v.0", elem)), fnRet, loc), then: [push(arrT, ref("v.0", elem))], else_: null, loc, diff --git a/packages/compiler/test/ts7/baselines/order-parity.json b/packages/compiler/test/ts7/baselines/order-parity.json index 2fab13504..d95e96e39 100644 --- a/packages/compiler/test/ts7/baselines/order-parity.json +++ b/packages/compiler/test/ts7/baselines/order-parity.json @@ -5569,8 +5569,14 @@ "diags": [] }, "/tests/corpus/2682-fs-rename.ts": { + "order": [ + "/tests/corpus/2682-fs-rename.ts" + ], + "diags": [] + }, + "/tests/corpus/2683-filter-truthy-predicate.ts": { "order": [ - "/tests/corpus/2682-fs-rename.ts" + "/tests/corpus/2683-filter-truthy-predicate.ts" ], "diags": [] }, @@ -5580,6 +5586,12 @@ ], "diags": [] }, + "/tests/corpus/2684-filter-no-predicate-dyn.cjs": { + "order": [ + "/tests/corpus/2684-filter-no-predicate-dyn.cjs" + ], + "diags": [] + }, "/tests/corpus/2684-fs-rename-abort.ts": { "order": [ "/tests/corpus/2684-fs-rename-abort.ts" @@ -5635,9 +5647,9 @@ "diags": [] }, "/tests/corpus/2700-wasi-core.ts": { - "order": [ - "/tests/corpus/2700-wasi-core.ts" - ], + "order": [ + "/tests/corpus/2700-wasi-core.ts" + ], "diags": [] }, "/tests/corpus/300-if-else.ts": { @@ -6977,6 +6989,30 @@ ], "diags": [] }, + "/tests/diagnostics/filter-void-predicate-dyn.ts": { + "order": [ + "/tests/diagnostics/filter-void-predicate-dyn.ts" + ], + "diags": [] + }, + "/tests/diagnostics/filter-void-predicate-dynamic.ts": { + "order": [ + "/tests/diagnostics/filter-void-predicate-dynamic.ts" + ], + "diags": [] + }, + "/tests/diagnostics/filter-void-predicate-island.ts": { + "order": [ + "/tests/diagnostics/filter-void-predicate-island.ts" + ], + "diags": [] + }, + "/tests/diagnostics/filter-void-predicate.ts": { + "order": [ + "/tests/diagnostics/filter-void-predicate.ts" + ], + "diags": [] + }, "/tests/diagnostics/function-forms.ts": { "order": [ "/tests/diagnostics/function-forms.ts" diff --git a/tests/corpus/2683-filter-truthy-predicate.ts b/tests/corpus/2683-filter-truthy-predicate.ts new file mode 100644 index 000000000..71f61d96a --- /dev/null +++ b/tests/corpus/2683-filter-truthy-predicate.ts @@ -0,0 +1,31 @@ +// Array.prototype.filter applies ToBoolean to predicate results; predicates +// need not return boolean. + +const words = ["", "a", "bb", "ccc"]; +console.log(words.filter((s) => s).join(",")); +console.log(words.filter((s) => s.length).join(",")); + +const nums = [-2, -1, 0, 1, 2]; +console.log(nums.filter((n) => n).join(",")); +console.log(nums.filter((n) => (n === 0 ? "" : "yes")).join(",")); + +// A `null` result is the unit-ONLY UNION, not a bare unit — it still rides +// the per-arm truthy helper, and every arm is falsy. (A `void`-returning +// predicate is NOT accepted: TS lets a value-returning function fill a +// void slot, so its real answer is unknowable once the ABI discards it.) +let unitCalls = 0; +console.log(nums.filter(() => { + unitCalls++; + return null; +}).length, unitCalls); + +// -0 and NaN are falsy; a non-empty string and a non-zero number truthy. +const edges = [-0, 0, NaN, 1]; +console.log(edges.filter((n) => n).length); +console.log(["", "0"].filter((s) => s).join("|")); + +// A mixed-arm union result routes through the interned per-arm helper. +function pick(n: number): string | number { + return n % 2 === 0 ? "" : n; +} +console.log(nums.filter((n) => pick(n)).join(",")); diff --git a/tests/corpus/2684-filter-no-predicate-dyn.cjs b/tests/corpus/2684-filter-no-predicate-dyn.cjs new file mode 100644 index 000000000..2a91d3dc4 --- /dev/null +++ b/tests/corpus/2684-filter-no-predicate-dyn.cjs @@ -0,0 +1,14 @@ +// A checked-dynamic filter call with no predicate reaches the runtime method, +// which throws the same callback TypeError as Node instead of crashing scriptc. +const source = JSON.parse('{"zero":0,"one":1}'); +try { + Object.keys(source).filter(); +} catch (error) { + console.log(error.name, error.message); +} + +const custom = JSON.parse('{}'); +custom.filter = (callback) => callback(); +custom.filter(() => { + console.log('custom filter'); +}); diff --git a/tests/diagnostics/filter-void-predicate-dyn.ts b/tests/diagnostics/filter-void-predicate-dyn.ts new file mode 100644 index 000000000..fc2f0f34f --- /dev/null +++ b/tests/diagnostics/filter-void-predicate-dyn.ts @@ -0,0 +1,6 @@ +// @dynamic +// A direct Object.keys call over a dynamic object has a checker-array type +// but a checked-dynamic value. Its predicate still cannot use void. +const pred: (n: string) => void = (n) => n; +console.log(Object.keys(JSON.parse('{"zero":0,"one":1}')).filter(pred).join(",")); +// The callback's erased return makes this unsupported. diff --git a/tests/diagnostics/filter-void-predicate-dynamic.ts b/tests/diagnostics/filter-void-predicate-dynamic.ts new file mode 100644 index 000000000..03c5f4eb3 --- /dev/null +++ b/tests/diagnostics/filter-void-predicate-dynamic.ts @@ -0,0 +1,5 @@ +// @dynamic +// The void ABI remains a static fence even with the dynamic engine enabled. +const pred: (n: number) => void = (n) => n; +console.log([0, 1].filter(pred).join(",")); +// The callback's erased return makes this unsupported. diff --git a/tests/diagnostics/filter-void-predicate-island.ts b/tests/diagnostics/filter-void-predicate-island.ts new file mode 100644 index 000000000..68a10455e --- /dev/null +++ b/tests/diagnostics/filter-void-predicate-island.ts @@ -0,0 +1,11 @@ +// @dynamic +// An overload can present an island array as static. The callback still uses +// the void ABI, so the engine must not silently receive an erased return. +function values(): number[]; +function values(): any { + return [0, 1]; +} + +const pred: (n: number) => void = (n) => n; +console.log(values().filter(pred).join(",")); +// The callback's erased return makes this unsupported. diff --git a/tests/diagnostics/filter-void-predicate.ts b/tests/diagnostics/filter-void-predicate.ts new file mode 100644 index 000000000..24f5e55dc --- /dev/null +++ b/tests/diagnostics/filter-void-predicate.ts @@ -0,0 +1,9 @@ +// `.filter()` takes truthy (non-boolean) predicate results, but NOT a +// `void`-returning one. void is a TYPE erasure, not a runtime value: TS +// lets a value-returning function sit in a void-returning slot, so the +// predicate's real answer can be truthy while the compiled ABI has already +// discarded it — under Node `[0, 1].filter(pred)` below is `[1]`. Fenced +// until the returned value can be preserved through the void ABI. +const pred: (n: number) => void = (n) => n; +console.log([0, 1].filter(pred).join(",")); +// The callback's erased return makes this unsupported. diff --git a/tests/harness/__snapshots__/filter-void-predicate-dyn.ts.txt b/tests/harness/__snapshots__/filter-void-predicate-dyn.ts.txt new file mode 100644 index 000000000..a0c8d5699 --- /dev/null +++ b/tests/harness/__snapshots__/filter-void-predicate-dyn.ts.txt @@ -0,0 +1,6 @@ +filter-void-predicate-dyn.ts:5:66 - error SC1090: '.filter()' with a void-returning predicate (the callback return value is erased before its truthiness can be tested) is not supported yet + + 4 | const pred: (n: string) => void = (n) => n; + 5 | console.log(Object.keys(JSON.parse('{"zero":0,"one":1}')).filter(pred).join(",")); + | ^~~~ + 6 | // The callback's erased return makes this unsupported. \ No newline at end of file diff --git a/tests/harness/__snapshots__/filter-void-predicate-dynamic.ts.txt b/tests/harness/__snapshots__/filter-void-predicate-dynamic.ts.txt new file mode 100644 index 000000000..dcba56469 --- /dev/null +++ b/tests/harness/__snapshots__/filter-void-predicate-dynamic.ts.txt @@ -0,0 +1,6 @@ +filter-void-predicate-dynamic.ts:4:27 - error SC1090: '.filter()' with a void-returning predicate (the callback return value is erased before its truthiness can be tested) is not supported yet + + 3 | const pred: (n: number) => void = (n) => n; + 4 | console.log([0, 1].filter(pred).join(",")); + | ^~~~ + 5 | // The callback's erased return makes this unsupported. \ No newline at end of file diff --git a/tests/harness/__snapshots__/filter-void-predicate-island.ts.txt b/tests/harness/__snapshots__/filter-void-predicate-island.ts.txt new file mode 100644 index 000000000..e6c78021a --- /dev/null +++ b/tests/harness/__snapshots__/filter-void-predicate-island.ts.txt @@ -0,0 +1,6 @@ +filter-void-predicate-island.ts:10:29 - error SC1090: '.filter()' with a void-returning predicate (the callback return value is erased before its truthiness can be tested) is not supported yet + + 9 | const pred: (n: number) => void = (n) => n; + 10 | console.log(values().filter(pred).join(",")); + | ^~~~ + 11 | // The callback's erased return makes this unsupported. \ No newline at end of file diff --git a/tests/harness/__snapshots__/filter-void-predicate.ts.txt b/tests/harness/__snapshots__/filter-void-predicate.ts.txt new file mode 100644 index 000000000..e928a41e0 --- /dev/null +++ b/tests/harness/__snapshots__/filter-void-predicate.ts.txt @@ -0,0 +1,6 @@ +filter-void-predicate.ts:8:27 - error SC1090: '.filter()' with a void-returning predicate (the callback return value is erased before its truthiness can be tested) is not supported yet + + 7 | const pred: (n: number) => void = (n) => n; + 8 | console.log([0, 1].filter(pred).join(",")); + | ^~~~ + 9 | // The callback's erased return makes this unsupported. \ No newline at end of file