Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

luajit/lua 5.1 compatibility? #291

Open
lluixhi opened this issue Apr 24, 2016 · 12 comments
Open

luajit/lua 5.1 compatibility? #291

lluixhi opened this issue Apr 24, 2016 · 12 comments

Comments

@lluixhi
Copy link

lluixhi commented Apr 24, 2016

luajit is significantly faster than the standard lua interpreter, and some distros like gentoo still consider lua 5.2+ unstable.

luajit is missing 5 functions you're using that are available in lua 5.2+.

lua_setuservalue
lua_getuservalue
lua_checkunsigned
lua_pushunsigned
luaL_optunsigned
luaL_setfuncs

Which you could possibly pull from here:
https://github.com/keplerproject/lua-compat-5.2/tree/master/c-api
(MIT licensed)

@martanne
Copy link
Owner

I'm fairly new to the "Lua Universe" and thus probably do not know about some of the subtler differences/incompatibilities among the language versions. How is the general ecosystem? Which version is most popular? Are there large number of packages only working with one, but not the other version?

I agree that aside from some distribution packaging issues, defaulting to Lua 5.2 probably does not make much sense. Either we should follow the latest release i.e. 5.3 or stick to 5.1 and thus support luajit. Maybe it would even be possible to use the corresponding compatibility module for Lua 5.3=

The fact that Lua 5.3 added a proper 64-bit integer type might be helpful for the Lua API (see also #292).

Having said all that did you encounter actual performance problems? Syntax highlighting might be the only slightly performance critical code, but so far the primitive approach used seems to be fast enough. Most of the heavy lifting (pattern patching) is performed by the lpeg.so C module anyway. Supporting luajit just because some distributions do not manage to package standard Lua seems wrong.

Using luajit might have other benefits like the nice FFI. But again, I'm not sure how useful that actually is for providing an "idomatic" Lua API and not just a mapping to a set of C functions.

The problem with adding luajit support is that it adds to the (admittedly currently mostly non-existing) testing matrix. It needs to be properly integrated into the build system, someone has to make sure it actually keeps working etc.

Dropping support for the standard Lua interpreter is not an option because I'm interested in some architectures which are not supported by luajit.

@lluixhi
Copy link
Author

lluixhi commented Apr 26, 2016

I'm fairly new to the "Lua Universe" and thus probably do not know about some of the subtler differences/incompatibilities among the language versions.

I'm new to it too.

Concerning API/ABI compatibility:
Lua 5.2 isn't compatible with the Lua 5.1 API
Lua 5.3 isn't compatible with the Lua 5.2 API
http://lua-users.org/wiki/CompatibilityWithLuaFive

Most of these incompatibilities are added features as far as I can tell.

LuaJIT is API/ABI compatible with Lua 5.1
LuaJIT can be made (mostly) API compatible with Lua 5.2, if you enable the correct compile-time option. (A patch exists to make it 100% compatible)
The guy in charge of LuaJIT has no interest in 5.3 support.

As a result, the APIs between LuaJit and reference Lua are diverging more and more, and it looks like the Lua community is starting to split into two camps, the people who like Mike Pall's LuaJit and want the FFI and performance, and the people who want to use the reference implementation.

That said, I believe most people support Lua 5.1+. If they want features from Lua 5.3 in previous versions, they use some form of compatibility.

If you look at the TextAdept mailing list, there's a thread where they argue about it, and they end up supporting both Lua 5.3 and LuaJit, with a utf8 compatibility module for LuaJit.
Neovim has a hard dependency on Luajit, and they have no interest in 5.3 compatibility.

As for speed, LuaJit, according to their own benchmarks, is 1.5-100x faster than the reference implementation (though some of their cases are unfair, and in reality its more like an average 1.5x-5.5x speedup), so imo it's worth supporting.

However, I have not actually encountered any performance problems yet. I imagine if someone is working on a very large source file (which is bad coding practice anyway), you might get some lag with syntax highlighting. If someone was interested in creating an autocompletion plugin in lua (like Vim's YouCompleteMe), it'd be much more likely. The plugin ended up being written as a separate program receiving information from Vim in a client-server fashion because it's so slow.

Dropping support for the standard Lua interpreter is not an option because I'm interested in some architectures which are not supported by luajit.

This isn't really a problem, you'd still be supporting Lua 5.1

In the end, it's your call :)

@martanne
Copy link
Owner

The recently added code to handle digraphs uses file:close() to get the exit status of an io.popen-ed sub process. This functionality isn't available in standard Lua 5.1.

@lluixhi
Copy link
Author

lluixhi commented Jan 30, 2017

In the luacompat-5.2 package I mentioned, the implementations for file:close() with those return values is at:
https://github.com/keplerproject/lua-compat-5.2/blob/master/compat52.lua#L579-L646 for Lua 5.1

Luajit appears to support it.

@mcepl
Copy link
Contributor

mcepl commented Oct 28, 2019

Neovim has a hard dependency on Luajit, and they have no interest in 5.3 compatibility.

That is not correct, neovim works both on LuaJIT and Lua 5.3 (there is compat module now in the tree). neovim/neovim#9280

@mcepl
Copy link
Contributor

mcepl commented Sep 7, 2020

@lluixhi What is the relationship between https://github.com/keplerproject/lua-compat-5.2 and https://github.com/keplerproject/lua-compat-5.3? Is the former subset of the latter? I have here SUSE-Enterprise-Linux 12, which has natively only Lua 5.1, and I wonder how complicated it would be to port vis to it.

@mcepl
Copy link
Contributor

mcepl commented Sep 10, 2020

I have here this lua51-compatibility.patch, which makes vis on ancient platforms dependent on lua-compat-5.2. I am able to compile vis with it, and :help shows Lua support: as yes, but it is just a proof of concept. Every start I get the error message:

error loading module 'plugins/number-inc-dec' from file '/usr/share/vis/plugins/number-inc-dec.lua':
        /usr/share/vis/plugins/number-inc-dec.lua:20: '=' expected near 'continue'

Not sure what is that about (is continue reserved keyword and not allowed as a label?).

CORRECTION: Sorry, that was the binary from the previous version of the patch. This one starts vis without any error.

@mcepl
Copy link
Contributor

mcepl commented Feb 14, 2024

I observe that many plugins now use functionality not available in Lua 5.1/LuaJIT. I am afraid that this is now a lost cause. Let’s close this.

@rnpnr
Copy link
Collaborator

rnpnr commented Feb 15, 2024

I'm not sure it's entirely a lost cause. Considering everything in vis still supports 5.2 I don't think it's implausible to fix compatibility with Luajit.

5.3 and 5.4 are much bigger (relatively speaking) when static linking so there is still an argument for not requiring them. On the other hand 5.1/Luajit have language bugs that will never be fixed (I don't understand Luajit's instance on inheriting things which are broken but I digress).

I'm undecided on the matter.

@mcepl
Copy link
Contributor

mcepl commented Mar 25, 2024

Hmm, when trying to build with LuaJIT (luajit-5.1.2.1.0+git.1707061634.0d313b2 from LuaJIT/LuaJIT@0d313b2) with this patch

From 06be327838384447000bdc279f638b3fe5944a5f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mat=C4=9Bj=20Cepl?= <[email protected]>
Date: Mon, 25 Mar 2024 15:51:22 +0100
Subject: [PATCH vis] Enable LuaJIT

---
 configure | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/configure b/configure
index 0cc48a25..2566c349 100755
--- a/configure
+++ b/configure
@@ -425,15 +425,15 @@ test "$lpeg" = "yes" && lua=yes
 
 if test "$lua" != "no" ; then
 
-   printf "checking for liblua >= 5.2 ...\n"
+ printf "checking for liblua >= 5.1 ...\n"
 
 cat > "$tmpc" <<EOF
 #include <lua.h>
 #include <lualib.h>
 #include <lauxlib.h>
 
-#if LUA_VERSION_NUM < 502
-#error "Need at least Lua 5.2"
+#if LUA_VERSION_NUM < 501
+#error "Need at least Lua 5.1"
 #endif
 
 int main(int argc, char *argv[]) {
@@ -444,7 +444,7 @@ int main(int argc, char *argv[]) {
 }
 EOF
 
-   for liblua in lua lua5.4 lua5.3 lua5.2 lua-5.4 lua-5.3 lua-5.2 lua54 lua53 lua52; do
+ for liblua in lua lua5.4 lua5.3 lua5.2 lua-5.4 lua-5.3 lua-5.2 lua54 lua53 lua52 luajit; do
                printf " checking for %s... " "$liblua"
 
                if test "$have_pkgconfig" = "yes" ; then
-- 
2.44.0

the build failed:

[   15s] cc -O2 -Wall -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -fstack-protector-strong -funwind-tables -fasynchronous-unwind-tables -fstack-clash-protection -Werror=return-type -flto=auto -g -fcommon -pipe -O2 -ffunction-sections -fdata-sections -fPIE -fstack-protector-all  -D_DEFAULT_SOURCE -D_XOPEN_SOURCE=600 -I/usr/include/ncursesw    -I/usr/include/luajit-5_1-2.1 -DLUA_COMPAT_5_1 -DLUA_COMPAT_5_2 -DLUA_COMPAT_5_3 -DLUA_COMPAT_ALL  -std=c99 -U_XOPEN_SOURCE -D_XOPEN_SOURCE=700 -DNDEBUG -MMD -DVERSION=\"v0.8-git\" -DHAVE_MEMRCHR=1 -DVIS_PATH=\"/usr/share/vis\" -DCONFIG_HELP=1 -DCONFIG_CURSES=1 -DCONFIG_LUA=1 -DCONFIG_LPEG=0 -DCONFIG_TRE=1 -DCONFIG_SELINUX=1 -DCONFIG_ACL=1 -U_FORTIFY_SOURCE -UNDEBUG -O0 -g3 -ggdb -Wall -Wextra -pedantic -Wno-missing-field-initializers -Wno-unused-parameter -o obj/vis-lua.o -c vis-lua.c
[   15s] vis-lua.c: In function ‘obj_new’:
[   15s] vis-lua.c:350:9: warning: implicit declaration of function ‘lua_setuservalue’; did you mean ‘lua_setupvalue’? [-Wimplicit-function-declaration]
[   15s]   350 |         lua_setuservalue(L, -2);
[   15s]       |         ^~~~~~~~~~~~~~~~
[   15s]       |         lua_setupvalue
[   15s] vis-lua.c: In function ‘index_common’:
[   15s] vis-lua.c:483:17: warning: implicit declaration of function ‘lua_getuservalue’; did you mean ‘lua_getupvalue’? [-Wimplicit-function-declaration]
[   15s]   483 |                 lua_getuservalue(L, 1);
[   15s]       |                 ^~~~~~~~~~~~~~~~
[   15s]       |                 lua_getupvalue
[   15s] vis-lua.c: In function ‘getpos’:
[   15s] vis-lua.c:499:16: warning: implicit declaration of function ‘lua_tounsigned’ [-Wimplicit-function-declaration]
[   15s]   499 |         return lua_tounsigned(L, narg);
[   15s]       |                ^~~~~~~~~~~~~~
[   15s] vis-lua.c: In function ‘pushpos’:
[   15s] vis-lua.c:517:17: warning: implicit declaration of function ‘lua_pushunsigned’; did you mean ‘lua_pushinteger’? [-Wimplicit-function-declaration]
[   15s]   517 |                 lua_pushunsigned(L, pos);
[   15s]       |                 ^~~~~~~~~~~~~~~~
[   15s]       |                 lua_pushinteger
[   15s] vis-lua.c: In function ‘motion’:
[   15s] vis-lua.c:989:29: warning: implicit declaration of function ‘luaL_checkunsigned’; did you mean ‘luaL_checkinteger’? [-Wimplicit-function-declaration]
[   15s]   989 |         enum VisMotion id = luaL_checkunsigned(L, 2);
[   15s]       |                             ^~~~~~~~~~~~~~~~~~
[   15s]       |                             luaL_checkinteger
[   15s] vis-lua.c: In function ‘close_subprocess’:
[   15s] vis-lua.c:1435:9: error: unknown type name ‘luaL_Stream’
[   15s]  1435 |         luaL_Stream *file = luaL_checkudata(L, -1, "FILE*");
[   15s]       |         ^~~~~~~~~~~
[   15s] vis-lua.c:1436:33: error: request for member ‘f’ in something not a structure or union
[   15s]  1436 |         int result = fclose(file->f);
[   15s]       |                                 ^~
[   15s] vis-lua.c:1438:21: error: request for member ‘f’ in something not a structure or union
[   15s]  1438 |                 file->f = NULL;
[   15s]       |                     ^~
[   15s] vis-lua.c:1439:21: error: request for member ‘closef’ in something not a structure or union
[   15s]  1439 |                 file->closef = NULL;
[   15s]       |                     ^~
[   15s] vis-lua.c: In function ‘communicate_func’:
[   15s] vis-lua.c:1461:17: error: unknown type name ‘luaL_Stream’
[   15s]  1461 |                 luaL_Stream stream;
[   15s]       |                 ^~~~~~~~~~~
[   15s] vis-lua.c:1470:85: error: request for member ‘closef’ in something not a structure or union
[   15s]  1470 |         inputfd->handler = vis_process_communicate(vis, name, cmd, &(inputfd->stream.closef));
[   15s]       |                                                                                     ^
[   15s] vis-lua.c:1472:32: error: request for member ‘f’ in something not a structure or union
[   15s]  1472 |                 inputfd->stream.f = fdopen(inputfd->handler->inpfd, "w");
[   15s]       |                                ^
[   15s] vis-lua.c:1473:32: error: request for member ‘closef’ in something not a structure or union
[   15s]  1473 |                 inputfd->stream.closef = &close_subprocess;
[   15s]       |                                ^
[   15s] vis-lua.c:1475:31: error: request for member ‘f’ in something not a structure or union
[   15s]  1475 |         return inputfd->stream.f ? 1 : luaL_fileresult(L, 0, name);
[   15s]       |                               ^
[   15s] vis-lua.c: In function ‘file_lines_iterator’:
[   15s] vis-lua.c:2717:23: warning: implicit declaration of function ‘luaL_optunsigned’; did you mean ‘luaL_optinteger’? [-Wimplicit-function-declaration]
[   15s]  2717 |         size_t line = luaL_optunsigned(L, 2, 1);
[   15s]       |                       ^~~~~~~~~~~~~~~~
[   15s]       |                       luaL_optinteger
[   15s] vis-lua.c: In function ‘communicate_func’:
[   15s] vis-lua.c:1476:1: error: control reaches end of non-void function [-Werror=return-type]
[   15s]  1476 | }
[   15s]       | ^
[   15s] cc1: some warnings being treated as errors

Complete build log

@rnpnr
Copy link
Collaborator

rnpnr commented Mar 26, 2024

Yes as I said above vis is using functions that are lua5.2+. Someone would need to replace those with the equivalent from older lua. For the integer functions its very likely that lua_integer is sufficient but it would need to be checked.

@efjimm
Copy link

efjimm commented May 27, 2024

I took a quick look at this. Was able to get vis compiling and running fine with LuaJIT after a few modifications. Here's what needs to change in the C code:

  1. Replace setuservalue/getuservalue with getfenv/setfenv respectively
  2. Use lua_Integer instead of lua_Unsigned. lua_Integer is a ptrdiff_t, compared to lua_Unsigned which is unsigned long. For most platforms this is going to half the integer range we can use. This doesn't matter on 64 bit targets, but on 32 bit targets essentially caps the file size at 2 GB rather than 4 GB. Since we use size_t everywhere we might want to add checks to make sure we don't exceed PTRDIFF_MAX.
  3. Rewrite the communicate function to not use luaL_Stream, as it does not exist in Lua 5.1. It's not possible to create file handles from C in LuaJIT in any way.
  4. Change lua_newstate to luaL_newstate and remove the parameters. LuaJIT doesn't support custom allocators on 64 bit targets by default, but the allocator we were using is just a wrapper around realloc, the same as the default Lua allocator.
  5. Replace LUA_OK with 0. LuaJIT supports this as an extension, but it is not a part of Lua 5.1.
  6. Replace luaL_setfuncs with luaL_register. LuaJIT supports setfuncs as an extension, but it is not a part of Lua 5.1.
  7. Remove the call to luaL_traceback. LuaJIT supports this as an extension, but it is not a part of Lua 5.1.

Only 1-4 are required to support LuaJIT. One way around 3 would be to return our own userdata type that wraps a FILE* and exposes the same functionality as a Lua file handle.

I haven't looked at the implications for Lua code yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants