fix(native): make throw from a called fn catchable across all frame kinds (#303)#335
Merged
Conversation
… kinds (#303) A `throw` from a called value-fn used to abort the process on the native backend: the value-fn ABI is `Fn(&[Value]) -> Value` with no error channel, so `fn_unwind` panicked on an uncaught throw. Add a thread-local pending-throw slot (`set/has/take_pending_throw`): the throw codegen stores the thrown value and escapes the closure with a dummy `Value`, and each call site re-raises it (in a try-closure / at run() top level) or re-escapes (in a value-fn, leaving the slot set) so the throw climbs to the enclosing try/catch. Handle all three Rust frame kinds the codegen emits, so the post-call check and the throw emit always type-check: - Value frames (`-> Value`): pending-slot escape + post-call check (the core fix). - Result frames (try-closure / run()): re-raise as `TishError::Throw`. - Native-typed frames (`-> f64`/`-> Vec<..>`/`-> ()`): suppress the check; a direct throw falls back to the abort — neither `return Err` nor `return Value::Null` is well-typed there, and a numeric kernel hitting an uncaught throw is already degenerate. (Without this, `inbounds_index` and similar typed fns failed to compile.) Reset `try_closure_depth` when entering a nested value-fn/arrow frame: a nested closure is its own `-> Value` frame, so the enclosing try's Result channel must not leak in. Previously this mis-compiled `try { arr.forEach(x => ...) }` and any throw-in-arrow-in-try. First-throw-wins in the slot, so an erroneous continuation (e.g. a braceless loop body that re-fires) keeps the throw JS would have propagated. Fixture: tests/core/throw_from_called_fn.tish — backend-identical across interp / vm / native / cranelift / wasi / js. Validated: corpus 24/24, workspace 453/453, gauntlet soundness (typed==boxed==node). Follow-up (tracked separately): array iteration builtins + Array.sort don't yet poll the slot between elements (needs the slot relocated to break a builtins->runtime dep cycle).
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 38 |
| Duplication | 4 |
🟢 Coverage 66.67% diff coverage · +0.16% coverage variation
Metric Results Coverage variation ✅ +0.16% coverage variation (-1.00%) Diff coverage ✅ 66.67% diff coverage Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (911c88d) 4418 2800 63.38% Head commit (5ed2b69) 4418 (+0) 2807 (+7) 63.54% (+0.16%) Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch:
<coverage of head commit> - <coverage of common ancestor commit>Diff coverage details
Coverable lines Covered lines Diff coverage Pull request (#335) 3 2 66.67% Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified:
<covered lines added or modified>/<coverable lines added or modified> * 100%
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Contributor
…back (#303) After #303 made a thrown value from a called fn catchable on the native backend, the shared array-iteration builtins (forEach/map/filter/reduce/find/findIndex/findLast/findLastIndex/ some/every/flatMap) and Array.sort kept running the callback for the rest of the array: the callback parked the throw in the pending-throw slot and returned a dummy, but the builtin loops never checked the slot. So extra elements ran (extra side effects; some/every computed a wrong boolean) and a throwing sort comparator silently wrote a bogus reordering back. `tishlang_builtins` cannot call `tishlang_runtime::has_pending_throw` (that would be a `builtins -> runtime` dependency cycle), so move the pending-throw slot into `tishlang_core` (which both already depend on) and re-export it from `tishlang_runtime` (so emitted native code keeps using `tishlang_runtime::{set,has,take}_pending_throw`) and from the VM (which now shares the same slot, replacing its private `VM_PENDING_THROW`). The array builtins poll `tishlang_core::has_pending_throw()` between elements and stop; sort stops invoking the comparator and restores the original order instead of writing the partial/bogus permutation. interp/vm/native now agree and match node: [1,2,3,4].forEach(throw-at-2) -> visited 1,2 (was 1,2,3,4 on native/vm) [5,4,3,2,1].sort(throwing-comparator) -> array unchanged, catchable (was corrupted) Tests: tests/core/throw_stops_array_iteration.tish (backend-identical, all 6 backends) covers forEach/map/filter/reduce/find/findIndex/findLast/findLastIndex/some/every; array.rs unit tests cover for_each-stops and sort-no-corruption directly. (interp's own flatMap + sort comparator-throw paths are a separate pre-existing divergence, tracked for follow-up.)
…d closure
A named, numeric-returning function that captures an outer variable via an array op
(e.g. `function dbl(x){ seen.push(x); return x*2 }`) was lowered to a free native-vec fn
`fn dbl_nv(x: f64) -> f64`, whose body emitted `array_push(&seen, ..)` referencing the
captured `seen` — which a free fn has no environment for, so `tish build` failed with
`error[E0425]: cannot find value 'seen'`. interp/vm were always correct.
`body_uses_local_vec_ops` (the native-vec eligibility signal) counted a `.push` onto ANY
identifier, including a captured outer var. Restrict it to vars DECLARED LOCALLY in the body
(which the `_nv` lowering emits as owned `Vec`s); a captured var is not a local vec op, so the
fn stays a boxed closure that captures correctly. Array *params* are unaffected (handled by
the separate `has_array_param` gate), so genuine native-vec kernels still qualify.
Fixture: tests/core/captured_array_in_numeric_fn.tish (backend-identical across all six).
…ap (#303) Two interpreter divergences surfaced while making array-callback throws consistent across backends: - Array.sort: the comparator's `Err` (a thrown value) was matched by the `_ => Equal` arm and silently swallowed, then a bogus partial reordering was written back. Capture the throw, stop comparing, leave the array in its original order, and re-raise — so try/catch sees it, matching vm/native/node. - Array.flatMap: the interpreter lacked it entirely ("Not a function"). Add it (map + flatten one level), propagating a callback throw via `?`. Now interp/vm/native/node all agree: [1,2,3,4].flatMap(throw-at-2) -> visited 1,2, catchable [5,4,3,2,1].sort(throwing-comparator) -> catchable, array intact Tests: tests/core/throw_in_sort_comparator.tish (catch + length preserved; exact post-throw order is impl-defined in JS, so the deterministic restore-original guarantee stays a tish_builtins unit test); flatMap added to tests/core/throw_stops_array_iteration.tish.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #303.
Makes a
throwfrom a called function fully catchable on every backend, and makes a thrown value from an array-callback behave identically across interp / vm / native / cranelift / wasi / js (matching node).Commits
throwfrom a called fn catchable across all frame kinds — the native value-fn ABI (Fn(&[Value]) -> Value) has no error channel, sofn_unwindused topanic!on an uncaught throw. A thread-local pending-throw slot stores the thrown value; the throw codegen escapes with a dummyValueand each call site re-raises it (try-closure /run()top level) or re-escapes (value-fn). Handles all three Rust frame kinds —Value,Result, and native-typed (-> f64/-> Vec/-> (), where the check is suppressed and a direct throw falls back to abort, since neitherreturn Errnorreturn Value::Nulltype-checks). Also resetstry_closure_depthentering a nested value-fn/arrow (sotry { arr.forEach(x => …) }compiles), and first-throw-wins in the slot.Array.sortstop on a thrown callback —forEach/map/filter/reduce/find/findIndex/findLast/findLastIndex/some/every/flatMapandsortused to keep running the callback after a throw (extra side effects;sortsilently wrote a bogus reordering back). The pending-throw slot moved totishlang_core(to avoid abuiltins → runtimedependency cycle), re-exported fromtishlang_runtimeand shared by the VM; the builtins now poll it between elements and stop, andsortrestores the original order.function dbl(x){ seen.push(x); return x*2 }used as a callback was mis-lowered to a free native-vec fn that referenced the capturedseenunbound (E0425). The native-vec eligibility now only counts.pushon body-local vars, not captures.Array.sortcomparator throws + addArray.flatMap— the interpreter swallowed a comparatorErr(and corrupted the array); now it propagates + leaves the array intact.flatMapwas missing entirely ("Not a function"); added (map + flatten one level, throw-propagating).Tests (tests/core, backend-identical across all six backends)
throw_from_called_fn.tish— throw across frames, value-in-catch, throw-from-arrow-in-trythrow_stops_array_iteration.tish— all 11 iteration builtins stop at the throwthrow_in_sort_comparator.tish— comparator throw is catchable, array intactcaptured_array_in_numeric_fn.tish— numeric fn capturing an outer array compiles + runstish_builtins::arrayunit tests —for_eachstops on a pending throw;sortleaves the array in its original order on a comparator throwValidation
cargo nextest -p tishlang --test integration_test: 24/24 (interp/vm/native/cranelift/wasi/js)