fix(analyser): widen Binding::Dynamic result to Any 🪟#140
Conversation
Adds a failing functional test (`bug0021`) that exercises the regression from #139: vectorized tuple arithmetic crashes when the result of one tuple op is consumed by another, because the analyser widens the dynamic result type to `Number` and the second op resolves to a numeric overload that bypasses the VM's vectorized fallback. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When dynamic dispatch sees a binary call whose arguments satisfy `supports_vectorization_with`, infer the result as `Tuple<Number, …>` matching the tuple shape instead of taking the LUB of the candidates' declared return types. Before this, `let diff = a - b` over tuples inferred `diff: Number` (the LUB of the numeric overloads), so a follow-up `diff * diff` exact-matched the `(Number, Number) -> Number` overload, emitted a direct `Call` instead of `OverloadSet` dispatch, and bypassed the VM's vectorized fallback — failing at runtime with "expected number, got Tuple<Int, Int, Int>". Mirroring the VM's vectorization rule in the analyser keeps chained tuple arithmetic on the dynamic path. Fixes #139. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f964d0f9e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Extend the vectorization shape detection so it also fires when tuple elements are `Any`. Stdlib natives that return `Value` (e.g. `first`, `last`) infer to `Any`, so a tuple built from their results gets type `Tuple<Any, ...>`. Without this, `let c = a - b` over such a tuple would still take the LUB path and infer `Number`, putting the follow- up `c * c` back on the direct-call path that bypasses vectorization. Extends bug0021 with the `(l.first, l.first) - (l.last, l.last)` shape that reproduced this gap. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 812b14f5f6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
LUB-of-declared-returns assumed one of the candidates would fire at runtime; that's unsound for runtime-dispatched calls (the value-level dispatcher can fall through to elementwise / vectorized dispatch and produce a value no overload declares). Treat Binding::Dynamic results as Any so downstream callers stay on dynamic dispatch instead of exact-matching a numeric overload that the value doesn't fit. Drops the special-cased vectorization detection — the broader rule covers every cascade depth of the issue #139 repro and any other runtime-dispatch surprise without coupling the analyser to a specific VM fallback path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
timfennis
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d00dee5ec3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
1 participant
Summary
Fixes #139. Regression from #131 (the type-annotation work): vectorized tuple arithmetic crashed at runtime whenever the result of one tuple op was fed into another, e.g.
let diff = a - b; diff * diffor(a - b) * (a - b).Root cause
In
resolve_function_with_argument_types, theBinding::Dynamicarm inferred the call's result as the LUB of every candidate's declared return type. That LUB is sound only when one of those candidates is statically guaranteed to fire. With dynamic dispatch the value-level dispatcher can fall through to elementwise / vectorized handling at runtime and produce a value no declared overload returns. Solet diff = a - bover tuples was inferred asdiff: Number(the LUB of the numeric overloads), and the follow-updiff * diffthen exact-matched(Number, Number) -> Number, emitted a directCallinstead ofOverloadSetdispatch, and bypassed dynamic dispatch entirely — surfacing asexpected number, got Tuple<Int, Int, Int>at runtime.The same hole shows up whenever a
Tupleflows through dynamic dispatch — e.g.(id(1), id(2)) - (id(3), id(4))whereidreturnsAny— because the analyser still gets back a confident-but-wrongNumber.Fix
Widen the
Binding::Dynamicarm's result toStaticType::Any. Runtime-dispatched calls don't have a sound static bound on their result, so the analyser stops pretending otherwise. Every cascade rung now stays on dynamic dispatch, and the VM's existing value-level dispatcher decides at runtime.Changes
ndc_analyser/src/analyser.rs—Binding::Dynamicarm returnsAnyinstead of LUB-of-candidate-returns. No more special-cased vectorization detection.tests/functional/programs/900_bugs/bug0021_chained_vectorized_tuple_arith.ndc— regression test covering the original repro from Vectorized tuple arithmetic breaks when result is consumed by another arithmetic op #139 plus aTuple<Any, …>variant (fn id(x) -> Any => xto keep the type-erasure independent of stdlib evolution).Performance
Acceptable regression — within noise on this machine.
hyperfine --warmup 3 --runs 30:A smaller 5-run sweep over
fibonacci,quicksort,hof_pipeline,ackermann,pi_approxall landed at 1.00×–1.06× in either direction with σ bigger than the delta.Trade-off worth flagging
Binding::Dynamicsites lose their LUB-derived inlay hints — e.g. a user-defined function with(Int) -> Intand(Float) -> Floatoverloads called via dynamic dispatch used to surfaceNumberon the LHS of alet, and will now surfaceAny(filtered from inlay hints). The LUB was always a heuristic that happened to look right; the soundness fix removes it. Happy to revisit if the LSP UX hit feels too steep.🤖 PR description by Claude.