Skip to content

[jnigen] [ffigen] Unify Dart API #2062

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

Open
5 tasks
HosseinYousefi opened this issue Mar 4, 2025 · 3 comments
Open
5 tasks

[jnigen] [ffigen] Unify Dart API #2062

HosseinYousefi opened this issue Mar 4, 2025 · 3 comments

Comments

@HosseinYousefi
Copy link
Member

HosseinYousefi commented Mar 4, 2025

  • ffigen uses FfiGen().run(...). jnigen uses the top level generateJniBindings
  • ffigen has an Uri output. jnigen has a OutputConfig since it also has a multifile mode
  • ffigen has multiple functions like shouldExposeFunctionTypedefFunc, ... jnigen has a List<Visitor> that combines all such functions
  • ffigen and jnigen both use Uri, should we use Strings instead to specify paths? (related: Making build/link APIs use Strings to represent filepaths instead of Uris #1506)
  • ffigen has DeclarationFilters for excluding elements, jnigen has a List<Visitor> that combines all such filters

@liamappelbe, let's discuss the points above and other points that come up in this issue.

cc @dcharkes

@dcharkes
Copy link
Collaborator

dcharkes commented Mar 4, 2025

  • jnigen uses the top level generateJniBindings

I think this should be changed to have run function to align with the sketch in dart-lang/sdk#56512 (comment). All configuration goes in the constructor declaratively. The run function runs (asynchronously).

  • ffigen has an Uri output. jnigen has a OutputConfig since it also has a multifile mode

Both FFIgen and JNIgen will at some point need to output a list of dependencies. #1009 #1008

So we should probably have an output objects that can easily be read to communicate to a generate hook what written files are, and what the read inputs (dependencies) are.

@liamappelbe
Copy link
Contributor

  • ffigen has an Uri output. jnigen has a OutputConfig since it also has a multifile mode

Ffigen already has multiple output files (Dart code, ObjC code, symbols), so it definitely makes sense to group them into an OutputConfig.

  • ffigen has multiple functions like shouldExposeFunctionTypedefFunc, ... jnigen has a List<Visitor> that combines all such functions
  • ffigen has DeclarationFilters for excluding elements, jnigen has a List<Visitor> that combines all such filters

Visitors definitely make more sense. But does that mean you have to expose your entire set of AST classes to the public API? Or does the visitor only see a more base-ish class that only exposes stuff the user is likely to want to alter (eg name, filtering flag etc)?

Ffigen unfortunately puts a lot of functionality into the AST classes, rather than having the AST be dumb data and doing all the manipulations using separate functions. So exposing all that to a visitor would show a lot of internal details. It would also make it way harder to refactor things in the future since that would all be part of the public API, so would be subject to semver. I'd want to do a gigantic refactor before exposing the AST, and we don't really have time for that if this change is going to land in EAP.

I'd prefer Strings. But I thought last time we discussed this we settled on using Uris? I remember having to switch my whole API from String to Uri.

@HosseinYousefi
Copy link
Member Author

Visitors definitely make more sense. But does that mean you have to expose your entire set of AST classes to the public API? Or does the visitor only see a more base-ish class that only exposes stuff the user is likely to want to alter (eg name, filtering flag etc)?

JNIgen also exposes a minimal AST.

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

No branches or pull requests

3 participants