Re: [Tarantool-patches] [PATCH luajit] test: adapt test checking tail calls debug info

[Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi, Maxim!

Thanks for the patch!
LGTM, except a few nits below.

On 30.09.21, Maxim Kokryashkin wrote:
> LuaJIT does not provide information about tail calls,
> unlike Lua does. getfenv() behavior for this test set is also different in
> LuaJIT, because tail calls do not provide additional call frame.
> 
> Closes tarantool/tarantool#5702
> Part of tarantool/tarantool#5845
> Part of tarantool/tarantool#4473

Looks like it should be 5870 instead 4473. Also, 5845 is already
closed.

> ---
> GitHub branch: https://github.com/tarantool/luajit/tree/fckxorg/gh-5702-adapt-getfenv-getinfo-PUC-Rio
> Issue: https://github.com/tarantool/tarantool/issues/5702
> See also https://luajit.org/status.html
> 
>  test/PUC-Rio-Lua-5.1-tests/db.lua | 61 +++++++++++++------------------
>  1 file changed, 26 insertions(+), 35 deletions(-)
> 
> diff --git a/test/PUC-Rio-Lua-5.1-tests/db.lua b/test/PUC-Rio-Lua-5.1-tests/db.lua
> index 56f59ea8..a94f07ab 100644
> --- a/test/PUC-Rio-Lua-5.1-tests/db.lua
> +++ b/test/PUC-Rio-Lua-5.1-tests/db.lua
> @@ -378,21 +378,22 @@ if jit_is_enabled then
>    jit.on()
>  end
>  
> -

Nit: this change is excess.

>  -- tests for tail calls
> +-- LuaJIT does not provide information about tail calls,
> +-- unlike Lua does. See also https://luajit.org/status.html.
> +-- getfenv() behavior is also different here, because tail calls
> +-- do not provide additional call frame for LuaJIT.
> +-- See also https://github.com/tarantool/tarantool/issues/5702.
> +-- This function is adapted to LuaJIT behavior.
>  local function f (x)
>    if x then
>      assert(debug.getinfo(1, "S").what == "Lua")
>      local tail = debug.getinfo(2)
> -    assert(not pcall(getfenv, 3))
> -    assert(tail.what == "tail" and tail.short_src == "(tail call)" and
> -           tail.linedefined == -1 and tail.func == nil)
> -    assert(debug.getinfo(3, "f").func == g1)
> +    assert(pcall(getfenv, 3))
> +    assert(tail.what == "Lua" and tail.linedefined == 403 and tail.func == g1)
> +    -- End of single assert

Don't get this comment.

>      assert(getfenv(3))
> -    assert(debug.getinfo(4, "S").what == "tail")
> -    assert(not pcall(getfenv, 5))
> -    assert(debug.getinfo(5, "S").what == "main")
> -    assert(getfenv(5))
> +    assert(debug.getinfo(3, "S").what == "main")
>      print"+"
>      end
>  end
> @@ -403,43 +404,33 @@ function g1(x) g(x) end
>  
>  local function h (x) local f=g1; return f(x) end
>  
> --- FIXME: LuaJIT does not provide information about tail calls,
> --- unlike Lua does. See also https://luajit.org/status.html.
> --- getfenv() behaviour is also different here, because tail calls
> --- do not provide additional call frame for LuaJIT and level
> --- number should be changed.
> --- Test is disabled for LuaJIT.
> --- See also https://github.com/tarantool/tarantool/issues/5702.
> --- h(true)
> +-- This fucntions is adapted to LuaJIT behavior. See the comment above.

Nit: This comment is excess IMHO: here nothing is changed.
Feel free to ignore.

> +h(true)
>  
>  local b = {}
> --- FIXME: Behavior is different for LuaJIT. See the comment above.
> --- Test is disabled for LuaJIT.
> --- debug.sethook(function (e) table.insert(b, e) end, "cr")
> --- h(false)
> --- debug.sethook()
> -local res = {"return",   -- first return (from sethook)
> -  "call", "call", "call", "call",
> -  "return", "tail return", "return", "tail return",
> -  "call",    -- last call (to sethook)
> +debug.sethook(function (e) table.insert(b, e) end, "cr")
> +-- This fucntions is adapted to LuaJIT behavior. See the comment above.
> +h(false)
> +debug.sethook()
> +-- This chunk is adapted to LuaJIT behavior. See the comment above.
> +local res = {"call",   -- first return (from sethook)
> +  "call", "call", "call", "return",
> +  "return", "call"
>  }
> --- FIXME: Behavior is different for LuaJIT. See the comment above.
> --- Test is disabled for LuaJIT.
> --- for _, k in ipairs(res) do assert(k == table.remove(b, 1)) end
> +for _, k in ipairs(res) do assert(k == table.remove(b, 1)) end
>  
>  
> -lim = 30000
> +-- This chunk is adapted to LuaJIT behavior. See the comment above.

Please mention the function nearby the comment as it was done before.
Here and above.

> +lim = 2
>  local function foo (x)
>    if x==0 then
> -    assert(debug.getinfo(lim+2).what == "main")
> -    for i=2,lim do assert(debug.getinfo(i, "S").what == "tail") end
> +    assert(debug.getinfo(lim + 1,"S").what == "main")
> +    assert(debug.getinfo(lim, "S").what == "main")
>    else return foo(x-1)
>    end
>  end
>  
> --- FIXME: Behavior is different for LuaJIT.
> --- See the comment to `h()` above. Test is disabled for LuaJIT.
> --- foo(lim)
> +foo(lim)
>  
>  
>  print"+"
> -- 
> 2.33.0
> 

-- 
Best regards,
Sergey Kaplun