Skip to content
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

libffi: hide platform-ignorant FFI ABI enum values #1318

Merged
merged 3 commits into from
Feb 1, 2023

Conversation

drcicero
Copy link
Contributor

This change ignores processing the whole ffitarget.h file, which If i understood correctly contains platform-specific values, but only for one platform, and therefore is on most platforms wrong. I believe the platform-specific values can already be accessed native methods handwritten in the preset. I added the otherwise missing FFI_TRAMPOLINE_SIZE() method.

I think only two other variables remain in the ffitarget.h file, which are FFI_CLOSURES = 1; FFI_GO_CLOSURES = 1;. I'm not super sure, but i believe those are only there to enable/disable definitions in the other header, it shouldn't hurt not to have them.

I tried mvn clean test --projects .,libffi and it succeeds. I have not figured out if there are more tests to run?

See #1315

@drcicero
Copy link
Contributor Author

drcicero commented Jan 31, 2023

Oh, the example https://github.com/bytedeco/javacpp/wiki/Mapping-Recipes#skipping-lines-from-header-files sounded like one can only skip lines of the form

// START COMPLEX DECLARATIONS
// ...
// END COMPLEX DECLARATIONS

but by studying the other InfoMaps, I found it is actually possible to skip individual definitions as well.

I suppose a simpler change would be to add the following:

.put(new Info("FFI_FIRST_ABI","FFI_SYSV","FFI_THISCALL","FFI_FASTCALL","FFI_STDCALL","FFI_PASCAL","FFI_REGISTER","FFI_MS_CDECL","FFI_LAST_ABI","FFI_DEFAULT_ABI").skip())

@drcicero
Copy link
Contributor Author

drcicero commented Feb 1, 2023

Now, the change is only the small addition of a few skips (.put(new Info("FFI_FIRST_ABI", "FFI_SYSV", "FFI_THISCALL", "FFI_FASTCALL", "FFI_STDCALL", "FFI_PASCAL", "FFI_REGISTER", "FFI_MS_CDECL", "FFI_LAST_ABI", "FFI_DEFAULT_ABI").skip())), because those ones already are defined in the preset. And as this leads to having the correct FFI_DEFAULT_ABI definition autogenerated, I removed the manual definition of FFI_DEFAULT_ABI from the preset.

@saudet
Copy link
Member

saudet commented Feb 1, 2023

For consistency though, we might as well add a skip for FFI_DEFAULT_ABI too and leave it with the rest in the manually declared ones.

@saudet saudet merged commit c5c7d4b into bytedeco:master Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants