[foreign] Improve error reporting when Java signature and native function layout don't match.

[Maurizio Cimadamore]

Hi Jorn,
your patch looks good, but I think I would like it to just limit things 
at a basic arity check (and void vs. non void check) in the function. 
Given how we treat native types (layout + carrier), how carriers are 
assigned to layouts is up to the method signature - if somebody really 
wants to see a pointer as a float, so be it, although I agree it looks 
silly.

More generally, I think that if we go down the path of enforcing more 
checks, we probably want to enforce carrier vs. layout compatibility in 
a different place - that is LayoutType. That is the natural place where 
to rule out ill-formed combinations.

So there are two checks here: one is the one you needed, which checks 
that a method signature is compatible with its function layout (same 
arity, same varargness, same kind of return). The other is a more in 
depth check performed on LayoutType, which I think is rather orthogonal 
w.r.t. former. I'd suggest to go ahead with the first.

Makes sense?

Maurizio



On 25/09/18 14:27, Jorn Vernee wrote:
> Hello,
>
> Thanks for accepting my last contribution!
>
> During testing I was running into an exception like this:
>
>     Exception in thread "main" 
> java.lang.ArrayIndexOutOfBoundsException: Index 0 out of bounds for 
> length 0
>     at 
> java.base/jdk.internal.foreign.NativeInvoker.invokeNormal(NativeInvoker.java:216)
>     at 
> java.base/jdk.internal.foreign.NativeInvoker.invoke(NativeInvoker.java:200)
>     at com.github.jdnl.Main$MyLib$Impl/0x000000010009c440.exit(Unknown 
> Source)
>       at com.github.jdnl.Main.main(Main.java:17)
>
> The cause turned out to be a mismatch between the Java method 
> signature and the native function layout:
>
>     @NativeHeader(declarations =
>         "exit=()v" // missing param
>     )
>     interface MyLib {
>         void exit(int code);
>     }
>
> Since this exception is pretty cryptic, I'd like to contribute a patch 
> that adds an explicit check for this to NativeInvoker, which does some 
> basic verification that the two signatures match. With the patch I get 
> this error instead (note the last 'Caused by'):
>
>     Exception in thread "main" java.lang.RuntimeException: Failed to 
> generate implementation for class interface com.github.jdnl.Main$MyLib
>     at 
> java.base/jdk.internal.foreign.LibrariesHelper.generateImpl(LibrariesHelper.java:65)
>     at 
> java.base/jdk.internal.foreign.LibrariesHelper$3.computeValue(LibrariesHelper.java:132)
>     at 
> java.base/jdk.internal.foreign.LibrariesHelper$3.computeValue(LibrariesHelper.java:124)
>     at java.base/java.lang.ClassValue.getFromHashMap(ClassValue.java:226)
>     at java.base/java.lang.ClassValue.getFromBackup(ClassValue.java:208)
>     at java.base/java.lang.ClassValue.get(ClassValue.java:114)
>     at 
> java.base/jdk.internal.foreign.LibrariesHelper.getHeaderImplClass(LibrariesHelper.java:155)
>     at 
> java.base/jdk.internal.foreign.LibrariesHelper.bind(LibrariesHelper.java:241)
>     at 
> java.base/jdk.internal.foreign.LibrariesHelper.bind(LibrariesHelper.java:260)
>     at java.base/java.foreign.Libraries.bind(Libraries.java:61)
>     at com.github.jdnl.Main.main(Main.java:17)
>     Caused by: java.lang.RuntimeException: Failed to generate method 
> public abstract void com.github.jdnl.Main$MyLib.exit(int)
>     at 
> java.base/jdk.internal.foreign.BinderClassGenerator.generateMembers(BinderClassGenerator.java:153)
>     at 
> java.base/jdk.internal.foreign.HeaderImplGenerator.generateMembers(HeaderImplGenerator.java:103)
>     at 
> java.base/jdk.internal.foreign.BinderClassGenerator.generate(BinderClassGenerator.java:103)
>     at 
> java.base/jdk.internal.foreign.LibrariesHelper.lambda$generateImpl$0(LibrariesHelper.java:62)
>     at java.base/java.security.AccessController.doPrivileged(Native 
> Method)
>     at 
> java.base/jdk.internal.foreign.LibrariesHelper.generateImpl(LibrariesHelper.java:61)
>     ... 10 more
>     Caused by: java.lang.IllegalArgumentException: Java method 
> signature and native layout not compatible: public abstract void 
> com.github.jdnl.Main$MyLib.exit(int) : ()v
>     at 
> java.base/jdk.internal.foreign.NativeInvoker.of(NativeInvoker.java:127)
>     at 
> java.base/jdk.internal.foreign.HeaderImplGenerator.generateFunctionMethod(HeaderImplGenerator.java:130)
>     at 
> java.base/jdk.internal.foreign.HeaderImplGenerator.generateMethodImplementation(HeaderImplGenerator.java:121)
>     at 
> java.base/jdk.internal.foreign.BinderClassGenerator.generateMembers(BinderClassGenerator.java:150)
>     ... 15 more
>
> And it is thrown when binding the interface, while the original 
> exception was thrown when invoking the method.
>
> I have kept the implementation fairly permissive, so things like 
> binding `void m(Pointer<?> p)` to `(u64)v` (converting a pointer to an 
> integer) should still be allowed, but maybe a stricter approach is 
> preferable? I have added a test to verify that it catches some basic 
> mistakes like missing parameters or return types.
>
> Diff: https://gist.github.com/JornVernee/2fb68d70bad1aa807b50b77840117b27
>
> I saw an RFR and JBS bug was made for my last contribution (thanks 
> Sundar!), so I've not added 'RFR' to the subject line this time.
>
> Regards,
> Jorn