Repository navigation
Add optional FFI_MAX_CLOSURE_ARGS to specification? #703
Description
It would be helpful if libffi add an extra optional constant FFI_MAX_CLOSURE_ARGS to the specification. Implementations that are limited in the number of arguments they can use in closures could use this. For instance, on my libffi Emscripten port I can define in ffitarget.h
#define FFI_MAX_CLOSURE_ARGS 122This could then be consumed by ctypes: they currently have:
#define CTYPES_MAX_ARGCOUNT 1024https://github.com/python/cpython/blob/main/Modules/_ctypes/ctypes.h
but it would be nice to replace this with:
#ifdef FFI_MAX_CLOSURE_ARGS
#define CTYPES_MAX_ARGCOUNT FFI_MAX_CLOSURE_ARGS
#else
#define CTYPES_MAX_ARGCOUNT 1024
#endifand then we could fix Python's ctypes test test_callback_too_many_args in Emscripten, which tries to call a function with CTYPES_MAX_ARGCOUNT aguments.
Activity
[-]Add optional FFI_MAX_CLOSURE_ARGS to interface[/-][+]Add optional FFI_MAX_CLOSURE_ARGS to specification?[/+]atgreen commentedon Sep 20, 2022
Why does your emscripten port only support 122 args? I think this is a fine proposal. Will you be contributing an emscripten port?
hoodmane commentedon Sep 20, 2022
It was due to a bug in Emscripten:
emscripten-core/emscripten#16658
It supports up to 1000 arguments now. The wasm vms in common browser engines and node have a hard cap that wasm functions can never take more than 1000 arguments.
Yeah I would like to do so if you are interested in accepting one.
hoodmane commentedon Nov 4, 2022
So my current blocker in upstreaming this is that I don't know how to merge master into https://github.com/hoodmane/libffi-emscripten. There are a bunch of merge conflicts in
config.guessand I don't understand the gnu tools well enough to figure out how to fix them. I don't think this task is actually very hard for someone who understands how these files work. Maybe @atgreen or @tiran or @kleisauke could help?kleisauke commentedon Nov 5, 2022
IIUC,
config.guessis auto-generated when runningautogen.shorautoreconf. These files usually differ significantly between Autoconf versions.I recently made a change in wasm-vips to track the required patches in forks, so perhaps this comparison will help:
master...kleisauke:wasm-vips
(but I'm not sure if that includes all test suite changes)
hoodmane commentedon Nov 5, 2022
So does that mean I should merge
configure.hostand then rerun autoconf or something?kleisauke commentedon Nov 6, 2022
config.guessis used to determine the host system (--build) on which you run theconfigurescript. It's usually updated when a new CPU architecture is supported by libffi, last time it was updated for theloongarch64architecture for example. So for WebAssembly, it doesn't make much sense to update that (i.e. you can safely omit any changes to that), since that's more like a target platform (--host). I'm not aware of anyone that ported GNU Autotools to Wasm.config.subis used to canonicalize the--build,--hostand--targetoptions. AFAIK, this is usually not necessary when compiling for Wasm, since one could just use--host=wasm32-unknown-linuxor even--host=wasm32on the configure line to flip things into a mode for cross-compiling. After this commit, one could also use thewasm32-unknown-emscriptentarget triplet. However, some distributedconfig.subfiles are outdated, and some Autotools projects are not supporting$host_os == emscripten(see e.g. this SO answer), so I recommend using the former target triplets.configure.hostis libffi specific and is used to determine if the specified--hostis supported and if so, which files are needed to compile.