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

[Igor Munkin <imun@tarantool.org>]

Vlad,

I discussed your comments with Sasha Tu. offline and he totally agreed
with both. Fixed, squashed and force-pushed the branch. Diffs are below.

On 06.03.20, Igor Munkin wrote:
> 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>
> ---
> 
> @ChangeLog (need to be added to already released versions):
> | __pairs/__ipairs metamethods handling is removed since we faced the
> | issues with the backward compatibility between Lua 5.1 and Lua 5.2
> | within Tarantool modules as well as other third party code (gh-4770).
> 
> Issue: https://github.com/tarantool/tarantool/issues/4770
> Branch: https://github.com/tarantool/tarantool/tree/imun/gh-4770-broken-pairs
> 
> v1: https://lists.tarantool.org/pipermail/tarantool-patches/2020-February/014375.html
> 
> Changes in v2:
> * moved test to a separate file
> 
>  cmake/luajit.cmake                            |  1 -
>  ...4770-broken-pairs-for-space-objects.result | 52 +++++++++++++++++++
>  ...70-broken-pairs-for-space-objects.test.lua | 18 +++++++
>  3 files changed, 70 insertions(+), 1 deletion(-)
>  create mode 100644 test/box/gh-4770-broken-pairs-for-space-objects.result
>  create mode 100644 test/box/gh-4770-broken-pairs-for-space-objects.test.lua
> 
> 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/gh-4770-broken-pairs-for-space-objects.result b/test/box/gh-4770-broken-pairs-for-space-objects.result
> new file mode 100644
> index 000000000..61618453e
> --- /dev/null
> +++ b/test/box/gh-4770-broken-pairs-for-space-objects.result
> @@ -0,0 +1,52 @@
> +-- test-run result file version 2
> +env = require('test_run')
> + | ---
> + | ...
> +test_run = env.new()
> + | ---
> + | ...
> +test_run:cmd("push filter 'table: .*' to 'table: <address>'")
> + | ---
> + | - true
> + | ...
> +
> +--
> +-- gh-4770: Iteration through space with Lua builtin pairs routine
> +--
> +box.cfg{}
> + | ---
> + | ...

================================================================================

diff --git a/test/box/gh-4770-broken-pairs-for-space-objects.result b/test/box/gh-4770-broken-pairs-for-space-objects.result
index 61618453e..2e31d5fbe 100644
--- a/test/box/gh-4770-broken-pairs-for-space-objects.result
+++ b/test/box/gh-4770-broken-pairs-for-space-objects.result
@@ -13,9 +13,6 @@ test_run:cmd("push filter 'table: .*' to 'table: <address>'")
 --
 -- gh-4770: Iteration through space with Lua builtin pairs routine
 --
-box.cfg{}
- | ---
- | ...
 s = box.schema.create_space('test')
  | ---
  | ...

================================================================================

> +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 == s.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
> + | ...

================================================================================

diff --git a/test/box/gh-4770-broken-pairs-for-space-objects.result b/test/box/gh-4770-broken-pairs-for-space-objects.result
index 2e31d5fbe..3ac5995c2 100644
--- a/test/box/gh-4770-broken-pairs-for-space-objects.result
+++ b/test/box/gh-4770-broken-pairs-for-space-objects.result
@@ -30,19 +30,11 @@ 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
+keys.name, keys.id, keys.engine
+ | ---
+ | - true
+ | - true
+ | - true
  | ...
 s:drop()
  | ---

================================================================================

> +s:drop()
> + | ---
> + | ...
> diff --git a/test/box/gh-4770-broken-pairs-for-space-objects.test.lua b/test/box/gh-4770-broken-pairs-for-space-objects.test.lua
> new file mode 100644
> index 000000000..15024a803
> --- /dev/null
> +++ b/test/box/gh-4770-broken-pairs-for-space-objects.test.lua
> @@ -0,0 +1,18 @@
> +env = require('test_run')
> +test_run = env.new()
> +test_run:cmd("push filter 'table: .*' to 'table: <address>'")
> +
> +--
> +-- gh-4770: Iteration through space with Lua builtin pairs routine
> +--
> +box.cfg{}

================================================================================

diff --git a/test/box/gh-4770-broken-pairs-for-space-objects.test.lua b/test/box/gh-4770-broken-pairs-for-space-objects.test.lua
index 15024a803..7acf76eb5 100644
--- a/test/box/gh-4770-broken-pairs-for-space-objects.test.lua
+++ b/test/box/gh-4770-broken-pairs-for-space-objects.test.lua
@@ -5,7 +5,6 @@ test_run:cmd("push filter 'table: .*' to 'table: <address>'")
 --
 -- gh-4770: Iteration through space with Lua builtin pairs routine
 --
-box.cfg{}
 s = box.schema.create_space('test')
 -- Check whether __pairs is set for the space object, since Lua Fun
 -- handles it manually underneath.

================================================================================

> +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 == s.pairs
> +-- 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

================================================================================

diff --git a/test/box/gh-4770-broken-pairs-for-space-objects.test.lua b/test/box/gh-4770-broken-pairs-for-space-objects.test.lua
index 7acf76eb5..315352b15 100644
--- a/test/box/gh-4770-broken-pairs-for-space-objects.test.lua
+++ b/test/box/gh-4770-broken-pairs-for-space-objects.test.lua
@@ -13,5 +13,5 @@ getmetatable(s).__pairs == s.pairs
 -- __pairs is set.
 keys = {}
 for k, v in pairs(s) do keys[k] = true end
-keys
+keys.name, keys.id, keys.engine
 s:drop()

================================================================================

> +s:drop()
> -- 
> 2.25.0
> 

-- 
Best regards,
IM