Skip to content

Conversation

egao1980
Copy link

@egao1980 egao1980 commented Mar 7, 2021

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.

@egao1980
Copy link
Author

egao1980 commented Mar 7, 2021

Fixes #811 and #812

@wsfulton
Copy link
Member

wsfulton commented Mar 8, 2021

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.

@egao1980
Copy link
Author

egao1980 commented Mar 9, 2021

Hi, thanks for the suggestion. I'll do that.

@egao1980
Copy link
Author

egao1980 commented May 8, 2021

QQ - CFFI is a library and not a compiler there's no "compiler" command. I'll try to hack something around it.

@ojwb
Copy link
Member

ojwb commented May 15, 2021

@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.

@egao1980
Copy link
Author

egao1980 commented May 15, 2021

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.

@ojwb ojwb added the CFFI label Dec 27, 2021
@ojwb
Copy link
Member

ojwb commented Dec 27, 2021

@egao1980 Did you get any further with this?

@egao1980
Copy link
Author

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 :)

@AeroNotix
Copy link

AeroNotix commented Feb 7, 2022

@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.

@AeroNotix
Copy link

@egao1980 can you push up all your latest changes even if they are broken / failing tests? I'd like to hack on this.

@egao1980
Copy link
Author

egao1980 commented Feb 7, 2022

Please see #2200

@egao1980
Copy link
Author

egao1980 commented Feb 7, 2022

I've created CI builds for (most) platforms.

@egao1980
Copy link
Author

egao1980 commented Feb 7, 2022

Current limitation is that the updated CFFI code requires cffi/cffi#154 to work with enum field references and constant arithmetic.

@egao1980
Copy link
Author

egao1980 commented Feb 7, 2022

That PR went through initial review - probably needs some extra attention.

wsfulton added a commit that referenced this pull request Oct 6, 2022
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.
@ojwb
Copy link
Member

ojwb commented Jun 24, 2023

Looks like you need to update for the changes from #2223 (SWIG_exit should now be Exit for example).

@egao1980 egao1980 marked this pull request as draft June 30, 2023 17:53
@@ -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);
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants