Skip to content
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

Implement multiple interfaces and Java intersection type support #1021

Closed
Tracked by #1569
HosseinYousefi opened this issue Mar 17, 2024 · 6 comments · Fixed by #1584
Closed
Tracked by #1569

Implement multiple interfaces and Java intersection type support #1021

HosseinYousefi opened this issue Mar 17, 2024 · 6 comments · Fixed by #1584

Comments

@HosseinYousefi
Copy link
Member

HosseinYousefi commented Mar 17, 2024

Currently one can create an object which implements an interface Foo:

Foo.implement($FooImpl(...));

What about the case where we want to implement both Foo and Bar, I propose adding a series of implementK functions to package:jni.

implement2(Foo.type, Bar.type, $FooImpl(...), $BarImpl(...)); // Returns JIntersection2<Foo, Bar>.

Probably subclassing JObjType<T> with a more specific JInterfaceType<T> and using it for interface type classes. This will then be accepted by implementK methods, since we can't implement any class.

And some classes to wrap multiple interfaces

class JIntersection2<T1 extends JObject, T2 extends JObject> extends JObject {
  static JIntersectionType2<T1, T2> type<T1 extends JObject, T2 extends JObject>(
          JObjType<T1> type1, JObjType<T2> type2) =>
      JIntersectionType2(type1, type2);

  late final T1 asType1 = type1.fromReference(reference);
  late final T2 asType2 = type2.fromReference(reference);

  @override
  late final JIntersectionType2<T1, T2> $type = type(type1, type2);

  final JObjType<T1> type1;
  final JObjType<T2> type2;

  JIntersection2.fromReference(this.type1, this.type2, super.reference)
      : super.fromReference();
}
@HosseinYousefi HosseinYousefi changed the title Implement multiple interfaces Implement multiple interfaces and Java intersection type support Mar 17, 2024
@dcharkes
Copy link
Collaborator

I remember we had some discussion in the past about using the implements clause in Dart versus using closures. I remember us postponing a more declarative API, because things such as runnables/callbacks benefit more from the closure approach. I can't find which GitHub issue had the design sketches for the other API.

We should make sure that whatever we come up with would also work with that API. Or maybe things would even be easier with that API if we can make it work with multiple implements clauses (and multiple mixins for the implementations - I vaguely remember something for generating mixins for implementations).

@HosseinYousefi
Copy link
Member Author

For reference, FFIgen uses a builder pattern to implement multiple interfaces:

        final consumer = ProtocolConsumer.new1();

        final protocolBuilder = ObjCProtocolBuilder();
        MyProtocol.addToBuilder(protocolBuilder,
            instanceMethod_withDouble_: (NSString s, double x) {
          return 'ProtocolBuilder: $s: $x'.toNSString();
        });
        SecondaryProtocol.addToBuilder(protocolBuilder,
            otherMethod_b_c_d_: (int a, int b, int c, int d) {
          return a * b * c * d;
        });
        final protocolImpl = protocolBuilder.build();

Pros:

  • Unlimited number of interfaces that can be implemented
  • No need to create a bunch of extra types and implement<K> methods

Cons:

  • protocolBuilder.build() returns a base object and the user has to remember the types and cast accordingly. The implement<K> way mentioned above can have specific .asType<i> getters
  • If we want to support generic intersection types we will need JIntersection<K> types

I initially wanted to wait for macros to get a better syntax but we need this for #1022. So I want to implement a version of it.

@dcharkes @liamappelbe @mkustermann @lrhn wdyt?

@HosseinYousefi HosseinYousefi added this to the JNI / JNIgen 0.12.0 milestone Sep 16, 2024
@lrhn
Copy link
Member

lrhn commented Sep 16, 2024

What problem are we trying to solve here?
I have not context. I assume it's jnigen, and you want to create an object which implements an interface, which means it's a proxy? (Otherwise it would eitehr inherently implement the type, or not?)

The code

Foo.implement($FooImpl(...));

suggests that Foo.implement is imperative, and makes $FooImpl implement Foo.
The semicolon is what makes me think that. If the value of the expression is actually to be used, then it makes more sense.
Let's assume that this returns an object implementing Foo that forwards its method calls to the handler object $FooImpl. (And then $FooImpl could just be a single Dart function dynamic Function(Invocation invocation))

Then you want to be able to have one object implement two interfaces. The implements2 operation takes two types to implement, and a handler object for each, and creates a single object implementing both, forwarding each interface to its own handler.

(In that case, could you perhaps create the individual handlers first and combine those?

  var fooProxy = Foo.implement(fooHandler);
  var barProxy = Bar.implement(barHandler);
  var fooBarProxy = combineImplements2<Foo, Bar>(fooProxy, barProxy);

That way there Type.implements is still the only way to combine a type with a handler, and the combine... method is the way to combine objects for multiple interfaces into one. Which could potentially work for non-proxy values too! Or would that be bad?)"

What I would consider is:

  • Have JType.proxy(dynamid Function(JavaInvocation invocation) onMemberInvocation) to create a proxy for any type.
  • Have JInvocation contain a member signature which includes name, function signature and source type (interface or class declaration) of the member being invoked. Then one handler can handle multiple interfaces.
  • Have a way to create intersection types: JType.and(JType) or JType & JType, which creates a new Java type, JIntersectionType which is a subtype of JType.
  • Then the way to create a proxy for that is (Foo & Bar).implements(handler).
  • Give a helper function to pass different interface members to different handlers, base on the source type of the invocaiton: invocationHandlerFrom(Map<JType, dynamic Function(JInvocation)> handlers), used like invocationHandlerFrom({Foo: fooHandler, Bar: barHandler}). (If Foo is an object, not a type. If it's a type, I might want something different, like invocationHandlerFrom([handlerFor<Foo>(fooHandler), ...]).)

Then, if you do (if Foo and Bar are objects:

  (Foo & Bar).implements(invocationHandlerFrom({Foo: fooHandler, Bar: barHandler})

you get the effect of implements2.
And if they are types, then:

Foo.and<Bar>().implements(invocationHandlerFrom([handlerFor<Foo>(fooHandler, handlerFor<Bar>(barHandler)]))

would work. (Strawman naming, can definitely be shorter.)

And after that long derailed talk ... I also have no context on the FFIGen protocol builder, so I don't know what is being built. Using a builder pattern in general is usually a good idea, rather than having to create a value immediately from the first operation. More flexible. Also more verbose, but shorthands for the most common cases is usually enough to get around that.

@HosseinYousefi
Copy link
Member Author

HosseinYousefi commented Sep 16, 2024

What problem are we trying to solve here?

We currently can implement a single Java interface, and by implement I mean create a proxy object that redirects to Dart.

The semicolon is what makes me think that. If the value of the expression is actually to be used, then it makes more sense.

Here's a simple example to give you more context:

abstract interface class $RunnableImpl {
  factory $RunnableImpl({
    required void Function() run,
  }) = _$RunnableImpl;

  void run();
}

class _$RunnableImpl implements $RunnableImpl {
  _$RunnableImpl({
    required void Function() run,
  }) : _run = run;

  final void Function() _run;

  void run() {
    return _run();
  }
}

class Runnable extends JObject {
  // Runnable has a factory named implement:
  factory Runnable.implement(
    $RunnableImpl $impl
  ) {
    // ...
  }
  // ...
}

Yes the value of the expression is used. For example Runnable.implement($RunnableImpl(run: () => print('hello'))) can be passed into a method expecting a Runnable.

The reason that the syntax is not simply Runnable.implement(run: ...) is so that users can implement $<ClassName>Impl classes separately in case the interface has many methods, and also have a default factory that makes _$<ClassName>Impl so that it allows creating "inline" implementations for things like quick callbacks.

(And then $FooImpl could just be a single Dart function dynamic Function(Invocation invocation))

To make life somewhat easier we currently generate $FooImpls per interface, meaning they are a collection of functions instead.

(In that case, could you perhaps create the individual handlers first and combine those?

I'm not a fan of this solution as it creates 3 objects in this case and makes object lifetimes a bit more complicated. If we're doing this, then it's better if .implement creates an "unbaked" object that we first combine with other unbaked objects and only then .bake() which is basically the builder pattern.

@lrhn
Copy link
Member

lrhn commented Sep 16, 2024

Thanks, makes sense.

So .implement is a constructor, its value is used, and it forwards the Java members to an object implementing a Dart interface called RunnableImpl with members of the same names and Dart signatures. (The class should probably drop the Impl from its names, it doesn't add anything, and is an abbreviation.)
And this works.

The problem here is to have a way to create a proxy for an intersection type, without having a Java class that implements both/all the types of the intersection. And preferably without having to create unnecessary intermediate objects. (Or at least not unnecessary Java objects?)

Java can do this using the Proxy class methods and suitable arguments, and I assume that's what we're using to create the Java object.
So this is a matter of specifying which types to implement, and which Dart method you forward to.

A builder is an option that doesn't stop at implement6 or some other arbitrary limit, and letting the class itself be the one telling the builder who it is makes it unnecessary to have a public intermediate representation. More object allocations, but only on the Dart side, and maybe only into existing lists.

So, given something like:

class JProxy {
  external factory JProxy();
  Object toObject();
}

and Java types:

class Foo extends JObject {
  external factory Foo.implement($FooImpl handler);
  external static void addToProxy(JProxy proxy, $FooImpl handler);
  // ...
}
abstract interface class $FooImpl { 
  // ...
}
class Bar extends JObject {
  external factory Bar.implement($FooImpl handler);
  external static void addToProxy(JProxy proxy, $FooImpl handler);
  // ...
}
abstract interface class $BarImpl { 
  // ...
}

you can use it as:

class FooBarHandler implements $FooImpl, $BarImpl {
  // ...
}

void main() {
  // Use one object or two different, your choice.
  var myHandler = FooBarHandler();
  var proxy = JProxy();
  Foo.addToProxy(proxy, myHandler);
  Bar.addToProxy(proxy, myHandler);
  dynamic fooBar = proxy.toObject();
  Foo foo = fooBar;
  Bar bar = fooBar;
  // Use it.
}

Not paticularly ergonomic, but when Foo is a Dart type, and it's the only one who who knows how to represent its Java type to the proxy, it's not something we can abstract over.

Does make sense. And works with arbitrary number of interfaces.

@HosseinYousefi
Copy link
Member Author

Makes sense, Thanks. It will also be quite similar to Liam's approach in ffigen so I'll go with this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants