[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8165827: Support private interface methods in JNI, JDWP, JDI and JDB
From:       David Holmes <david.holmes () oracle ! com>
Date:       2016-10-18 23:56:52
Message-ID: 4a476eaa-07be-d029-6fef-c7ebbb357708 () oracle ! com
[Download RAW message or body]

On 19/10/2016 12:27 AM, Daniel D. Daugherty wrote:
> On 10/17/16 9:59 PM, David Holmes wrote:
>> Hi Lois, Dan, Serguei,
>>
>> Went to push this today and realized I had left off the updated JNI
>> method lookup tests. As I said in the bug report JNI behaves as
>> expected, but there weren't any testcases so I added them:
>>
>> http://cr.openjdk.java.net/~dholmes/8165827/webrev.hotspot/
>
> test/runtime/jni/PrivateInterfaceMethods/PrivateInterfaceMethods.java
>     L74:         lookup(A.class.getName(), "onlyA", null); //should succeed
>     :
>     :
>     L90:         lookup(Impl2.class.getName(), "onlyC",
> NoSuchMethodError.class); //should fail
>         nit: please add a space after '//'
>
>     L138:         String desc = " Lookup of " + definingClass + "." +
> method;
>         nit: any particular reason for the space before Lookup?

Just checking your powers of observation :)

>
>
> test/runtime/jni/PrivateInterfaceMethods/libPrivateInterfaceMethods.c
>     L78: blank line at the end of the file. jcheck will probably complain.

Yeah I deal with that at commit time.

>
> Thumbs up! Feel free to ignore the nits. No need to see a new
> webrev if you fix them.

Thanks. Will fix the nits.

David

> Dan
>
>
>>
>> Thanks,
>> David
>>
>> On 11/10/2016 11:55 AM, David Holmes wrote:
>>> Turns out the only place changes were needed were in JDI.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8165827
>>>
>>> webrev: http://cr.openjdk.java.net/~dholmes/8165827/webrev/
>>>
>>> The spec change in ObjectReference is very simple and there is a CCC
>>> request in progress to ratify that change.
>>>
>>> The implementation change in ObjectReferenceImpl mirrors the updated
>>> spec and use the same format as already present in the class version of
>>> the check method.
>>>
>>> The test is a little more complex. This is obviously an extension to
>>> what is already tested in InterfaceMethodsTest. However IMT has a number
>>> of problem with the way it is currently written [1] - specifically it
>>> doesn't properly separate method lookup from method invocation. So I've
>>> added the capability to separate lookup and invocation for use with the
>>> private interface methods - I have not tried to address shortcomings of
>>> the existing tests. Though I did fix the return value checking logic!
>>> And did some clarifying comments and renaming in a couple of place.
>>>
>>> Still on the test I can't add the negative tests I would like to add
>>> because they actually pass due to a different long standing bug in JDI -
>>> [2]. So the actual private interface method testing is very simple: can
>>> I get the Method from the InterfaceType for the interface declaring the
>>> method? Can I then invoke that method on an instance of a class that
>>> implements the interface.
>>>
>>> Thanks,
>>> David
>>>
>>> [1] https://bugs.openjdk.java.net/browse/JDK-8166453
>>> [2] https://bugs.openjdk.java.net/browse/JDK-8167416
>
[prev in list] [next in list] [prev in thread] [next in thread]