* [Tarantool-patches] [PATCH v2] luajit: adde flag of partial comp. with Lua 5.2
@ 2019-11-20 8:53 Olga Arkhangelskaia
2019-11-20 12:54 ` Igor Munkin
0 siblings, 1 reply; 2+ messages in thread
From: Olga Arkhangelskaia @ 2019-11-20 8:53 UTC (permalink / raw)
To: tarantool-patches
Although we do not turn on full compatibility with Lua 5.2 we do need pairs/ipairs
metamethods. Patch introduces LUAJIT_ENABLE_PAIRSMM flag.
If the flag is set - metamethods are available.
---
Closes #4560
Branch https://github.com/tarantool/luajit/tree/OKriw/gh-4560-Enable-LUAJIT_ENABLE_LUA52COMPAT-when-bulding-luajit
Changes in v2:
- flag is renamed
- now flag is used only for pairsmm
- added test case
src/Makefile | 3 +++
src/lib_base.c | 4 ++--
src/lj_arch.h | 6 ++++++
src/lj_obj.h | 2 +-
src/vm_x86.dasc | 4 ++--
test/mmpairs_test.lua | 48 +++++++++++++++++++++++++++++++++++++++++++
6 files changed, 62 insertions(+), 5 deletions(-)
create mode 100755 test/mmpairs_test.lua
diff --git a/src/Makefile b/src/Makefile
index 827d4a4..af84f0c 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -101,6 +101,9 @@ XCFLAGS=
# Note: this does not provide full compatibility with Lua 5.2 at this time.
#XCFLAGS+= -DLUAJIT_ENABLE_LUA52COMPAT
#
+#Enable pairs/ipairs methametods from Lua 5.2.
+XCFLAGS+= -DLUAJIT_ENABLE_PAIRSMM
+#
# Disable the JIT compiler, i.e. turn LuaJIT into a pure interpreter.
#XCFLAGS+= -DLUAJIT_DISABLE_JIT
#
diff --git a/src/lib_base.c b/src/lib_base.c
index 3a75787..950a580 100644
--- a/src/lib_base.c
+++ b/src/lib_base.c
@@ -81,12 +81,12 @@ LJLIB_ASM(next)
return FFH_UNREACHABLE;
}
-#if LJ_52 || LJ_HASFFI
+#if LJ_PAIRSMM || LJ_HASFFI
static int ffh_pairs(lua_State *L, MMS mm)
{
TValue *o = lj_lib_checkany(L, 1);
cTValue *mo = lj_meta_lookup(L, o, mm);
- if ((LJ_52 || tviscdata(o)) && !tvisnil(mo)) {
+ if ((LJ_PAIRSMM || tviscdata(o)) && !tvisnil(mo)) {
L->top = o+1; /* Only keep one argument. */
copyTV(L, L->base-1-LJ_FR2, mo); /* Replace callable. */
return FFH_TAILCALL;
diff --git a/src/lj_arch.h b/src/lj_arch.h
index c8d7138..6dfb4fb 100644
--- a/src/lj_arch.h
+++ b/src/lj_arch.h
@@ -564,4 +564,10 @@
#define LJ_52 0
#endif
+/* Partial compatibility with Lua 5.2 */
+#if defined(LUAJIT_ENABLE_PAIRSMM) || defined (LUAJIT_ENABLE_LUA52COMPAT)
+#define LJ_PAIRSMM 1
+#else
+#define LJ_PAIRSMM 0
+#endif
#endif
diff --git a/src/lj_obj.h b/src/lj_obj.h
index f368578..4d0693f 100644
--- a/src/lj_obj.h
+++ b/src/lj_obj.h
@@ -530,7 +530,7 @@ enum {
#define MMDEF_FFI(_)
#endif
-#if LJ_52 || LJ_HASFFI
+#if LJ_HASFFI || LJ_PAIRSMM
#define MMDEF_PAIRS(_) _(pairs) _(ipairs)
#else
#define MMDEF_PAIRS(_)
diff --git a/src/vm_x86.dasc b/src/vm_x86.dasc
index 56bee14..eae07f7 100644
--- a/src/vm_x86.dasc
+++ b/src/vm_x86.dasc
@@ -1724,7 +1724,7 @@ static void build_subroutines(BuildCtx *ctx)
|.ffunc_1 pairs
| mov TAB:RB, [BASE]
| cmp dword [BASE+4], LJ_TTAB; jne ->fff_fallback
-#if LJ_52
+#if LJ_PAIRSMM
| cmp dword TAB:RB->metatable, 0; jne ->fff_fallback
#endif
| mov CFUNC:RB, [BASE-8]
@@ -1791,7 +1791,7 @@ static void build_subroutines(BuildCtx *ctx)
|.ffunc_1 ipairs
| mov TAB:RB, [BASE]
| cmp dword [BASE+4], LJ_TTAB; jne ->fff_fallback
-#if LJ_52
+#if LJ_PAIRSMM
| cmp dword TAB:RB->metatable, 0; jne ->fff_fallback
#endif
| mov CFUNC:RB, [BASE-8]
diff --git a/test/mmpairs_test.lua b/test/mmpairs_test.lua
new file mode 100755
index 0000000..3334c2d
--- /dev/null
+++ b/test/mmpairs_test.lua
@@ -0,0 +1,48 @@
+#!/usr/bin/env tarantool
+
+tap = require('tap')
+
+test = tap.test("PAIRSMM-is-set")
+test:plan(2)
+
+--- To make sure that we are using only pairs/ipairs mmethods within Lua5.1
+--- we use os.execute, because its interface has changed significantly in 5.2
+
+local mt = {
+ __pairs = function(self)
+ local function stateless_iter(tbl, k)
+ local v
+ k, v = next(tbl, k)
+ while k and v > 0 do k, v = next(tbl, k) end
+ if v then return k,v end
+ end
+ return stateless_iter, self, nil
+ end,
+ __ipairs = function(self)
+ local function stateless_iter(tbl, k)
+ local v
+ k, v = next(tbl, k)
+ while k and v < 0 do k, v = next(tbl, k) end
+ if v then return k,v end
+ end
+ return stateless_iter, self, nil
+ end
+}
+
+local t = setmetatable({ }, mt)
+t[1] = 10
+t[2] = 20
+t[3] = -10
+t[4] = -20
+
+local pairs_res = 0
+local ipairs_res = 0
+for k, v in pairs(t) do
+ pairs_res = v + pairs_res
+end
+for k, v in ipairs(t) do
+ ipairs_res = v + ipairs_res
+end
+test:is(pairs_res + ipairs_res, 0)
+os_exec_res = os.execute()
+test:is(os_exec_res, 1)
--
2.20.1 (Apple Git-117)
^ permalink raw reply [flat|nested] 2+ messages in thread* Re: [Tarantool-patches] [PATCH v2] luajit: adde flag of partial comp. with Lua 5.2
2019-11-20 8:53 [Tarantool-patches] [PATCH v2] luajit: adde flag of partial comp. with Lua 5.2 Olga Arkhangelskaia
@ 2019-11-20 12:54 ` Igor Munkin
0 siblings, 0 replies; 2+ messages in thread
From: Igor Munkin @ 2019-11-20 12:54 UTC (permalink / raw)
To: Olga Arkhangelskaia; +Cc: tarantool-patches
Olya,
Thanks for the patch, the changes you made since v1 are relevant,
however please consider several new comments I left below.
On 20.11.19, Olga Arkhangelskaia wrote:
> Although we do not turn on full compatibility with Lua 5.2 we do need pairs/ipairs
> metamethods. Patch introduces LUAJIT_ENABLE_PAIRSMM flag.
> If the flag is set - metamethods are available.
Please adjust the subject of the commit (there is a typo and its format
doesn't respect the contribution guide).
> ---
> Closes #4560
Strictly saying this commit will never "close" the #4560, due to it's
not in the projects queue. I propose to append "Part of"/"Needed by"
label to the commit message (move it prior to three dashes).
> Branch https://github.com/tarantool/luajit/tree/OKriw/gh-4560-Enable-LUAJIT_ENABLE_LUA52COMPAT-when-bulding-luajit
>
> Changes in v2:
> - flag is renamed
> - now flag is used only for pairsmm
> - added test case
>
>
> src/Makefile | 3 +++
> src/lib_base.c | 4 ++--
> src/lj_arch.h | 6 ++++++
> src/lj_obj.h | 2 +-
> src/vm_x86.dasc | 4 ++--
> test/mmpairs_test.lua | 48 +++++++++++++++++++++++++++++++++++++++++++
> 6 files changed, 62 insertions(+), 5 deletions(-)
> create mode 100755 test/mmpairs_test.lua
>
> diff --git a/src/Makefile b/src/Makefile
> index 827d4a4..af84f0c 100644
> --- a/src/Makefile
> +++ b/src/Makefile
> @@ -101,6 +101,9 @@ XCFLAGS=
> # Note: this does not provide full compatibility with Lua 5.2 at this time.
> #XCFLAGS+= -DLUAJIT_ENABLE_LUA52COMPAT
> #
> +#Enable pairs/ipairs methametods from Lua 5.2.
> +XCFLAGS+= -DLUAJIT_ENABLE_PAIRSMM
IMHO there is no need to enable this flag by default within the root
Makefile.
> +#
> # Disable the JIT compiler, i.e. turn LuaJIT into a pure interpreter.
> #XCFLAGS+= -DLUAJIT_DISABLE_JIT
> #
> diff --git a/src/lib_base.c b/src/lib_base.c
> index 3a75787..950a580 100644
> --- a/src/lib_base.c
> +++ b/src/lib_base.c
> @@ -81,12 +81,12 @@ LJLIB_ASM(next)
> return FFH_UNREACHABLE;
> }
>
> -#if LJ_52 || LJ_HASFFI
> +#if LJ_PAIRSMM || LJ_HASFFI
> static int ffh_pairs(lua_State *L, MMS mm)
> {
> TValue *o = lj_lib_checkany(L, 1);
> cTValue *mo = lj_meta_lookup(L, o, mm);
> - if ((LJ_52 || tviscdata(o)) && !tvisnil(mo)) {
> + if ((LJ_PAIRSMM || tviscdata(o)) && !tvisnil(mo)) {
> L->top = o+1; /* Only keep one argument. */
> copyTV(L, L->base-1-LJ_FR2, mo); /* Replace callable. */
> return FFH_TAILCALL;
> diff --git a/src/lj_arch.h b/src/lj_arch.h
> index c8d7138..6dfb4fb 100644
> --- a/src/lj_arch.h
> +++ b/src/lj_arch.h
> @@ -564,4 +564,10 @@
> #define LJ_52 0
> #endif
>
> +/* Partial compatibility with Lua 5.2 */
Please adjust the comment.
> +#if defined(LUAJIT_ENABLE_PAIRSMM) || defined (LUAJIT_ENABLE_LUA52COMPAT)
> +#define LJ_PAIRSMM 1
> +#else
> +#define LJ_PAIRSMM 0
> +#endif
> #endif
> diff --git a/src/lj_obj.h b/src/lj_obj.h
> index f368578..4d0693f 100644
> --- a/src/lj_obj.h
> +++ b/src/lj_obj.h
> @@ -530,7 +530,7 @@ enum {
> #define MMDEF_FFI(_)
> #endif
>
> -#if LJ_52 || LJ_HASFFI
> +#if LJ_HASFFI || LJ_PAIRSMM
> #define MMDEF_PAIRS(_) _(pairs) _(ipairs)
> #else
> #define MMDEF_PAIRS(_)
> diff --git a/src/vm_x86.dasc b/src/vm_x86.dasc
> index 56bee14..eae07f7 100644
> --- a/src/vm_x86.dasc
> +++ b/src/vm_x86.dasc
I see no option except either introducing the flag for all others VMs
or adjusting its enabling in lj_arch.h only for those supported by
Tarantool (x64, arm). Considering the changes to be done, I guess the
first approach is the right way to fix the current inconsistency, but
feel free to choose the one that seems to be the most convenient to you.
> @@ -1724,7 +1724,7 @@ static void build_subroutines(BuildCtx *ctx)
> |.ffunc_1 pairs
> | mov TAB:RB, [BASE]
> | cmp dword [BASE+4], LJ_TTAB; jne ->fff_fallback
> -#if LJ_52
> +#if LJ_PAIRSMM
> | cmp dword TAB:RB->metatable, 0; jne ->fff_fallback
> #endif
> | mov CFUNC:RB, [BASE-8]
> @@ -1791,7 +1791,7 @@ static void build_subroutines(BuildCtx *ctx)
> |.ffunc_1 ipairs
> | mov TAB:RB, [BASE]
> | cmp dword [BASE+4], LJ_TTAB; jne ->fff_fallback
> -#if LJ_52
> +#if LJ_PAIRSMM
> | cmp dword TAB:RB->metatable, 0; jne ->fff_fallback
> #endif
> | mov CFUNC:RB, [BASE-8]
> diff --git a/test/mmpairs_test.lua b/test/mmpairs_test.lua
> new file mode 100755
> index 0000000..3334c2d
> --- /dev/null
> +++ b/test/mmpairs_test.lua
> @@ -0,0 +1,48 @@
> +#!/usr/bin/env tarantool
> +
> +tap = require('tap')
> +
> +test = tap.test("PAIRSMM-is-set")
> +test:plan(2)
> +
> +--- To make sure that we are using only pairs/ipairs mmethods within Lua5.1
> +--- we use os.execute, because its interface has changed significantly in 5.2
I guess the comment can be extended with the following rationale.
| Currently there is no Lua way to detect whether LJ_PAIRSMM is enabled
| or not. Therefore taking into account the fact it's enabled by default
| in tarantool built and considering the existing testing pipeline we
| can check the following scenario: while we can overload the
| pairs/ipairs behaviour via metamethod as designed in Lua 5.2,
| os.execute still preserves the Lua 5.1 interface.
> +
> +local mt = {
> + __pairs = function(self)
> + local function stateless_iter(tbl, k)
> + local v
> + k, v = next(tbl, k)
> + while k and v > 0 do k, v = next(tbl, k) end
> + if v then return k,v end
> + end
> + return stateless_iter, self, nil
> + end,
> + __ipairs = function(self)
> + local function stateless_iter(tbl, k)
> + local v
> + k, v = next(tbl, k)
> + while k and v < 0 do k, v = next(tbl, k) end
> + if v then return k,v end
> + end
> + return stateless_iter, self, nil
> + end
> +}
> +
> +local t = setmetatable({ }, mt)
> +t[1] = 10
> +t[2] = 20
> +t[3] = -10
> +t[4] = -20
> +
> +local pairs_res = 0
> +local ipairs_res = 0
> +for k, v in pairs(t) do
> + pairs_res = v + pairs_res
> +end
> +for k, v in ipairs(t) do
> + ipairs_res = v + ipairs_res
> +end
> +test:is(pairs_res + ipairs_res, 0)
> +os_exec_res = os.execute()
> +test:is(os_exec_res, 1)
> --
> 2.20.1 (Apple Git-117)
>
--
Best regards,
IM
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-11-20 12:56 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-20 8:53 [Tarantool-patches] [PATCH v2] luajit: adde flag of partial comp. with Lua 5.2 Olga Arkhangelskaia
2019-11-20 12:54 ` Igor Munkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox