tjdevries / vim9jit

a vim9script -> lua transpiler (written in Rust)
MIT License
510 stars 21 forks source link

Ternary expressions containing lambdas result in error instead of evaluating #24

Open joom opened 1 year ago

joom commented 1 year ago

Consider the following vim9script code:

vim9script
var Foo = true ? (x: number): number => x : (x: number): number => x
echo Foo(10)

(this returns 10 in Vim 9)

This is currently compiled into this Lua code:

local vim9 = require('_vim9script')
local M = {}
local Foo = nil
-- vim9script
Foo = vim9.ternary(true, function(x)
  return x
end, function(x)
  return x
end)
print(Foo(10))
return M

... which throws an error for me:

Error detected while processing /Users/joomy/work/vim9jit/ex/bug.vim:
line    3:
E5108: Error executing lua ./bug.lua:19: attempt to call local 'Foo' (a nil value)
stack traceback:
        ./bug.lua:19: in function 'autoload'
        [string "luaeval()"]:1: in main chunk

(line 19 is the print(Foo(10)) line)

Given how ternary is implemented in _vim9script.lua, as long as the inputs to ternary are functions, they are called with no arguments: https://github.com/tjdevries/vim9jit/blob/af7d608a6a60077900b9954800761e2bfa4b7b11/lua/_vim9script.lua#L14-L28

And the "suspender" functions are only used if the branches of the ternary expressions have side effects: https://github.com/tjdevries/vim9jit/blob/af7d608a6a60077900b9954800761e2bfa4b7b11/crates/vim9-gen/src/lib.rs#L1275-L1294 and lambda expressions are considered not to have side effects: https://github.com/tjdevries/vim9jit/blob/af7d608a6a60077900b9954800761e2bfa4b7b11/crates/vim9-gen/src/lib.rs#L1249

But even without that, even an identifier can evaluate to a function, which will make type(if_true) == 'function' true in the ternary implementation in Lua. This identifier can come from outside, so you cannot determine whether a term has side effects syntactically.

The solution seems to be making ternary expression generation wrap the branches in function() return .. end all the time. (Lua doesn't seem to have lazy and/or either so that also doesn't work).

tjdevries commented 1 year ago

Great bug report! :)

I'll think about the best way to solve. Thanks