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

[Daniel D. Daugherty]

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

src/jdk.jdi/share/classes/com/sun/jdi/ObjectReference.java
     No comments. (Thanks for also fixing the typo.)

src/jdk.jdi/share/classes/com/sun/tools/jdi/ObjectReferenceImpl.java
     L352:         if (isNonVirtual(options)) {
     L353:             if (method.isAbstract()) {
     L354:                 throw new IllegalArgumentException("Abstract 
method");
     L355:             }
         Any particular reason for breaking the logic into two
         distinct if-statements?

         Perhaps:

                   if (isNonVirtual(options) && method.isAbstract()) {
                       throw new IllegalArgumentException("Abstract 
method");
                   }

         Also, perhaps "unexpected Abstract method" is more clear?

test/com/sun/jdi/InterfaceMethodsTest.java
     L526:             if (t.getClass() != expectedException) {
     L527:                 System.err.println("--- FAILED");
     L528:                 failure("FAILED: " + t);
     L529:                 return null;
     L530:             }
         You should also report the expectedException value here to
         aid in failure analysis.

Thumbs up! I don't need to see another webrev if you decide to
make the above small tweaks.

Dan


>
> 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