Clicked send a little too soon...

Very nice job on this API addition for supporting default and static
interface methods. In particular, congratulations on getting this
approved via CCC for the JDK8-Update train! It is very rare that we
can make API changes in an update...

Dan


On 4/25/14 12:08 PM, Daniel D. Daugherty wrote:
On 4/24/14 6:52 AM, Jaroslav Bachorik wrote:
Please, review the following patch:

Issue : https://bugs.openjdk.java.net/browse/JDK-8031195
Webrev: http://cr.openjdk.java.net/~jbachorik/8031195/webrev.04

make/data/jdwp/jdwp.spec
    line 1256: "<p>Since JDWP version 1.8
        Missing the ending double quote.

    line 1281: "suspended by an event or by command. "
        "by command" isn't clear. "by this command", "by a command"
        or something else more specific would be good.

line 1322: (Error INVALID_METHODID "methodID is not the ID of a method.")
        The above description permits a 'methodID' value that
        is not for a method in the 'clazz' interface. Perhaps
        add "in the interface specified by clazz" at the end
        of the description?

src/share/back/VirtualMachineImpl.c
    No comments.

src/share/back/debugDispatch.c
    No comments.

src/share/back/util.c
    No comments.

src/share/classes/com/sun/jdi/ClassType.java
    No comments.

src/share/classes/com/sun/jdi/InterfaceType.java
    line 88: * but not a static initializer.
        The 'jdwp.spec' wording does not mention this restriction.

    typo line 107: * enclosing class's class loader).
typo line 184: * loaded through the enclosing class's class loader.
               ->  enclosing class' class loader

    line 189: * @throws VMCannotBeModifiedException ...
        Please add the following after line 189:

            *
            * @since 1.8

    line 193-196: These exception are not named in the throws clause:
        IllegalArgumentException, VMCannotBeModifiedException


src/share/classes/com/sun/jdi/Method.java
    line 144: * false otherwise
        Please add the following after line 144:

            *
            * @since 1.8

src/share/classes/com/sun/jdi/ObjectReference.java
    No comments.

src/share/classes/com/sun/tools/example/debug/expr/LValue.java
    No comments.

src/share/classes/com/sun/tools/jdi/ClassTypeImpl.java
    line 32: final public class ClassTypeImpl extends InvokableTypeImpl
        The switch to "final" caught my eye. I presume that
        SA-JDI does not extend this implementation class.

    Most of these changes appear to be due to refactoring with
    the new InvokableTypeImpl.java. Tried to do a visual diff
    between the common parts of this file and InvokableTypeImpl.java.
    Didn't see anything obviously wrong.

src/share/classes/com/sun/tools/jdi/InterfaceTypeImpl.java
    Most of these changes appear to be due to refactoring with
    the new InvokableTypeImpl.java.  Tried to do a visual diff
    between the common parts of this file and InvokableTypeImpl.java.
    Didn't see anything obviously wrong.

src/share/classes/com/sun/tools/jdi/MethodImpl.java
    No comments.

src/share/classes/com/sun/tools/jdi/ObjectReferenceImpl.java
    No comments.

src/share/classes/com/sun/tools/jdi/VirtualMachineManagerImpl.java
    No comments.

src/share/back/InterfaceTypeImpl.c src/share/back/ClassTypeImpl.c
    No comments.

src/share/back/InterfaceTypeImpl.h src/share/back/ClassTypeImpl.h
    No comments.

src/share/classes/com/sun/tools/jdi/InvokableTypeImpl.java
    Most of this code came from refactoring  ClassTypeImpl.java or
    InterfaceTypeImpl.java.

    line 98: throws clause does not mention:
        IllegalArgumentException or VMCannotBeModifiedException

        But I also have to wonder why this JavaDoc is here since
        this is an impl class...

test/com/sun/jdi/EvalInterfaceStatic.sh
    line 35: #  the above error occurs.  jdb doesnt notice that this is
        Not sure what "the above error" is. I don't see an error
        example above this line.

        typo: "doesnt" -> "doesn't"

test/com/sun/jdi/InterfaceMethodsTest.java
    Very nice test!

Dan



With JDK8 it became possible to have methods with implementation in interfaces - static and default interface methods. However the JDI and JDWP were not updated to reflect these capabilities so it is not currently possible to invoke a static or default interface method programatically from the debugger.

This patch adds support for static and default interface methods to JDI, JDWP and JDB.

Thanks,

-JB-


Reply via email to