q66 / cffi-lua

A portable C FFI for Lua 5.1+
MIT License
176 stars 24 forks source link

MetaType __len not working correctly #31

Closed mavriq-dev closed 2 years ago

mavriq-dev commented 2 years ago

I'm trying to duplicate the following code from luajit ffi

local cffi = require("cffi")
cffi.cdef[[
typedef struct { double x, y; } point_t;
]]

local point
local mt = {
  __add = function(a, b) return point(a.x+b.x, a.y+b.y) end,
  __len = function(a) return math.sqrt(a.x*a.x + a.y*a.y) end,
  __index = {
    area = function(a) return a.x*a.x + a.y*a.y end,
  },
}
point = cffi.metatype("point_t", mt)

local a = point(3, 4)
print(a.x, a.y)  --> 3  4
print(#a)        --> 5
print(a:area())  --> 25
local b = a + point(0.5, 8)
print(#b)        --> 12.5

Everything works with the exception of __len. when the code reaches print(#a) I get the error 'struct 0' is not callable.

__add and __index both work fine if I remove the code using __len Tried on macos and Windows.

mavriq-dev commented 2 years ago

I should also mention this is Lua 5.3, both stand alone an embedded in an application (Reaper).

mavriq-dev commented 2 years ago

I also tried the code from lua-cffi documentation that is similar:

local cffi = require("cffi")
cffi.cdef [[
    typedef struct point_t { double x; double y; } point_t;
]]

local point
point = cffi.metatype("point_t", {
    __add = function(a, b)
        return point(a.x + b.x, a.y + b.y)
    end,
    __len = function(a)
        return math.sqrt(a:area())
    end,
    __index = {
        area = function(a)
            return a.x * a.x + a.y * a.y
        end
    }
})

local pt = point(3, 4)
print(pt.x, pt.y) -- 3, 4
print(#pt) -- 5
print(pt:area()) -- 25
local pt2 = a + point(0.5, 8)
print(#pt2) -- 12.5

with simliar results 'struct point_t' is not callable

q66 commented 2 years ago

i will look into it later and add a testcase, seems like a bug

but I'm not home until end of the upcoming week, so it'll have to wait a little

mavriq-dev commented 2 years ago

Sounds good. Much appreciated. This is an invaluable resource.

FYI, This affects latest version, 0.2.1 and 0.2.0. I built and tested them all. So it must have been around for a while.

q66 commented 2 years ago

it's probably been there from the beginning, it's just surprising there is not a test case for it, since the test suite has grown relatively extensive (and largely on par with luajit's own ffi tests)

mavriq-dev commented 2 years ago

Ya, I did look in the tests to see if I was missing something. I noticed there are relatively few metamethod tests given the number that Lua now supports. The other tests are quite extensive.

q66 commented 2 years ago

most of the metamethods should not need testing as they are implemented identically to others (the code paths are the same, as they are generated via a macro)

unary metamethods are a bit special though

q66 commented 2 years ago

this should now be fixed in a way that i'm happy with, feel free to give it a test if you like (i'll cut a release during next week)

mavriq-dev commented 2 years ago

Thanks. Your work is greatly appreciated. I'll give it a shot shortly.

Incidentally, I'm trying to build libffi on mac either cross compiling to arm64 or a universal build. I can't seem to figure out how. I've tried many things with configure and a merged a pull request that adds cmake. It always builds x86_64 it seems. and macports is actually broken. Their universal lib won't link with arm64. Lipo says it is universal but the LD complains that it is unknown_x86_64.

Any ideas?

On Sun, Jun 19, 2022 at 10:31 AM q66 @.***> wrote:

this should now be fixed in a way that i'm happy with, feel free to give it a test if you like (i'll cut a release during next week)

— Reply to this email directly, view it on GitHub https://github.com/q66/cffi-lua/issues/31#issuecomment-1159741875, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJYH27W6IJ5EOQ5ETJIDPTVP4VMZANCNFSM5ZEAEZSQ . You are receiving this because you authored the thread.Message ID: @.***>

-- Geoff Van Brunt

q66 commented 2 years ago

never tried, i always installed libffi from homebrew when testing on macos, but never tested arm64 (should probably be the same?)

but libffi has no dependencies, so as long as you have the xcode commandline tools installed, i don't see why the usual configure+make+make install method wouldn't work?

mavriq-dev commented 2 years ago

It's the cross compile part that doesn't work. Doesn't seem to produce arm64 libs no matter what settings i've tried. libffi also doesn't currently build on M1(lots of open issues about it), so you have to cross compile.

Apple has an upstream fork of libffi and has working arm binaries apparently. I might have a peek at that and see if there is anything there. Not sure if cffi will work with that fork.

On Sun, Jun 19, 2022 at 11:51 AM q66 @.***> wrote:

never tried, i always installed libffi from homebrew when testing on macos, but never tested arm64 (should probably be the same?)

but libffi has no dependencies, so as long as you have the xcode commandline tools installed, i don't see why the usual configure+make+make install method wouldn't work?

— Reply to this email directly, view it on GitHub https://github.com/q66/cffi-lua/issues/31#issuecomment-1159762131, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJYH24KHVNXYWC65BPCXYTVP46YLANCNFSM5ZEAEZSQ . You are receiving this because you authored the thread.Message ID: @.***>

-- Geoff Van Brunt

q66 commented 2 years ago

as long as the fork does not break API (at least those that cffi uses), it will work (cffi does not make any non-portable assumptions about the target or the library)

never tried cross-compilation on macos; the generic approach with configure scripts is to pass --host with the correct triplet (not --target) and possibly set CC (and possibly some other tool vars, like AR) to the right cross-compiler (you might also want --disable-shared and static-link libffi into the module)

mavriq-dev commented 2 years ago

Think I got it. You need to compile arm64 and x86_64 separately. Then lipo them together.

To cross compile use:

env CFLAGS="-arch arm64" LDFLAGS="-arch arm64" ./configure --host=aarch64-apple-darwin20

Don't have an M1 to test on, but 'nm -g' and lipo seem to indicate a correct file. Have to check if all exported symbols available on arm64, I'll do that next...

mavriq-dev commented 2 years ago

So no luck with using a cross compile file, but then again not really familiar with meson.

With clang all you have to do is specify '-arch arm64' and it cross compiles. so in the ninja build file I did this:

[image: image.png]

Adding to the beginning of LINK_ARGS in the same file and it will build an arm64 build. Is there a way to pass that to meson on the command line so It can be build without having to modify the ninja build file?

On Sun, Jun 19, 2022 at 1:46 PM Geoff Van Brunt @.***> wrote:

Still having an issue with the arm64 version when trying to cross compile cffi using:

CFLAGS="-arch arm64 " LDFLAGS="-arch arm64 " meson .. -Dlua_version=5.3 -Dlibffi=vendor

I get:

../meson.build:3:0: ERROR: Could not invoke sanity test executable: [Errno 86] Bad CPU type in executable: '/Users/geoffvanbrunt/Downloads/cffi-lua/build/meson-private/sanitycheckcpp.exe'.

Not familiar with meson. Any ideas as to what is the issue?

Thanks

On Sun, Jun 19, 2022 at 1:22 PM Geoff Van Brunt @.***> wrote:

Think I got it. You need to compile arm64 and x86_64 separately. Then lipo them together.

To cross compile use:

env CFLAGS="-arch arm64" LDFLAGS="-arch arm64" ./configure --host=aarch64-apple-darwin20

Don't have an M1 to test on, but 'nm -g' and lipo seem to indicate a correct file. Have to check if all exported symbols available on arm64, I'll do that next...

-- Geoff Van Brunt

-- Geoff Van Brunt

mavriq-dev commented 2 years ago

Now that I have a working file, I tested and indeed len works just fine with your two most recent commits.

On Sun, Jun 19, 2022 at 10:01 PM Geoff Van Brunt @.***> wrote:

So no luck with using a cross compile file, but then again not really familiar with meson.

With clang all you have to do is specify '-arch arm64' and it cross compiles. so in the ninja build file I did this:

[image: image.png]

Adding to the beginning of LINK_ARGS in the same file and it will build an arm64 build. Is there a way to pass that to meson on the command line so It can be build without having to modify the ninja build file?

On Sun, Jun 19, 2022 at 1:46 PM Geoff Van Brunt @.***> wrote:

Still having an issue with the arm64 version when trying to cross compile cffi using:

CFLAGS="-arch arm64 " LDFLAGS="-arch arm64 " meson .. -Dlua_version=5.3 -Dlibffi=vendor

I get:

../meson.build:3:0: ERROR: Could not invoke sanity test executable: [Errno 86] Bad CPU type in executable: '/Users/geoffvanbrunt/Downloads/cffi-lua/build/meson-private/sanitycheckcpp.exe'.

Not familiar with meson. Any ideas as to what is the issue?

Thanks

On Sun, Jun 19, 2022 at 1:22 PM Geoff Van Brunt @.***> wrote:

Think I got it. You need to compile arm64 and x86_64 separately. Then lipo them together.

To cross compile use:

env CFLAGS="-arch arm64" LDFLAGS="-arch arm64" ./configure --host=aarch64-apple-darwin20

Don't have an M1 to test on, but 'nm -g' and lipo seem to indicate a correct file. Have to check if all exported symbols available on arm64, I'll do that next...

-- Geoff Van Brunt

-- Geoff Van Brunt

-- Geoff Van Brunt

mavriq-dev commented 1 year ago

Still having an issue with the arm64 version when trying to cross compile cffi using:

CFLAGS="-arch arm64 " LDFLAGS="-arch arm64 " meson .. -Dlua_version=5.3 -Dlibffi=vendor

I get:

../meson.build:3:0: ERROR: Could not invoke sanity test executable: [Errno 86] Bad CPU type in executable: '/Users/geoffvanbrunt/Downloads/cffi-lua/build/meson-private/sanitycheckcpp.exe'.

Not familiar with meson. Any ideas as to what is the issue?

Thanks

On Sun, Jun 19, 2022 at 1:22 PM Geoff Van Brunt @.***> wrote:

Think I got it. You need to compile arm64 and x86_64 separately. Then lipo them together.

To cross compile use:

env CFLAGS="-arch arm64" LDFLAGS="-arch arm64" ./configure --host=aarch64-apple-darwin20

Don't have an M1 to test on, but 'nm -g' and lipo seem to indicate a correct file. Have to check if all exported symbols available on arm64, I'll do that next...

-- Geoff Van Brunt