Re: [Tarantool-patches] [PATCH] luajit: adde flag of partial comp. with Lua 5.2

[Igor Munkin <imun@tarantool.org>]

Olya,

Thanks for the patch. It provides the requested feature but in slightly
different way than we discussed before. The partial support of Lua 5.2
is already provided via LUAJIT_ENABLE_LUA52COMPAT flag, and the new flag
is designed to introduce only __pairs/__ipairs metamethods handling
(I've written this in a review for the patch with box.cfg modification fix
and in #4560[1]).

Furthermore, let's see whether any tests can be added for this
changeset.

On 19.11.19, Olga Arkhangelskaia wrote:

Use imperative mood in the subject.

> Although we do not turn on full compatibility with Lua 5.2 we do need some of
> its features (eg. __pairs/__ipairs).This functionality brakes existing code.

Typo: s/brakes/breakes/ + adjust whitespace.

> However, if the necessity is very high or is strongly demanded we should
> have possibility to use it. We introduce LUAJIT_ENABLE_LUA52COMPAT_PART
> flag.
> 
> Some of the Lua 5.2's features that are likely to break existing code can be
> turned on under this flag. At the moment it is pirs/ipairs metmethod.

Typo: s/pirs/pairs/.

Feel free to add `Part-of tarantool gh-4560` here referring the issue
where the feature has been requested.

> ---
>  src/Makefile    | 4 ++++
>  src/lib_base.c  | 4 ++--
>  src/lj_arch.h   | 6 ++++++
>  src/lj_obj.h    | 2 +-
>  src/vm_x86.dasc | 4 ++--
>  5 files changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/src/Makefile b/src/Makefile
> index 827d4a4..e8fd91a 100644
> --- a/src/Makefile
> +++ b/src/Makefile
> @@ -101,6 +101,10 @@ XCFLAGS=
>  # Note: this does not provide full compatibility with Lua 5.2 at this time.
>  #XCFLAGS+= -DLUAJIT_ENABLE_LUA52COMPAT
>  #
> +#Features from Lua 5.2 that was demanded more than once or are essential for
> +#for the project. At the moment only pairs/ipairs can be enabled via this flag.
> +#XCFLAGS+= -DLUAJIT_ENABLE_LUA52COMPAT_PART
> +#
>  # 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..6cc0fcc 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_52_PART || 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_52_PART || 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..39d70d5 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_LUA52COMPAT_PART) || defined (LUAJIT_ENABLE_LUA52COMPAT)
> +#define LJ_52_PART			1
> +#else
> +#define LJ_52_PART			0
> +#endif
>  #endif
> diff --git a/src/lj_obj.h b/src/lj_obj.h
> index f368578..3788cee 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_52 || LJ_HASFFI || LJ_52_PART

Minor: check for LJ_52 is an excess here considering the introduced
flag.

>  #define MMDEF_PAIRS(_) _(pairs) _(ipairs)
>  #else
>  #define MMDEF_PAIRS(_)
> diff --git a/src/vm_x86.dasc b/src/vm_x86.dasc
> index 56bee14..ad6216e 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_52_PART
>    |  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_52_PART
>    |  cmp dword TAB:RB->metatable, 0; jne ->fff_fallback
>  #endif
>    |  mov CFUNC:RB, [BASE-8]
> -- 
> 2.20.1 (Apple Git-117)
> 

[1]: https://github.com/tarantool/tarantool/issues/4560#issuecomment-552891850

-- 
Best regards,
IM