feat(compiler): accept truthy filter predicates - #153
Conversation
`.filter(fn)` required the callback to return exactly bool. JS applies ToBoolean to whatever the predicate answers, so `xs.filter((s) => s)` — the idiomatic drop-the-falsy — was refused for no semantic reason. The filter loop now wraps the call in the same toBool an `if` statement would apply. A bool answer is unchanged and emits identical IR. A union answer routes through its interned per-arm truthy helper, requested at the call site where a real node exists for the diagnostic. A unit-only answer is constantly falsy, and the call still runs for its effects. The kinds with no native ToBoolean keep the fence: void has no value at all, and dyn/jsval/caught truthiness needs the embedded engine. Verified against Node: filtering strings, numbers, and a `(string | undefined)[]` all print identically in both. Diagnostics 100, filter corpus 9 across both backends.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
| L.unsupported("SC1090", call, "spread arguments in calls through 'unknown' values"); | ||
| } | ||
| const args = call.arguments.map((a) => L.lowerExprExpecting(a, DYN)); | ||
| const predicate = name === "filter" ? L.lowerExpr(call.arguments[0]!) : null; |
There was a problem hiding this comment.
.filter() with zero arguments on a checked-dynamic array crashes the compiler (uncaught ICE) instead of emitting a diagnostic, because L.lowerExpr(call.arguments[0]!) is called with undefined.
There was a problem hiding this comment.
Having some agents make sure we thoroughly fix this.
There was a problem hiding this comment.
On it — I've kicked off a thorough re-verification of the fix. It's checking three things:
- That the one-line guard actually removes the ICE for zero-arg
.filter()on a checked-dynamic array in a.jsfile, and that the resulting empty-argdynInvokesurfaces the arity error at runtime (matching otherDYN_DISPATCH_METHODS) rather than silently producing wrong output. - Sibling paths from this PR: the jsval/island branch (
loweredArgs[0]?.type.kind), the staticlowerArrayHofCallguard, and non-function / optional-chained (?.filter()) first-argument cases. - Whether any additional guard beyond the single-line dyn-path patch is needed to make this comprehensive.
I'll follow up here with the results.
There was a problem hiding this comment.
Thorough re-verification is done — the single-line guard is sufficient and comprehensive. Findings:
The fix works. Guarding on call.arguments[0] makes zero-arg filter() yield predicate === null, so the call rides dynInvoke with an empty arg list and the dyn engine surfaces the arity error at runtime — same as every other DYN_DISPATCH_METHODS entry. No silent wrong output. The truthiness check also narrows call.arguments[0] to ts.Expression, so dropping the ! still type-checks.
Sibling paths are all safe — no additional guards needed:
- jsval/island path (~L157): uses
loweredArgs[0]?.type.kindand maps overcall.arguments, so zero args is a no-op — no crash. - static path
lowerArrayHofCall(L647): already guardsif (!argNode) L.unsupported("SC1090", call, "this call form"). - optional-chained
?.filter(): excluded by the existing!call.questionDotToken && !access.questionDotTokencondition, so it never reaches this branch. - non-function first arg: only skips the void-predicate diagnostic, then
coerceIntohandles it — no crash.
One caveat: the sub-agent couldn't run tsc in its sandbox (deps not installed), but the narrowing is standard TS control-flow and the change is minimal. The patch applies cleanly. Confident this is the complete fix.
2 participants
Summary
Array.prototype.filterpredicate results and lower them throughToBooleanvoidpredicates, including static, island-backed, and checked-dynamic arraysValidation
pnpm exec vitest run tests/harness/diagnostics.test.tspnpm exec vitest run tests/harness/differential.test.ts --testNamePattern "2683-filter-truthy-predicate"pnpm exec vitest run packages/compiler/test/ts7/order-parity.test.ts