dart-lang / native

Dart packages related to FFI and native assets bundling.
BSD 3-Clause "New" or "Revised" License
114 stars 40 forks source link

Generated ffi_bindings.dart.m doesn't handle platform differences #1469

Closed stuartmorgan closed 1 week ago

stuartmorgan commented 3 weeks ago

I'm using ffigen on code that supports both iOS and macOS, and because of https://github.com/dart-lang/native/issues/1439 I'm ending up with a bunch of macOS-specific types like NSMenu in ffi_bindings.dart.m. There's no target platform #ifdefing in the generated file, so it doesn't compile on iOS. (In my case I think it will compile on macOS, but it wouldn't be hard to construct a situation where it wouldn't compile on either one.)

Ideally the transitive walk of headers should track platform ifdefs, and then automatically ifdef accordingly in the generated file. Failing that, a manual workaround could be a way to list platform exclusions in the ffigen config file.

liamappelbe commented 3 weeks ago

Can you share your ffigen config? Usually when I specify entry point headers, there's a mac or ios in the path. So if you've got significant platform differences, maybe you need a separate ffigen run for ios vs macos?

Ideally the transitive walk of headers should track platform ifdefs, and then automatically ifdef accordingly in the generated file.

If the classes you don't want are gated behind ifdefs, you could just add those defines to the compiler-opts.

dcharkes commented 3 weeks ago

Can you share your ffigen config? Usually when I specify entry point headers, there's a mac or ios in the path. So if you've got significant platform differences, maybe you need a separate ffigen run for ios vs macos?

Ideally the transitive walk of headers should track platform ifdefs, and then automatically ifdef accordingly in the generated file.

If the classes you don't want are gated behind ifdefs, you could just add those defines to the compiler-opts.

@liamappelbe Does this also occur in the platform headers that we might want to pre-generate a package for? Because then we'd really like to avoid duplicating all of them? Or maybe in general we'd like to avoid duplicating. (Both the Dart and objc generated code.)

Some thoughts for if we want to solve this:

  1. Maybe we can tool FFIgen to have two configs, one per OS, and then "merge" the bindings in the "transform" phase before doing code gen. (Probably that's a good reason to use the Dart API as config instead of YAML, then you can edit or copyWith the config instead of fully duplicating yaml files.)
stuartmorgan commented 3 weeks ago

Can you share your ffigen config? Usually when I specify entry point headers, there's a mac or ios in the path.

In this case the relevant entry point is my own header within the plugin, where I have a class that has one ifdef to use UIView in one place on iOS, but NSView on macOS.

So if you've got significant platform differences, maybe you need a separate ffigen run for ios vs macos?

I have probably 95% shared code, and one platform difference.

I'm not clear on how separate ffigen runs would work. The result would be two large Dart files that are almost all identical, and two native files that are maybe half identical (much more once I can do method filtering to get rid of all the parts I don't want). On the native side, I could fully conditionally compile them (although conceptually it would be a bit weird because of the duplication), but what would I do on the Dart side? Import both of them prefixed, and arbitrary use one almost all of the time? That would be weird, because I'd have a bunch of cross-platform code using, say ios.foo. Or have every single call Platform-switched, even though almost all of them are the same? That would be much worse than the corresponding native or Pigeon code.

Ideally the transitive walk of headers should track platform ifdefs, and then automatically ifdef accordingly in the generated file.

If the classes you don't want are gated behind ifdefs, you could just add those defines to the compiler-opts.

In this particular case I could probably restructure things artificially to hide this difference from ffigen-visible headers, but that's not going to be the case in general. What if we want to convert webview_flutter_wkwebview? WKWebView inherits from UIView on iOS, and NSView on macOS. In one case we need to interact with UIScrollView only on iOS.

It isn't that I don't want the classes, it's that they only exist on, and can thus only be compiled for, one of the two platforms I'm implementing.

dcharkes commented 3 weeks ago

It isn't that I don't want the classes, it's that they only exist on, and can thus only be compiled for, one of the two platforms I'm implementing.

Right, we could be generating the Dart wrappers as the union of everything (except for conflicts). This would be similar to an API not being available on either MacOS/iOS (https://github.com/dart-lang/native/issues/1338).

However, the generated objective swift code should be ifdeffed. Maybe we can try to run libclang twice (the parse step) and pass in TARGET_OS_IPHONE or TARGET_IPHONE_SIMULATOR, and TARGET_OS_MAC.

(As an even deeper question, what about C code that is if-deffed differently for Windows vs Linux vs MacOS etc.? We have so far not supported that at all. We don't generate native code for normal C bindings. But we also don't merge the Dart bindings.)

liamappelbe commented 3 weeks ago

However, the generated objective swift code should be ifdeffed. Maybe we can try to run libclang twice (the parse step) and pass in TARGET_OS_IPHONE or TARGET_IPHONE_SIMULATOR, and TARGET_OS_MAC.

clang might give me the availability info I need without running twice. If so, I can scan each block signature, and put them behind ifdefs if any of the args or returns are platform specific.

Actually I thought of a way simpler solution, but it's a bit of a kludge. All these trampolines are doing is calling retain, so I don't actually care what type the arg is. I can probably just use id for all the object/block types in the signatures. This also means I can dedupe trampolines that would have the same signature after this transformation, shrinking the generated code.