ReviewOS

also looking at this

home-lang/home

fix(interpreter): keep the return value across deferred user calls

#929
Open mkantautas wants to merge fix/interpreter-defer-return-value into main
2 files +203 -84
8 days ago

Problem

A defer that calls a user function erases the value of a pending return:

fn cleanup() {
}

fn direct(): int {
  defer cleanup()
  return 7
}

fn main() {
  print(direct())
}

home run aborts with panic: attempt to use null value in callUserFunctionWithNamed instead of printing 7. The same happens when the deferred call sits in a method, an if/else branch, a while or for body or a bare block; in a closure the call returns void instead of the value. A deferred builtin such as defer print("bye") is not affected.

return stores its value in Interpreter.return_value and unwinds with error.Return. Defers run during that unwinding, and a user function call clears return_value on entry, so the value is gone when the function reads self.return_value.?.

Fix

The fourteen copies of the defer loop become one runDefers helper, which each scope calls with the error it is exiting with. The helper puts the pending value back only when its scope exits with error.Return and the deferred expressions left return_value empty; otherwise it leaves the slot as the defers left it, so a deferred block that runs its own return still sets the returned value. Defers still run in reverse order and their errors are still ignored.

Tests

New tests in packages/interpreter/tests/program_eval_test.zig:

  • eval: deferred user function calls keep the pending return value (function, method, if, else, while, for over a range and an array, bare block, closure), eval: a value left behind by a call inside a defer does not replace the pending return and eval: deferred calls run in reverse order before the return value is used crash the test runner on main with panic: attempt to use null value and pass with this change.
  • eval: a return inside a deferred block still sets the returned value (as the last statement and before it) passes on main and with this change; it fails if the helper restores the pending value unconditionally. The leftover-value test also fails if the helper restores outside a returning scope.

zig build test -Denable_jsc=false -Dfilter=interpreter on macOS: 27/27. On Linux the interpreter test step does not compile on main or on this branch, because the parser's module resolver calls libc access and that test target does not link libc.

GitHub Actions does not run on this repo at the moment, so I ran the full suite on Linux x86_64 with the pinned Zig 0.17.0-dev.2163, on main (c2793af) and on this branch. zig build test --summary all passes 9150/9161 on both, with the same 3 failing tests, 8 skips and 21 failed build steps (the interpreter test step is one of them). The diagnostic snapshot harness gives identical output on both (70 of 95 cases pass on main). With the built home, a program that returns through deferred calls in a closure, a for loop, a function and a function whose deferred call runs a closure prints void and then panics on main, and prints 9, 8, 7, 10 on this branch.

2 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.