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

[Igor Munkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Max,

Thanks for the patch! LGTM, except a couple of nits, I've fixed by
myself.

On 13.10.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

Typo: This text is out of bounds (72 symbols for commit message).

> LuaJIT, because tail calls do not provide additional call frame.
> 

Typo: "Resolves tarantool/tarantool#5702" is missing.

> Part of tarantool/tarantool#5870
> ---
> GitHub branch: https://github.com/tarantool/luajit/tree/fckxorg/gh-5702-adapt-getfenv-getinfo-PUC-Rio
> CI: https://github.com/tarantool/tarantool/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 | 58 +++++++++++++------------------
>  1 file changed, 24 insertions(+), 34 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..cfe54cac 100644
> --- a/test/PUC-Rio-Lua-5.1-tests/db.lua
> +++ b/test/PUC-Rio-Lua-5.1-tests/db.lua
> @@ -380,19 +380,20 @@ end
>  
>  
>  -- 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))

Minor: this assertion is excess, considering the another one below.

> +    assert(tail.what == "Lua" and tail.linedefined == 403 and tail.func == g1)
>      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,32 @@ 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)
> +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.

Typo: s/fucntions/function/.

> +h(false)
> +debug.sethook()
> +-- This chunk is adapted to LuaJIT behavior. See the comment above.
> +local res = {"call",   -- first return (from sethook)

Minor: The comment is not relevant to the code. Furthermore, I see no
comment why there is no "return" entry for <debug.sethook> return.

> +  "call", "call", "call", "return",
> +  "return", "call"

Minor: The comment is missing, since last "call" entry belongs to
debug.sethook.

>  }
> --- 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
> +lim = 2
> +-- This function is adapted to LuaJIT behavior. See the comment above.
>  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")

Minor: Again, the comment for such change is missing here.

>    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,
IM