[Tarantool-patches] [PATCH v2 luajit 20/30] test: adapt PUC Lua test for args in vararg func

[Igor Munkin]

Sergey,

Thanks for the patch! I can't understand why this patch is separated
from the previous two (10 and 11). Could you provide a rationale for
this, please? Also consider the comments below.

On 26.03.21, Sergey Kaplun wrote:
> Lua 5.1 interprets ... in the vararg functions like additional first

Typo: s/like additional/as an additional/.

> argument, unlike LuaJIT does. All extra arguments is set into `arg`

Typo: s/arguments is set/arguments are set/.

> variable.
> 
> Implicit `arg` parameter for old-style vararg functions was finally
> removed in Lua 5.2. This patch adjust tests in vararg.lua considering

Minor: Did LuaJIT always respect such behaviour? If no, please mention
the commit where it has been changed.

> Lua 5.2 test suite taken from
> https://www.lua.org/tests/lua-5.2.0-tests.tar.gz.
> 
> Closes tarantool/tarantool#5712

As we discussed before: s/Closes/Resolves/.

> Part of tarantool/tarantool#5845
> Part of tarantool/tarantool#4473
> ---
>  test/PUC-Lua-5.1-tests/vararg.lua | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/test/PUC-Lua-5.1-tests/vararg.lua b/test/PUC-Lua-5.1-tests/vararg.lua
> index ae068fa..efb76c5 100644
> --- a/test/PUC-Lua-5.1-tests/vararg.lua
> +++ b/test/PUC-Lua-5.1-tests/vararg.lua
> @@ -2,9 +2,13 @@ print('testing vararg')
>  
>  _G.arg = nil
>  
> +-- Lua 5.1 interprets ... in the vararg functions like additional

Typo: s/like additional/as an additional/.

> +-- first argument, unlike LuaJIT does. All extra arguments is set

Typo: s/arguments is set/arguments are set/.

> +-- into `arg` variable. This extension is from Lua 5.2.
> +-- See also https://github.com/tarantool/tarantool/issues/5712.

Side note: What is the difference between #5712 and #5694? They look
like duplicates to me, and so provide another rationale for squashing
all three patches into a single one.

> +-- LuaJIT: Test is adapted from PUC-Rio Lua 5.2 test suite.
>  function f(a, ...)
> -  assert(type(arg) == 'table')
> -  assert(type(arg.n) == 'number')
> +  local arg = {n = select('#', ...), ...}

Why did you drop the assertions above? They are trivial (as many of
others in this suite), but still check that everything is fine (e.g.
that <select> yields a number).

>    for i=1,arg.n do assert(a[i]==arg[i]) end
>    return arg.n
>  end

<snipped>

> @@ -42,7 +48,9 @@ a = call(print, {'+'})
>  assert(a == nil)
>  
>  local t = {1, 10}
> -function t:f (...) return self[arg[1]]+arg.n end
> +-- LuaJIT: Test chunk is adapted from PUC-Rio Lua 5.2 test suite.
> +-- See the comment above.
> +function t:f (...) local arg = {...}; return self[...]+#arg end

Why didn't you create the same structure as elsewhere above? Why did you
change <self> indexing? AFAIU, after creating the *right* <arg> variable
everything should work with no other changes.

>  assert(t:f(1,4) == 3 and t:f(2) == 11)
>  print('+')
>  
> -- 
> 2.31.0
> 

-- 
Best regards,
IM