HaxeFoundation / haxe

Haxe - The Cross-Platform Toolkit
https://haxe.org
6.14k stars 656 forks source link

[lua] Requiring bit32 breaks LuaJIT #5954

Closed shakesoda closed 7 years ago

shakesoda commented 7 years ago

Hi, trying to fix some of my code for the Lua target that worked in 3.3, I found that at some point Lua 5.2's bit32 library is required. In LuaJIT this should be just bit (the libraries are compatible). Both can be handled either by detecting LuaJIT or with a pcall, something like:

local ok, bit = pcall(require, "bit") -- luajit
if not ok then
    ok, bit = pcall(require, "bit32") -- lua 5.2+
    if not ok then error("no bit library") end
end

edit: I see some other stuff in the build output already checks for jit just using a type check, should've noticed :)

nadako commented 7 years ago

This also breaks hxdefold, fix pl0x.

nadako commented 7 years ago

This is a regression from 3.3 :)

jdonaldson commented 7 years ago

There's some subtle differences between those libs... I'll need to do even more version checks to sort it all out. :(

jdonaldson commented 7 years ago

Curiously, my builds of luajit all come with bit32. I don't believe I specified any special compatibility mode there. E.g. the travis builds for luajit (installed via hererocks) all seem to be fine.

shakesoda commented 7 years ago

@jdonaldson is this affecting it? https://github.com/HaxeFoundation/haxe/blob/2fcd720fdb73cc7dbc7efd756fb972f1f1a1c261/tests/RunCi.hx#L475

if luarocks has already installed bit32 for lua 5.1, luajit could be picking that up.

jdonaldson commented 7 years ago

That's what I thought initially as well. However, I can verify that bit32 is not installed via luarocks, and is not available on the searchpath. (edit : whoops, no, now I see the problem).

jdonaldson commented 7 years ago

quick question : is it possible to get bit32 installed? The workaround means trying to load the "bit" library instead. However, that's got some problems with overflow. Int64 won't work right, for one.

nadako commented 7 years ago

i don't think e.g. a casual defold user would know how to install bit32 for defold (considering it's even available there)

shakesoda commented 7 years ago

int64 won't work correctly anyways prior to lua 5.3 without using FFI, as you can't accurately represent all 64-bit ints with a double.

jdonaldson commented 7 years ago

@shakesoda I'm talking about haxe.Int64, which is a special abstract type.

jdonaldson commented 7 years ago

I think I found an easy workaround for this. Luajit has a slightly different overflow behavior on that bit library. But, I think I can make a quick fix.

nadako commented 7 years ago

@jdonaldson Unfortunately the require('bit') still fails on Defold (so it's unusable), where there's just a global bit object available (using http://bitop.luajit.org/api.html). I wonder if it could be somehow configurable by a -D define or something...

Also, in that compilation, I actually never used bit operations, but it was included, probably because of lua.Thread was included, but I didn't use it as well. It seems that it was included because of __instanceof being included, which ultimately was included because of Std.string. This might become a bit of a problem for integrating Haxe/Lua as a scripting for game engines like Defold, so I think we should think about some solution.

jdonaldson commented 7 years ago

Ok, another fix is en route. I think this will get things working again.

In general, I don't have feature detection working for bit ops. It wasn't a super high priority. For now, it's just a polyfill.

Thanks for using the target, I appreciate the feedback and reports.