[ffigen] Vibe coded prototype of transformer API by liamappelbe · Pull Request #3482 · dart-lang/native

liamappelbe · 2026-07-20T03:00:45Z

Following the design of JNIgen's transformers, add a public facing AST that wraps the internal AST and exposes a subset of the fields, then provide a visitor pattern that the user can use to mutate that AST in limited ways.

Collaborated with Gemini on a design, then let it do the migration. I haven't yet done a full review of its code, so this is not ready to land. Let's just see if we like the approach first.

Notes

To continue supporting the old callback system, we construct a visitor from those callbacks, and run it before the user's visitors. Probably should deprecate the callbacks though.
Types are not exposed at all. I could see them being useful as an additional way to identify methods, but it would be complicated. If a user has a concrete use case for it, then we'll design a solution based on that use case.

github-actions · 2026-07-20T03:02:40Z

PR Health

Changelog Entry ❗

Package	Changed Files
package:jni	pkgs/jni/lib/src/third_party/global_env_extensions.dart pkgs/jni/lib/src/third_party/jni_bindings_generated.dart
package:objective_c	pkgs/objective_c/lib/src/c_bindings_generated.dart pkgs/objective_c/lib/src/objc_built_in_types.dart pkgs/objective_c/lib/src/objective_c_bindings_generated.dart

Changes to files need to be accounted for in their respective changelogs.

This check can be disabled by tagging the PR with skip-changelog-check.

API leaks ✔️

The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.

Package	Leaked API symbol	Leaking sources

This check can be disabled by tagging the PR with skip-leaking-check.

Breaking changes

⚠️

Package	Change	Current Version	New Version	Needed Version	Looking good?
jni	Breaking	1.0.3	1.0.3	2.0.0 Got "1.0.3" expected >= "2.0.0" (breaking changes)	⚠️
code_assets	Non-Breaking	1.2.1	1.3.0-wip	1.3.0-wip	✔️
objective_c	Breaking	9.5.0	9.5.0	10.0.0 Got "9.5.0" expected >= "10.0.0" (breaking changes)	⚠️
hooks_runner	Non-Breaking	1.6.1	1.6.2-wip	1.6.2-wip	✔️

This check can be disabled by tagging the PR with skip-breaking-check.

dcharkes

🙌 😄

To continue supporting the old callback system, we construct a visitor from those callbacks, and run it before the user's visitors. Probably should deprecate the callbacks though.

For the vibe coded solution, it would be useful to remove the fields to see what it looks like. I saw at least 1 field which needs to find a new home when we do so. (For the actual change we can use deprecated and document where it should be configured instead.)

Some high level questions:

I don't see any migration of any real world FFIgen config. So it's a bit hard to imagine what the impact of this is for users on the Dart API or the yaml config currently. Could we add some larger migrations to the PR? Do we have any real-world configs in this repo or would we need to look at some user repos? E.g. cupertino_http etc.

Should we consider not providing IncludeSetVisitor? E.g. how much boilerplate is it to write this yourself? My thinking is that if you want something that you don't have a predefined visitor for, you'll need to write a custom one yourself, at which point you might as well fold the predefined sets into your custom one?

What do you envision as the typical user-journey for complicated configurations:

A long list of visitors all doing one aspect.
A single custom visitor?

Which of those two options is more readable? Can you look at some complicated existing configurations and sketch out both options? Is there a clear winner?

(If a long list of visitors is the API winner, are we concerned about performance? If that's the case would we want to mix in visitor aspects into a single visitor for performance reasons?)

dcharkes · 2026-07-20T08:57:31Z

 // TODO: Add a code snippet example.
 final class FfiGenerator {
+  /// User custom visitors to modify/filter AST elements.
+  final List<Visitor>? visitors;


In which order do these run?

One at a time, first to last.

Let's add that to the doc comment :)

dcharkes · 2026-07-20T09:03:51Z

 /// headers.
 // TODO: Add a code snippet example.
 final class FfiGenerator {
+  /// User custom visitors to modify/filter AST elements.


All the other fields are called "configuration".

Maybe we should say something along the lines of:

/// Visitors to configure the generation options for target language declarations. /// /// You can filter: /// code snippet /// /// You can rename: /// code snippet /// /// The visitors run in order. /// some explanation or example

dcharkes · 2026-07-20T09:04:08Z

We need to get rid of all of this. Could you vibecode the removal of all those fields?

Sure. Running it now.

dcharkes · 2026-07-20T09:07:05Z

+      }),
+      RecordUseVisitor(),
+    ],
+    enums: const Enums(silenceWarning: true),


Hm, we need to think where to put everything that doesn't end up in visitors that fit next to the declaration-type-specific config before. How many of such elements are there?

WDYM? I figured we'd just leave those fields where they are, in the hierarchy. Are you suggesting we delete the whole existing hierarchy?

liamappelbe · 2026-07-21T04:56:02Z

I don't see any migration of any real world FFIgen config. So it's a bit hard to imagine what the impact of this is for users on the Dart API or the yaml config currently. Could we add some larger migrations to the PR? Do we have any real-world configs in this repo or would we need to look at some user repos? E.g. cupertino_http etc.

I had it working on migrating some of our configs. I'll upload when it's ready.

Should we consider not providing IncludeSetVisitor? E.g. how much boilerplate is it to write this yourself? My thinking is that if you want something that you don't have a predefined visitor for, you'll need to write a custom one yourself, at which point you might as well fold the predefined sets into your custom one?

I thought of it as not so much a boilerplate saving as something to ease migration. But as you pointed out, it means that we have to lump all the includes into one set (or else it gets to be a pretty messy API with different sets per type). I don't have a strong opinion either way. Maybe it would be better to ease migration by just having good examples in the documentation.

What do you envision as the typical user-journey for complicated configurations:

A long list of visitors all doing one aspect.

A single custom visitor?

Which of those two options is more readable? Can you look at some complicated existing configurations and sketch out both options? Is there a clear winner?

(If a long list of visitors is the API winner, are we concerned about performance? If that's the case would we want to mix in visitor aspects into a single visitor for performance reasons?)

I expect most users could get by with one visitor. More complicated workflows (like our internal visitors) may need to use several visitors. Other users may choose to split up their visitors for readability, eg one visitor per high level task. I don't think performance is a significant issue, so long as users aren't for example creating one visitor per element they want to filter. I don't think even power users would need more than a handful of visitors.

liamappelbe · 2026-07-24T00:06:44Z

Migrated a bunch more monorepo configs, as well as cupertino_http and sqlite3.

liamappelbe · 2026-07-24T05:22:15Z

-
+final class Enums {
  /// Whether to silence warning for enum integer type mimicking.
  final bool silenceWarning;


Not much point having nested classes with only one field. Should flatten this.

liamappelbe · 2026-07-24T05:24:26Z

 }

 /// Configuration for C++.
 final class Cpp {


This is now just a sentinel class. The Config.cpp field is essentially a bool. I'd probably keep this around though, as it's quite likely that we'll need to add C++ specific options again.

liamappelbe · 2026-07-24T05:26:00Z

  /// the [Declarations], but are transitively included by other bindings,
  /// will be code-genned as if they were included. If disabled, these
  /// transitively included interfaces will be generated as stubs instead.
  final bool includeTransitive;


This is a leftover from the YAML config. Might be worth removing (always false). This would force users to explicitly include all the things they care about.

Same for Protocols.includeTransitive.

Categories.includeTransitive could probably also be deleted, but it defaults to true. I think we'd want to keep it that way? The main issue is that there are a lot of undocumented categories in ObjC APIs that hold extension methods of classes which the official documentation treats as ordinary methods on that class. So it's super confusing for users when those methods just don't show up in the generated bindings.

Hm interesting, so now that we visit individual objects, we don't have a place to stick global config w.r.t. all objects of that kind.

We could also put something on visitors to put it closer, but then it becomes mutable state for all the global config, which is also a bit weird. I think in general it's weird to have config on global state.

Should we consider moving the global options to be per definition?

e.g. silence the warning per enum.

And for including categories a setter on Class bool get includeCategories => true.

or on a Class a getter categories so that you can loop over them and include them.

Ditto for the other transitive things, can we navigate the the kinds hierarchy so that you could do transitivity by just walking the graph of definitions?

Doing transitivity that way isn't possible without exposing more of the Types, which is a much bigger change. For example, you don't have access to the arg/return types of any of the methods.

Yeah lets remove the include transitives for now.

And the categories we don't actually need transitive right? It's just one step from the classes. So having a setter on class is easy. I agree that it should default to true. (It would also be annoying if in Dart you would not generate extension methods on classes by default.)

And for the enum int warning, can we do that as a setter on the enum?

Do we then have any of the kinds left?

dcharkes

Thanks @liamappelbe having the sample migrations is really useful! 🙏

Some high level thoughts:

I think what I liked about the yaml config and the previous config with the callbacks is that it facilitates configuration being in a predictable place in one physical location in the code. You build one (giant) object literal, and you can exactly know where inside that literal you can find some code.
- I think one way to get back to that is anonymous classes that implement the visitor. But we don't have those in Dart. The next best thing is a constructor that takes a named param callback for each method that's overridable.
- Secondly, all const sets of included names, and const maps for renames should be inlined in the visit functions where they are used. That way they are in the most narrow context. You don't have to scroll around in the code.
I'd love to be able to get rid of all previous Enums, Functions config etc. Otherwise we'd have configuration in two places (1) in the visitor pertaining to one definition at the time (2) in those global fields pertaining to all objects at the time. Can we express all use cases by adding properties to single definitions getting rid of all the global ones that apply to all?
- What about those transitives?

dcharkes · 2026-07-24T07:31:21Z

 import 'wrapper_generators/logging.dart';

+class JniVisitor extends ffigen.Visitor {
+  static const enumRenames = {


I think I'd inline the const sets that are only used in one method in the method. So that they are more scoped to a narrow context.

dcharkes · 2026-07-24T07:33:48Z

+
+class ObjCBindingsVisitor extends Visitor {
+  static const interfaces = {
+    'DOBJCDartInputStreamAdapter': 'DartInputStreamAdapter',


Could this be expressed more succinctly instead of having a map where a bunch of keys are the same as the values?

dcharkes · 2026-07-24T07:35:17Z

+  }
+}
+
+List<String> writeBuiltInTypes(String out, String bindingsFile) {


This could do with some documentation I think.

dcharkes · 2026-07-24T07:46:59Z

+      generateForPackageObjectiveC: true,
+      categories: Categories(includeTransitive: false),
+    ),
+    visitors: [const ObjCBindingsVisitor()],


I'd kinda want an anonymous class (dart-lang/language#2927), so that the visitor is inlined here. That way we keep the configuration as a single big object literal.

We could achieve that if we provide some kind of base class where all methods are callbacks provided in the constructor, but then they don't have access to a this. But I guess this doesn't have any useful info typically anyways.

FfiGenerator( headers: Headers( entryPoints: [ root.resolve('src/foundation.h'), root.resolve('src/input_stream_adapter.h'), root.resolve('src/ns_number.h'), root.resolve('src/observer.h'), root.resolve('src/protocol.h'), ], ), visitors: [ Visitor( function: (Func node) { node.isExcluded = true; }, objcInterface: (ObjCInterface node) { const interfaces = { 'DOBJCDartInputStreamAdapter': 'DartInputStreamAdapter', // .. }; final renamed = interfaces[node.originalName]; if (renamed != null) { node.isExcluded = false; node.name = renamed; } if (node.originalName == 'NSBundle') { for (final method in node.methods) { if (method.originalName == 'localizedStringForKey:value:table:localizations:') { method.isExcluded = true; } } } } ), ],

We could make the Visitor constructor a forwarding factory that forwards to an impl that has callbacks for each method.

@goderbauer WDYT about the design goal to be able to to nest everything inside the FfiGenerator as an object literal?

Looking at the package:cuppertino_http and package:squlite3 examples I think this is a good design goal.

goderbauer · 2026-07-24T11:59:36Z

-  ),
+  objectiveC: const ObjectiveC(),
+  visitors: const [
+    IncludeSetVisitor({'AVAudioPlayer'}),


One information that's lost now is what kind of thing we are including here. In the old code you could see whether we are retaining an interface, a function, a whatever. Should we keep that information around in the API?

I left a similar comment earlier, I'd either want no include set visitor at all. Or something like

IncludeSetVisitor(functions: {'AVAudioPlayer'}, structs: ...),

goderbauer · 2026-07-24T12:01:20Z

 // TODO: Add a code snippet example.
 final class FfiGenerator {
+  /// User custom visitors to modify/filter AST elements.
+  final List<Visitor>? visitors;


Let's add that to the doc comment :)

goderbauer · 2026-07-24T12:02:58Z

 // TODO: Add a code snippet example.
 final class FfiGenerator {
+  /// User custom visitors to modify/filter AST elements.
+  final List<Visitor>? visitors;


Is there a difference between setting this to null and setting it to []? If not, I would make this non-nullable and use [] to represent the "no visitor set" state. If there is a difference, let document.

goderbauer · 2026-07-24T12:08:35Z

+  bool get isExcluded;
+  set isExcluded(bool value);


Left this comment elsewhere, but this api with double negatives like node.isExcluded = false reads a little awkward and makes it harder to reason about code (e.g. something like node.isExcluded = !_structNames.contains(node.originalName);). Could we turn this API around (isIncluded) or is there a strong reason for having it this way around?

Mainly historical, for consistency with JNIgen. But we could switch both to isIncluded, and in JNIgen keep around (deprecated?) isExcluded getter/setters that delegate to isIncluded for backwards compatibility.

liamappelbe · 2026-07-30T10:05:01Z

SGTM to do the imported in a follow up PR. But I think we should aim to do it before publishing. (I haven't thought about how yet though.) Alternatively, we could replace all the imported fields in the individual subconfigs with a toplevel imported: Imported(functions: ..., structs: ...) and mark that with the TODO to migrate.

As far as I can tell, Structs.imported, Integers.imported etc were pretty much redundant. They've been removed, and all the tests still pass. I'll double check this when I do the full cleanup.

liamappelbe · 2026-07-30T10:21:13Z

Uploaded more API changes. Gemini is currently addressing some of the general feedback about the way that existing configs were migrated. Might take a while

github-actions · 2026-08-03T03:46:25Z

Package publishing

If you have publishing permissions, you can use the links below to publish the changes after merging this PR.

Package	Version	Status	Publish tag (post-merge)
package:code_assets	1.3.0-wip	WIP (no publish necessary)
package:data_assets	0.20.0	already published at pub.dev
package:ffi	2.2.0	already published at pub.dev
package:hooks	2.2.0-wip	WIP (no publish necessary)
package:hooks_runner	1.6.2-wip	WIP (no publish necessary)
package:jni_flutter	1.0.2	already published at pub.dev
package:jni_util	1.0.0	already published at pub.dev
package:native_toolchain_c	0.19.4-wip	WIP (no publish necessary)
package:record_use	1.1.0	ready to publish	`record_use-v1.1.0`
package:swift2objc	0.3.0-wip	WIP (no publish necessary)
package:swiftgen	0.1.3	already published at pub.dev

Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation.

liamappelbe · 2026-08-03T03:56:30Z

I'm pretty happy with how the new API looks. I'm gonna start splitting this PR up into more manageable chunks.

liamappelbe · 2026-08-03T04:19:50Z

This is the proposed JNIgen API, where I've attempted to align it with the FFIgen one: https://gist.github.com/liamappelbe/d6b70114fb835a12d367cac3ad879ae7

dcharkes · 2026-08-03T08:45:43Z

This is the proposed JNIgen API, where I've attempted to align it with the FFIgen one: https://gist.github.com/liamappelbe/d6b70114fb835a12d367cac3ad879ae7

I'd need to browse the migrated invocations rather than the API to get a feel.

liamappelbe added 2 commits July 20, 2026 12:39

[ffigen] Vibe coded prototype of transformer API

1b014b7

fix tests and migrate existing Dart API ffigen configs

7044019

github-actions Bot added package:ffigen package:code_assets labels Jul 20, 2026

liamappelbe requested review from dcharkes and goderbauer July 20, 2026 03:05

nits

39c2a1b

dcharkes reviewed Jul 20, 2026

View reviewed changes

liamappelbe added 2 commits July 21, 2026 14:57

fix tests

bd69dfe

Delete old API, and migrate a bunch more configs

66b3ab2

github-actions Bot added package:hooks_runner package:jni package:objective_c labels Jul 23, 2026

This was referenced Jul 24, 2026

Migrate to the prototype visitor API liamappelbe/sqlite3.dart#1

Draft

Migrate to the prototype visitor API dart-lang/http#1955

Draft

liamappelbe requested a review from dcharkes July 24, 2026 00:06

liamappelbe commented Jul 24, 2026

View reviewed changes

dcharkes reviewed Jul 24, 2026

View reviewed changes

goderbauer reviewed Jul 24, 2026

View reviewed changes

liamappelbe added 6 commits July 29, 2026 14:33

visitChildren

68bba3a

callback based visitor

dbba048

shard the IncludeSetVisitor

dcd9218

flip isExcluded to isIncluded, and make visitor list non-null

4e08daf

per-enum warning

36358a9

remove include transitive flags

cc8cb0b

liamappelbe added 3 commits July 30, 2026 18:35

Headers -> Input

173954c

Move varArgs

22839e7

more cleanup

983d31f

Split out _VisitorImpl

cceba6d

github-actions Bot added the package:swiftgen label Jul 30, 2026

liamappelbe added 6 commits July 30, 2026 21:51

migrate all configs

1de61b7

Merge branch 'main' into ffigen_transformer

bd53b4f

config docs

a532462

typedefTypeMappings -> typedefImports

b86079b

mac only test

1b767f7

fix large_test

b57df8c

liamappelbe added 6 commits August 3, 2026 14:31

cleaning up ObjC bindings

2b5a5b3

fixes

4f3aacc

clean up

e60f0c8

clean up

ac6d86f

clean up

3df1a17

cleaning

a2006b2

liamappelbe added 7 commits August 4, 2026 10:02

more cleanup

eaad4da

clean up

c4b1e14

fix enum recommendation behavior

98c09b9

clean

c425c00

clean up

eb0f393

clean up

aa9fb00

clean up

a68ad13

Uh oh!

Conversation

liamappelbe commented Jul 20, 2026

Notes

Uh oh!

github-actions Bot commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Health

Uh oh!

dcharkes left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

liamappelbe Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

liamappelbe commented Jul 21, 2026

Uh oh!

liamappelbe commented Jul 24, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dcharkes left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dcharkes Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jul 20, 2026 •

edited

Loading

liamappelbe Jul 21, 2026 •

edited

Loading

dcharkes Jul 24, 2026 •

edited

Loading