[foreign] RFC simplify callback API

[Jorn Vernee]

Hi,

I'm not so sure about this. Some comments:

> 1) it allows a subclass of Callback to still act as a functional
> interface (only one abstract method)
> 2) it gives an error whenever the user is attempting to pass an
> unwrapped lambda to some native function

So, to use it, you still need to call allocateCallback:

     try(var scope = Scope.newNativeScope()) {
         QsortComparator comp = (a, b) -> Integer.compare(a.get(), 
b.get());
         QsortComparator realComp = scope.allocateCallback(comp);

         ...

         qsort(arr.elementPointer(), 6, 4, realComp);
     }

If you pass `comp` or the lambda expression directly you end up with an 
exception. The method signature makes it seem like you can pass a lambda 
or method reference directly, but in reality you can't. This seems like 
a source for bugs to me.

> * callbacks now have a common supertypes, as structs do - which means
> the various methods Scope::allocateCallback and LayoutType::ofFunction
> can be expressed with much stronger static guarantees

I'm not sure I see what you mean here? AFAICT the only hole in the 
static guarantees given by those methods is the fact that a given 
'callback' type might or might not be annotated, which is still there.

> * APIs feel less cluttered - e.g. the signature of qsort just says
> 'QsortComparator' rather than 'Callback<QsortComparator>'

 From a different perspective you could say that the signature is lacking 
essential information; that you need to pass something returned by 
Scope::allocateCallback, i.e. an object that actually points to an 
upcall stub, and not just any implementation of 'QsortComparator'.

> Is this a simplification worth exploring? Or is lumping callback and
> functional interfaces a step too far?

I think I'm in the "step too far" camp on this one for now.

Maybe the civilization layer could handle some of the conversions 
between functional interface and Callback in the future to make the API 
simpler?

Jorn

Maurizio Cimadamore schreef op 2018-11-28 18:48:
> Hi,
> We currently have a Callback<T> type which exposes two functionalities:
> 
> * returns the entry point (a Pointer) of the underlying native stub
> * allows Java code to view it as a functional interface (of type T)
> 
> While this is ok, I've always been a bit puzzled that we needed to
> separate the Callback interface from the functional interface which
> represents it in the Java world. Let's consider a concrete case (from
> StdLIbTest):
> 
> void qsort(Pointer<Integer> base, int nitems, int size,
> Callback<QsortComparator> comparator);
> 
> @NativeCallback("(u64:i32u64:i32)i32")
> interface QsortComparator {
>     int compare(Pointer<Integer> u1, Pointer<Integer> u2);
> }
> 
> 
> The fact that the qsort API has to say Callback<QsortComparator> seems
> a bit cumbersome; of course there's a good reason why it's this way:
> we wanted to force an explicit callback instantiation (via
> Scope::allocateCallback) and move the responsibility of allocating
> callback objects onto clients. So, having a different type in the
> qsort signature force the client to think about the fact that a
> callback has to be allocated here.
> 
> 
> Thinking more generally about this problem, and how to improve over
> the status quo, there are two issues here: (i) on the one hand we'd
> like some functional interface to be available somewhere, so that we
> can express the callback code in terms of a lambda expression; (ii) on
> the other hand we want to protect the binder code from running with
> random functional interface implementations. The status quo achieves
> these objectives by moving the functional interface 'away' from the
> user - wrapping it in a Callback<F> type (where F is the functional
> interface).
> 
> 
> Another solution to achieve this would be to have a Callback interface
> defined as follows:
> 
> interface Callback<T extends Callback<T>> extends Resource {
> 
>     default Pointer<?> ptr() {
>         throw new UnsupportedOperationException("Invalid callback!");
>     }
> 
>     @Override
>     default Scope scope() {
>         return ptr().scope();
>     }
> }
> 
> 
> That is, a Callback still has a method to obtain a pointer (the entry
> point of the callback) - but this time we just reuse the ptr() method
> declared on the Resource interface. Also note that the ptr() method
> has been defaulted with some throwing code. This achieves two effects:
> 
> 1) it allows a subclass of Callback to still act as a functional
> interface (only one abstract method)
> 2) it gives an error whenever the user is attempting to pass an
> unwrapped lambda to some native function
> 
> 
> This would allow us to rewrite the above example as follows:
> 
> void qsort(Pointer<Integer> base, int nitems, int size,
> QsortComparator comparator);
> 
> @NativeCallback("(u64:i32u64:i32)i32")
> interface QsortComparator extends Callback<QsortComparator> {
>     int compare(Pointer<Integer> u1, Pointer<Integer> u2);
> }
> 
> 
> Now, I'm well aware that this is not a 100% clean approach; but there
> seem to be some advantages to it that, I think, are worth discussing:
> 
> * callbacks now have a common supertypes, as structs do - which means
> the various methods Scope::allocateCallback and LayoutType::ofFunction
> can be expressed with much stronger static guarantees
> 
> * clients no longer have to call 'asFunction' on the callback object
> to be able to call the underlying Java method - they can just invoke
> that on the Callback
> 
> * APIs feel less cluttered - e.g. the signature of qsort just says
> 'QsortComparator' rather than 'Callback<QsortComparator>'
> 
> * some of the irregularities surrounding callbacks in LayoutType are
> gone (e.g. we no longer need the ad-hoc getFunctionalIntf() impl
> method, we can just rely on the carrier)
> 
> * we no longer need CallbackImpl
> 
> * allocating a callback is much more similar to allocate a struct: we
> fetch a pointer and we allocate an instance of the synthetic class
> (generated by the binder for us)
> 
> 
> The patch below is an example of how this can be made to work:
> 
> http://cr.openjdk.java.net/~mcimadamore/panama/simpler_callback_intf_v2/
> 
> 
> Is this a simplification worth exploring? Or is lumping callback and
> functional interfaces a step too far?
> 
> Maurizio