-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Revive CFFI Common Lisp support #1966
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
base: master
Are you sure you want to change the base?
Conversation
Hi @egao1980 that's great. I presume you've found the prerequisites for adding a new language module to SWIG at http://swig.org/Doc4.0/Extending.html#Extending_prerequisites. There are quite a few steps and one glaring omission is "The examples and test-suite must also be fully functioning on the Travis Continuous Integration platform." Please use your own Travis account and repo to get this working where you can temporarily enable just CFFI as pushing a patch here uses up a lot of time consuming resources on Travis. I suggest posting a link to your development branch on your egao1980 Github repo for ongoing feedback until you have the test-suite passing. |
Hi, thanks for the suggestion. I'll do that. |
QQ - CFFI is a library and not a compiler there's no "compiler" command. I'll try to hack something around it. |
@egao1980 Presumably it's possible to write CLISP that exercises the generated wrappers though, similar to how someone would actually use SWIG-generated CFFI wrappers? Or taking a step back, what the CI is there for is to ensure the generated bindings work (and so demonstrates that a target language has a decent level of supported functionality), and also that they continue to work after future changes to SWIG - having comprehensive CI really helps us avoid introducing breakage into releases. So what we're after here is something that fails if someone changes the CFFI backend or something it relies on such that generated wrappers don't work as they are meant to. |
Yep, I've managed to create a working CI configuration. I have a script that can compile SWIG wrappers and run them. Working on a few *_run.lisp examples. I'll update the PR as soon as those changes are ready. Please note that CFFI only supports C so C++ tests are currently disabled. I might attempt C++ support but as a separate stream of work. |
@egao1980 Did you get any further with this? |
Last time I've checked I had it running in CI. I'll get back to this one in a few days. It's dormant but alive :) |
@egao1980 if you could write up a list of things which are/are not supported, or otherwise need attention it would help. I am also willing to help maintain the cffi backend. |
@egao1980 can you push up all your latest changes even if they are broken / failing tests? I'd like to hack on this. |
Please see #2200 |
I've created CI builds for (most) platforms. |
Current limitation is that the updated CFFI code requires cffi/cffi#154 to work with enum field references and constant arithmetic. |
That PR went through initial review - probably needs some extra attention. |
No meaningful progress to update CFFI to experimental status has been made since CFFI was disabled in SWIG-4.0.0 as the first stage to removal. This commit is the final stage to remove it. See issue #1966 for an attempt at updating CFFI to experimental status. Anyone wishing for SWIG to support CFFI again might want to utilise this work.
Looks like you need to update for the changes from #2223 ( |
@@ -134,15 +134,15 @@ int CFFI::top(Node *n) { | |||
File *f_lisp = NewFile(lisp_filename, "w", SWIG_output_files()); | |||
if (!f_lisp) { | |||
FileErrorDisplay(lisp_filename); | |||
SWIG_exit(EXIT_FAILURE); | |||
exit(EXIT_FAILURE); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be Exit(EXIT_FAILURE);
now (NB capitalisation exactly like this) - this provides an exit hooking mechanism as part of the DOH layer so we can remove any output files SWIG has started to create if it exits due to an error, which only happened in some cases before with SWIG_exit()
- see 55377bd if you want the gory details.
Hi team,
I'm happy to present an updated CFFI module.
I've also prepared the corresponding PR to CFFI project, see cffi/cffi#154
I'm ready to maintain this module and fix issues for at least a year.