RFR: 8165827: Support private interface methods in JNI, JDWP, JDI and JDB

[serguei.spitsyn at oracle.com]

Hi David,

It looks good, thank you for test improvements.

One minor comment.

http://cr.openjdk.java.net/~dholmes/8165827/webrev/test/com/sun/jdi/InterfaceMethodsTest.java.frames.html



511 private Method testLookup(ReferenceType targetClass, String 
methodName, String methodSig,
512 boolean declaredOnly, Class<?> expectedException) {
513
514 System.err.println("Looking up " + targetClass.name() + "." + 
methodName + methodSig);
515 try {
516 Method m = declaredOnly ?
517 lookupDeclaredMethod(targetClass, methodName, methodSig) :
518 lookupMethod(targetClass, methodName, methodSig);
519
520 if (expectedException == null) {
521 System.err.println("--- PASSED");
522 return m;
523 }
524 }
525 catch (Throwable t) {
526 if (t.getClass() != expectedException) {
527 System.err.println("--- FAILED");
528 failure("FAILED: got exception " + t + " but expected exception "
529 + expectedException.getSimpleName());
530 return null;
531 }
532 else {
533 System.err.println("--- PASSED");
534 return null;
535 }
536 }
537 System.err.println("--- FAILED");
538 failure("FAILED: lookup succeeded but expected exception "
539 + expectedException.getSimpleName());
540 return null;
541 }

   I'd be better to keep the fragments 520-523 and 537-540 together as 
they are logically bound.
   Perhaps, it is better to move the 520-523 to move before the L537.
   There are more cases to use the testLookup() in this test but it is 
probably for future improvements.


Thanks,
Serguei



On 10/10/16 18:55, 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