Re: [Tarantool-patches] [PATCH] build: disable LUAJIT_ENABLE_PAIRSMM

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Hi! Thanks for the patch!

See 2 comments below.

>     build: disable LUAJIT_ENABLE_PAIRSMM
>     
>     Since this build flag has been removed as a result of reverting the
>     tarantool/luajit@d4e985a, its definition in the corresponding Tarantool
>     cmake file is irrelevant.
>     
>     Furthermore, considering the breakage faced in #4770 the following tests
>     are introduced:
>     * the check whether space __pairs metamethod is set to space.pairs to
>       create a Lua Fun iterator that handles __pairs manually underneath.
>     * the check whether pairs builtin behaviour doesn't change when __pairs
>       is set e.g. on space object.
>     
>     Follow-up #4560
>     Closes #4770
>     
>     Signed-off-by: Igor Munkin <imun@tarantool.org>
> 
> diff --git a/cmake/luajit.cmake b/cmake/luajit.cmake
> index 072db8269..10df633d5 100644
> --- a/cmake/luajit.cmake
> +++ b/cmake/luajit.cmake
> @@ -217,7 +217,6 @@ macro(luajit_build)
>          add_definitions(-DLUAJIT_USE_ASAN=1)
>          set (luajit_ldflags ${luajit_ldflags} -fsanitize=address)
>      endif()
> -    add_definitions(-DLUAJIT_ENABLE_PAIRSMM=1)
>      add_definitions(-DLUAJIT_SMART_STRINGS=1)
>      # Add all COMPILE_DEFINITIONS to xcflags
>      get_property(defs DIRECTORY PROPERTY COMPILE_DEFINITIONS)
> diff --git a/test/box/misc.result b/test/box/misc.result
> index 5ac5e0f26..6fc035171 100644
> --- a/test/box/misc.result
> +++ b/test/box/misc.result
> @@ -1431,3 +1431,43 @@ test_run:grep_log('default', 'test log')
>  ---
>  - test log
>  ...
> +--
> +-- gh-4770: Iteration through space with Lua builtin pairs routine
> +--
> +box.cfg{}

1. box.cfg{} is already done by test/box/box.lua, which starts
misc.test.lua.

> +---
> +...
> +s = box.schema.create_space('test')
> +---
> +...
> +-- Check whether __pairs is set for the space object, since Lua Fun
> +-- handles it manually underneath.
> +getmetatable(s).__pairs == space.pairs
> +---
> +- true
> +...
> +-- Check whether pairs builtin behaviour doesn't change when the
> +-- __pairs is set.
> +keys = {}
> +---
> +...
> +for k, v in pairs(s) do keys[k] = true end
> +---
> +...
> +keys
> +---
> +- engine: true
> +  before_replace: true
> +  field_count: true
> +  id: true
> +  on_replace: true
> +  temporary: true
> +  index: true
> +  is_local: true
> +  enabled: true
> +  name: true
> +  ck_constraint: true

2. I wouldn't print all the keys, since this test will break
anytime when something added to space. I would just check that
a couple of fields exists: keys.engine, keys.id, keys.name.
Up to you.