Better block names

[liamappelbe]

Instead of ObjcBlock, ObjCBlock1, ObjCBlock2 etc, we assign block names based on their type. These are much more verbose (eg ObjCBlock_Int32_Int32) but at least won't change based on header parse order or including more headers like the numbers can. This stability allows users to typedef the long type names down to more readable names based on their use case.

Fixes dart-lang/native#472.

[liamappelbe]

@dcharkes Ideally we'd just have a single ObjCBlock class and template it on the function type, but I'm not sure what to do about the listener constructor. We could use extension types to add methods to specific ObjCBlock<T>, but that wouldn't work for the listener constructor. Maybe there's a way to make it work if I change the constructors to static methods? It would also mean more dynamic dispatch if we're storing all the functions in a single Map<int, Function> and casting them at runtime to the specific function type. WDYT?

[dcharkes]

I'd like to use consistent naming among FFIgen and JNIgen. We had the same discussion in JNIgen but didn't make a move there. dart-lang/native#585

I believe the renumbering here might be worse because the blocks are not named within a class like the constructors in Java or Kotlin. Do you have some examples of how bad the renumbering gets on some example APIs?

cc @HosseinYousefi

[liamappelbe]

Do you have some examples of how bad the renumbering gets on some example APIs?

The HealthKit example I'm working on gets up to ObjCBlock54. As for stability, any time you change the config to include or exclude a class, if that class uses any blocks, that will likely change the numbering of a bunch of ObjCBlocks. I just ran into this when I added the void block type to block_test.dart for the listener PR, and it renamed the int block to ObjCBlock1. I'd say this is a pretty serious polish/usability issue.

I definitely want to switch to either this PR's type name approach, or figure out how to do it using templates. The question is whether we like ObjCBlock_Int32_Int32 or ObjCBlock<ffi.Int32 Function(ffi.Int32)>, and whether the latter is feasible. Both are verbose, but it's easy for the user to typedef to something shorter (like I do in block_test.dart), since these names are stable.

[dcharkes]

The HealthKit example I'm working on gets up to ObjCBlock54. As for stability, any time you change the config to include or exclude a class, if that class uses any blocks, that will likely change the numbering of a bunch of ObjCBlocks.

Okay, that sounds rather painful. So we definitely want a solution here.

If you run FFIgen on a substantial part of the APIs, how large to the names get?

or figure out how to do it using templates

wdym? You're using a type argument there right? Not a template?

[liamappelbe]

If you run FFIgen on a substantial part of the APIs, how large to the names get?

The longest name in the HealthKit bindings is ObjCBlock_ffiVoid_ObjCObject_ffiUnsignedLong_bool. Under the type argument approach, it'd be even longer: ObjCBlock<ffi.Void Function(ObjCObject, ffi.UnsignedLong, bool)>.

wdym? You're using a type argument there right? Not a template?

Yeah, I meant type arguments. In C++ land these are called templates, and I've been using the same term in Dart. What does "template" mean to you, in a Dart context?

Anyway, @dcharkes I was just wondering if you had any opinion about whether ObjCBlock_Int32_Int32 or ObjCBlock<ffi.Int32 Function(ffi.Int32)> is better. Personally I lean towards the type argument approach, assuming it's possible, because I have another thing on my todo list to pull the basic classes (eg NSString) out into their own library (#612), and it's possible we could do that for ObjCBlock too if we go with the type arg approach. The main downside is the type arg version is even more verbose.

UPDATE: I tried implementing the type arg approach today, but I can't figure out a clean way of doing the constructors. They need to be code genned for each block type, but we don't have static extension methods, so there's no good way of doing this AFAICT. Best I can do is put a static method on the extension (and name the extension using the scheme in this PR), which the user would have to invoke like this:

ObjCBlock<ffi.Int32 Function(ffi.Int32)> myBlock = ObjCBlock_Int32_Int32.fromFunction(lib, func);

The benefits I mentioned above would still apply, so it's arguably still better than just using ObjCBlock_Int32_Int32. But what I really want is to be able to do this:

final myBlock = ObjCBlock<ffi.Int32 Function(ffi.Int32)>.fromFunction(lib, func);

Unfortunately I don't see a way of doing that without static extension methods.

[dcharkes]

Yeah, I meant type arguments. In C++ land these are called templates, and I've been using the same term in Dart. What does "template" mean to you, in a Dart context?

Off-topic:
Templates in C++ are compile-time evaluated, while in Dart generics have a runtime representation. Runtime representation means you can put different stuff in and type check at runtime. So in my mind they're two different concepts. But I see why you would say they are similar.

And in doing code generation, a template could just be a string with dollar variable names: https://github.com/flutter/flutter/tree/master/packages/flutter_tools/templates. But that is using templates without type arguments, but in the context of code generation, which we are doing here as well.

On-topic:
Trying to get the type arg approach work sgtm.

The benefits I mentioned above would still apply, so it's arguably still better than just using ObjCBlock_Int32_Int32. But what I really want is to be able to do this:

final myBlock = ObjCBlock<ffi.Int32 Function(ffi.Int32)>.fromFunction(lib, func);

Can't we achieve that by a factory constructor on ObjCBlock? Or does the body need to call some generated code based on the type argument? (As that wouldn't work if ObjCBlock is pre-defined before that code is generated.)

[liamappelbe]

After playing around a bit, I don't think it's possible to implement a clean ObjCBlock<T> approach. So I think the best way to move forward with this PR is to just land it as is, and when I get to dart-lang/native#430 I'll have all these generated classes implement an ObjCBlock<T> interface that lives in that shared library.

[dcharkes]

After playing around a bit, I don't think it's possible to implement a clean ObjCBlock<T> approach.

It's because we need to call some specific generated code based on the type right? It might be worth leaving some documentation trail with examples.

So I think the best way to move forward with this PR is to just land it as is, and when I get to dart-lang/native#430 I'll have all these generated classes implement an ObjCBlock<T> interface that lives in that shared library.

sgtm

[dcharkes]

lgtm!

Thanks @liamappelbe !