Skip to content
This repository has been archived by the owner on Jan 28, 2024. It is now read-only.

Support for ABI-specific integers #261

Merged
merged 20 commits into from
Jan 22, 2022

Conversation

mannprerak2
Copy link
Contributor

@mannprerak2 mannprerak2 commented Dec 26, 2021

Closes dart-lang/native#530.
This PR lays the groundwork for the generation of ABI-specific integers.

  • Added 2 new configs - library-imports and type-map
  • Removed config keys - size-map and typedef-map.
  • type-map can target native types, typedefs, structs and unions.
  • Update readme, changelog, version

@mannprerak2
Copy link
Contributor Author

@dcharkes This PR currently does not generate or use any ABI-specific integers by default but it's left for the user to do by themselves via the given config. I think it'd better to wait till package:ffi has defined these ABI-specific integers before ffigen uses them by default.

CHANGELOG.md Show resolved Hide resolved
@dcharkes
Copy link
Contributor

This PR currently does not generate or use any ABI-specific integers by default but it's left for the user to do by themselves via the given config.

When there's a Dart dev release with it, I can make a dev release for package:ffi.

@mannprerak2
Copy link
Contributor Author

When there's a Dart dev release with it, I can make a dev release for package:ffi.

I'm currently using the 2.16.0-dev.124 release which I guess supports Abi specific integers. So we can possibly hook up package:ffi in this PR if you want.

@dcharkes
Copy link
Contributor

dcharkes commented Jan 4, 2022

I'll make a pre-release once reviewed:

Copy link
Contributor

@dcharkes dcharkes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy new year! 🧨 🎉

LGTM with comments addressed.

I think we should only map int8_t etc to dart:ffi types and target typedefs or AbiSpecificIntegers in package:ffi for all other types (including char, int, etc.)

.github/workflows/test-package.yml Outdated Show resolved Hide resolved
CHANGELOG.md Show resolved Hide resolved
example/libclang-example/custom_import.dart Show resolved Hide resolved
example/libclang-example/generated_bindings.dart Outdated Show resolved Hide resolved
lib/src/code_generator/imports.dart Outdated Show resolved Hide resolved
lib/src/strings.dart Outdated Show resolved Hide resolved
pubspec.yaml Outdated Show resolved Hide resolved
test/example_tests/libclang_example_test.dart Outdated Show resolved Hide resolved
@mannprerak2
Copy link
Contributor Author

@dcharkes I've updated the code to use package:ffi everywhere, by using a git dependency. But this seems to have broken the tests on CI. Would update the PR to point to the dev version once it's released.

Copy link
Contributor

@dcharkes dcharkes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

I'll try to get the package:ffi PR landed soon as well :)

FYI: https://dart-review.googlesource.com/c/sdk/+/228541 We're adding the C types to dart:ffi in Dart 2.17 (too late for 2.16) so that we can use them in @FfiNative annotations (which are used in places where we cannot import package:ffi). I'll update package:ffi to just export the dart:ffi definitions when Dart 2.17 comes around.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
example/c_json/pubspec.yaml Outdated Show resolved Hide resolved
example/libclang-example/pubspec.yaml Outdated Show resolved Hide resolved
lib/src/code_generator/imports.dart Show resolved Hide resolved
test/large_integration_tests/large_test.dart Outdated Show resolved Hide resolved
test/large_integration_tests/large_test.dart Outdated Show resolved Hide resolved
@dcharkes
Copy link
Contributor

https://pub.dev/packages/ffi/versions/1.2.0-dev.0/changelog is here!

(Note I dropped ssize_t and off_t because they're only part of the posix extension not of C themselves. If we turn out to need these, we can make an extra PR to package:ffi to include some posix types.)

@dcharkes dcharkes merged commit 443fd70 into dart-archive:master Jan 22, 2022
@dcharkes
Copy link
Contributor

Thanks @mannprerak2! 🥇

https://pub.dev/packages/ffigen/versions/5.0.0-dev.0

@mannprerak2 mannprerak2 deleted the abi_specific_integers branch January 22, 2022 11:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Support size_t, int, and other common C types // platform independent bindings
2 participants