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

Lua 5.4 #48

Open
daurnimator opened this issue Jul 5, 2020 · 11 comments
Open

Lua 5.4 #48

daurnimator opened this issue Jul 5, 2020 · 11 comments

Comments

@daurnimator
Copy link
Contributor

daurnimator commented Jul 5, 2020

How should compat-5.3 be updated for Lua 5.4?

At least to allow projects to compile against 5.4 we need something like:

diff --git a/vendor/compat53/c-api/compat-5.3.h b/vendor/compat53/c-api/compat-5.3.h
index 8e10893..fb6cc25 100644
--- a/vendor/compat53/c-api/compat-5.3.h
+++ b/vendor/compat53/c-api/compat-5.3.h
@@ -397,11 +397,11 @@ COMPAT53_API void luaL_requiref (lua_State *L, const char *modname,


 /* other Lua versions */
-#if !defined(LUA_VERSION_NUM) || LUA_VERSION_NUM < 501 || LUA_VERSION_NUM > 503
+#if !defined(LUA_VERSION_NUM) || LUA_VERSION_NUM < 501 || LUA_VERSION_NUM > 504

-#  error "unsupported Lua version (i.e. not Lua 5.1, 5.2, or 5.3)"
+#  error "unsupported Lua version (i.e. not Lua 5.1, 5.2, 5.3, or 5.4)"

-#endif /* other Lua versions except 5.1, 5.2, and 5.3 */
+#endif /* other Lua versions except 5.1, 5.2, 5.3, and 5.4 */

However, maybe we should be deprecating the compat-5.3 project and creating a compat-5.4?

@siffiejoe
Copy link
Contributor

I haven't had a closer look at the API changes, yet, but my first impression is that we can't do much about 5.4. I've seen:

  • lua_version has changed
  • lua_resume has an extra parameter
  • we could try to emulate multiple uservalues (at some performance cost)
  • lua_pushfail is trivial
  • maybe something about the new warning system
  • luaL_addgsub
  • luaL_argexpected
  • luaL_buffaddr
  • luaL_buflen
  • luaL_bufsub
  • luaL_tolstring is slightly stricter
  • luaL_typeerror

For now, the newly released version should work for Lua 5.4.

@alerque
Copy link
Member

alerque commented Jul 8, 2020

Thanks for allowing this to run on Lua 5.4 systems. Can we get release on Luarocks too? Pretty please with a cherry?

@alerque
Copy link
Member

alerque commented Jul 8, 2020

Aaha I see you did a Luarocks release for compat53, but can we get a matching one for bit32?

@siffiejoe
Copy link
Contributor

I've added the bit32 library to this repository. I hope it works as intended ...

@alerque
Copy link
Member

alerque commented Jul 9, 2020

Not really, but it's complicated to test because of this luarocks issue.

Even when that is sorted out though I think there is going to be a problem with the bit32 module itself. Lua 5.3 by default included this this, so the bit32 load being a noop was sort of correct. But it didn't include it if you disabled the Lua 5.2 compatibility flag. Lua 5.4 doesn't include it at all.

In other words for code to be compatible with Lua 5.2, the bit32 module needs to stop being a noop and start actually providing something for 5.1 and 5.4, plus check 5.3 for whether the module exists already.

@gvvaughan
Copy link

gvvaughan commented Jul 10, 2020 via email

@daurnimator
Copy link
Contributor Author

use this: luaposix/luaposix@5d5e06e

Be very careful with such code; different bit libraries (and the operators) all behave differently in regards to signedness, overflow and other properties. See https://github.com/daurnimator/lua-http/blob/47225d081318e65d5d832e2dd99ff0880d56b5c6/http/bit.lua#L3-L7

@siffiejoe
Copy link
Contributor

@alerque I'm not sure I can follow. The bit32 backport in this repo compiles and works (according to my very primitive tests) for Lua 5.1 and Lua 5.4. The module should be unnecessary for Lua 5.2 and Lua 5.3 because those versions contain a native version of bit32, and LuaRocks will favor this native version automatically.

@alerque
Copy link
Member

alerque commented Jul 10, 2020

and LuaRocks will favor this native version automatically.

That's what should happen, but it isn't what Luarocks 3.3.1 actually does. Even when Lua 5.3 is compiled without 5.2 compatibly flags it assumes that it has a bit32 module internally and refuses to install the rock on the grounds that it's provided by the VM ... even though it hasn't actually checked that is the case.

@siffiejoe
Copy link
Contributor

Lua 5.3 without compatibility does provide a builtin bit32 module -- it just throws an error when loaded. Even if LuaRocks installed the rock, Lua 5.3 would still use the builtin, error-throwing one. Also, bit32 is a compatibility library, so it makes sense to assume that you either built with compatibility (which is the default) and you already have it, or you explicitly disabled compatibility and thus you don't want the non-error-throwing one.

@hishamhm
Copy link
Member

hishamhm commented Jan 2, 2021

However, maybe we should be deprecating the compat-5.3 project and creating a compat-5.4?

Makes sense to me!

I haven't had a closer look at the API changes, yet, but my first impression is that we can't do much about 5.4.

Hopefully the changes are not too drastic, then?

we could try to emulate multiple uservalues (at some performance cost)

What do you have in mind? I guess a weak-keys table in the registry mapping userdata to arrays would do the trick?

maybe something about the new warning system

Even something super basic like this would be nice as a start:

do
   local warnings_on = false
   function warn(msg, ...)
      if msg == "@on" and select("#", ...) == 0 then
         warnings_on = true
      elseif msg == "@off" and select("#", ...) == 0 then
         warnings_on = false
      elseif warnings_on then
         io.stderr:write("Lua warning: ", msg, ...)
         io.stderr:write("\n")
      end
   end
end

...though of course it could get fancier if it was to support the C API side of it as well. For user-produced warnings looks that would be doable. I don't think a library author cares about the Lua-generated warnings added in 5.4; those are the concern of whoever is embedding Lua, and if they care about warnings for these failure conditions they'll be probably embedding 5.4 anyway.

For now, the newly released version should work for Lua 5.4.

This is great, thank you! :)

Looking at the library incompatibilities list for the Lua standard library side of things, I think one possible change in compat-5.4 would be to pick the new lutf8lib.c which adds the lax argument.

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

No branches or pull requests

5 participants