qtiuto / lua-for-android

A high-performance bridge for lua and java or c in Android with mutil-thread and almost all java features supported
BSD 3-Clause "New" or "Revised" License
111 stars 26 forks source link

Now there is an error with meta methods #7

Open mingodad opened 5 years ago

mingodad commented 5 years ago

Hello ! Thanks for fix the issues reported before. But now with the changes you've made to the meta methods it compiles fine when applied to the original lua-ffi but when try to test it "lua test.lua" we get this error:

ljs: test.ljs:685: type struct vls has no member key
stack traceback:
    [C]: in metamethod '__index'
    test.ljs:685: in main chunk
    [C]: in ?

Note I'm using it with https://github.com/mingodad/ljs but otherwise it's the same in Lua. And it was working before the latest changes.

Cheers !

mingodad commented 5 years ago

Here is the code that fail:

ffi.cdef ([=[
struct vls {
    struct {
        char a;
        struct {
            char b;
            char v[?];
        } c;
    } d;
};
struct vls2 {
    char pad;
    union {
        uint8_t a;
        uint16_t b;
    };
};
]=]);

assert(ffi.sizeof('struct vls', 3) == 5);
assert(ffi.sizeof(ffi.new('struct vls', 4).d.c) == 5);
assert(ffi.offsetof('struct vls2', 'a') == 2);
assert(ffi.sizeof('struct vls2') == 4);

ffi.cdef ([=[ static const int DUMMY = 8 << 2; ]=]);
assert(ffi.C.DUMMY == 32);

ffi.new('struct {const char* foo;}', {'foo'});

assert(! pcall(function() {
    ffi.new('struct {char* foo;}', {'ff'});
}));

var mt = {};
var vls = ffi.new(ffi.metatype('struct vls', mt), 1);

assert(! pcall(function() { return vls.key; }));

mt.__index = function(vls, key) {
    return function(vls, a, b) {
        return 'in index ' .. key .. ' ' .. vls.d.a .. ' ' .. a .. ' ' .. b;
    };
};

vls.d.a = 3;
check(vls->key('a', 'b'), 'in index key 3 a b');  //  here it fail: ljs: test.ljs:685: type struct vls has no member key
qtiuto commented 5 years ago

For my optimzation, I assume that the metatable should not be changed after set by ffi.metatype. If you change the metatable, reset it by ffi.metatype. For the lua state, I do the similar optimzation. Howerver, I can monitor the change for table in the lua State while I can not do that in the ffi library. Moreover, I'm sorry that the error for the io.tmpfile is not induced by the incompatiblity but the limitation enforced by the android platform. I have fixed the problem by overriding the c library function tmpfile by something like File.createTempFile in java.

Domingo Alvarez Duarte notifications@github.com 于 2019年1月16日周三 16:59写道:

Here is the code that fail:

ffi.cdef ([=[ struct vls { struct { char a; struct { char b; char v[?]; } c; } d; }; struct vls2 { char pad; union { uint8_t a; uint16_t b; }; }; ]=]);

assert(ffi.sizeof('struct vls', 3) == 5); assert(ffi.sizeof(ffi.new('struct vls', 4).d.c) == 5); assert(ffi.offsetof('struct vls2', 'a') == 2); assert(ffi.sizeof('struct vls2') == 4);

ffi.cdef ([=[ static const int DUMMY = 8 << 2; ]=]); assert(ffi.C.DUMMY == 32);

ffi.new('struct {const char* foo;}', {'foo'});

assert(! pcall(function() { ffi.new('struct {char* foo;}', {'ff'}); }));

var mt = {}; var vls = ffi.new(ffi.metatype('struct vls', mt), 1);

assert(! pcall(function() { return vls.key; }));

mt.__index = function(vls, key) { return function(vls, a, b) { return 'in index ' .. key .. ' ' .. vls.d.a .. ' ' .. a .. ' ' .. b; }; };

vls.d.a = 3; check(vls->key('a', 'b'), 'in index key 3 a b'); // here it fail: ljs: test.ljs:685: type struct vls has no member key

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/qtiuto/lua-for-android/issues/7#issuecomment-454702239, or mute the thread https://github.com/notifications/unsubscribe-auth/AJHwUs0x8ZhTpR6ueMuoUBuFPIyb8G5Pks5vDunzgaJpZM4aCl4Z .

qtiuto commented 5 years ago

@mingodad If don't like the feature, just remove it. I think it's kind of easy for you.

mingodad commented 5 years ago

I'm testing the meta method optimisation on ubuntu x64 but I can not get it to work and it segfaults with this sample:

local ffi = require("ffi");

ffi.cdef [[
struct vls {
    struct {
        char a;
        struct {
            char b;
            char v[?];
        } c;
    } d;
};
struct vls2 {
    char pad;
    union {
        uint8_t a;
        uint16_t b;
    };
};
]]

assert(ffi.sizeof('struct vls', 3) == 5)
assert(ffi.sizeof(ffi.new('struct vls', 4).d.c) == 5)
assert(ffi.offsetof('struct vls2', 'a') == 2)
assert(ffi.sizeof('struct vls2') == 4)

local mt = {}

local vls = ffi.new(ffi.metatype('struct vls', mt), 1)

Cheers !

qtiuto commented 5 years ago

Now it should work fine with lastest two commit.

Domingo Alvarez Duarte notifications@github.com 于 2019年1月17日周四 17:24写道:

I'm testing the meta method optimisation on ubuntu x64 but I can not get it to work and it segfaults with this sample:

local ffi = require("ffi");

ffi.cdef [[ struct vls { struct { char a; struct { char b; char v[?]; } c; } d; }; struct vls2 { char pad; union { uint8_t a; uint16_t b; }; }; ]]

assert(ffi.sizeof('struct vls', 3) == 5) assert(ffi.sizeof(ffi.new('struct vls', 4).d.c) == 5) assert(ffi.offsetof('struct vls2', 'a') == 2) assert(ffi.sizeof('struct vls2') == 4)

local mt = {}

local vls = ffi.new(ffi.metatype('struct vls', mt), 1)

Cheers !

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/qtiuto/lua-for-android/issues/7#issuecomment-455101113, or mute the thread https://github.com/notifications/unsubscribe-auth/AJHwUrgCRgjovSQK_OH6Ld97tyauaebpks5vEEFEgaJpZM4aCl4Z .

mingodad commented 5 years ago

Does the sample code above works for you ?

qtiuto commented 5 years ago

After fix, it works. it's just a index bug. I forgot to decrease index after changing lua_rawget to lua_rawgeti. And I have fixed the way compiling the metatable to ensure it starts at 1 rather than zero.

Domingo Alvarez Duarte notifications@github.com 于 2019年1月17日周四 19:53写道:

Does the sample code above works to you ?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/qtiuto/lua-for-android/issues/7#issuecomment-455145760, or mute the thread https://github.com/notifications/unsubscribe-auth/AJHwUobs81ZkXgohm9u7gprUkME6wAynks5vEGQ5gaJpZM4aCl4Z .

mingodad commented 5 years ago

Hello ! I just did a pull request and tested again but still getting segfault.

mingodad commented 5 years ago

Here is the whole folder I'm testing just in case you can see/test too. luaffib-arm-full.zip

qtiuto commented 5 years ago

The small fix will work for you.I just forgot to handle empty metatable case which won't generate a valid compiled metatanle on the stack. Now, I push nil on the stack.

Domingo Alvarez Duarte notifications@github.com 于 2019年1月17日周四 20:08写道:

Here is the whole folder I'm testing just in case you can see/test too. luaffib-arm-full.zip https://github.com/qtiuto/lua-for-android/files/2768545/luaffib-arm-full.zip

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/qtiuto/lua-for-android/issues/7#issuecomment-455149980, or mute the thread https://github.com/notifications/unsubscribe-auth/AJHwUjHzLcjlM0AG3nDfAAgrC8X6pU6uks5vEGe9gaJpZM4aCl4Z .

mingodad commented 5 years ago

Hello ! Indeed with the last fix it do not segfault anymore, but when we try to run test.lua it doesn't go forward. I understand that you said that we need to reset the ffi.metatype and I did for the first occurrence but when I did for the next one it gives another error with meta method "__add".

Can you make it work with test.lua ? If so could you share what adjusts you did to make it work ? Cheers !

qtiuto commented 5 years ago

You can add the below to set the metatable again. ffi.metatype('struct vls', mt) Index for call is unfixed in the previous commit and I fix it in the later commit. In the laset commit I fix mode for arm64. Cheers!

Domingo Alvarez Duarte notifications@github.com 于 2019年1月17日周四 21:18写道:

Hello ! Indeed with the last fix it do not segfault anymore, but when we try to run test.lua it doesn't go forward. I understand that you said that we need to reset the ffi.metatype and I did for the first occurrence but when I did for the next one it gives another error with meta method "__add".

Can you make it work with test.lua ? If so could you share what adjusts you did to make it work ? Cheers !

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/qtiuto/lua-for-android/issues/7#issuecomment-455168937, or mute the thread https://github.com/notifications/unsubscribe-auth/AJHwUvY_aRUOtyo6E38TlnB2Zd8e0nClks5vEHgxgaJpZM4aCl4Z .

mingodad commented 5 years ago

Thank you for share it ! Doing so give now the following error :

test.lua 
Running test
lua: test.lua:734: only function callbacks are callable
stack traceback:
    [C]: in local 'vls'
    test.lua:734: in main chunk
    [C]: in ?

test.lua at that position:

check(vls <= u64(6), true)
check(-vls, -5)
local a,b = vls('6') -- ??? here is the error
check(a, '__call')

Are you testing the test.lua ?

Cheers !

qtiuto commented 5 years ago

Sorry, I just close my vpn then. The error is fixed now in the lastest two commit.

Domingo Alvarez Duarte notifications@github.com 于 2019年1月17日周四 22:13写道:

Thank you for share it ! Doing so give now the following error :

test.lua Running test lua: test.lua:734: only function callbacks are callable stack traceback: [C]: in local 'vls' test.lua:734: in main chunk [C]: in ?

test.lua at that position:

check(vls <= u64(6), true) check(-vls, -5) local a,b = vls('6') -- ??? here is the error check(a, '__call')

Are you testing the test.lua ?

Cheers !

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/qtiuto/lua-for-android/issues/7#issuecomment-455185638, or mute the thread https://github.com/notifications/unsubscribe-auth/AJHwUiOJMif4fEZGDZpCShyzPTHOJS8Yks5vEIUbgaJpZM4aCl4Z .