From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id ACAAA469719 for ; Fri, 6 Mar 2020 20:46:53 +0300 (MSK) Date: Fri, 6 Mar 2020 20:41:30 +0300 From: Igor Munkin Message-ID: <20200306174130.GE404@tarantool.org> References: <42037456891c91be0109634ff50505b054f6eac8.1583511306.git.imun@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <42037456891c91be0109634ff50505b054f6eac8.1583511306.git.imun@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v2] build: disable LUAJIT_ENABLE_PAIRSMM List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy Cc: tarantool-patches@dev.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 > --- > > @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:
'") > + | --- > + | - 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:
'") -- -- 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:
'") > + > +-- > +-- 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:
'") -- -- 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