RFR 10 (L): 8183012: Code cleanup in com.sun.tools.jdi

[Langer, Christoph]

Hi Thomas,

thanks for your review!

> com/sun/tools/jdi/InvokableTypeImpl.java:
> 
> import com.sun.jdi.VMCannotBeModifiedException; ?Why do we need to
> import this type, we do not seem to use it?

This is needed for the documentation, see line 97, the throws documentation.

> ObjectReferenceImpl:
> 
> the removed code: Looks like part of the checks are missing since a long time.
> The remains could be understood as a check that the class for the method to
> be invoked exists in the debuggee.  But then, there are no guarantees
> anyway that inbetween firing the invoke command to the debuggee and the
> debuggee processing the invoke command that class may not be unloaded,
> so we may just rely on the jdwp invoke command failing for that case. So, ok
> to remove it IMHO.

I thought so, too.

> SDE.java:
> 
> @SuppressWarnings("unused") ? which member is unused?

It's about member "njplsEnd". Still it looks cleaner not to remove it but to suppress the warning.

> "SocketTransportService.java: pull out SocketConnection to an own file
> SocketConnection.java" - why?

In the codebase that I was merging the package with, the class SocketConnection needed to be declared public for some reason.
This is only allowed if the code lives in a file which is named like the class. And since I think it's generally not wrong to have classes
in their own file and I find other package private classes of com.sun.tools.jdi their own class files, too, I thought it's a win-win to pull it out. :)
It will ease future merging.

> com/sun/tools/jdi/VMModifiers.java:
> 
> Not touched by your change, but weird: In VMModifiers, some of the
> constants share numerical values:
> 
> 28 public interface VMModifiers {
> ...
>   35     int VOLATILE = 0x00000040;      /* can cache in registers */
>   36     int BRIDGE = 0x00000040;        /* Bridge method generated by compiler
> */
> 
>   37     int TRANSIENT = 0x00000080;     /* not persistant */
>   38     int VARARGS = 0x00000080;       /* Method accepts var. args*/
> ...
> 
> So, VOLATILE == BRIDGE and TRANSIENT == VARARGS? This seems wrong, no?

This really looks weird, I agree. But it's out of scope for me to dig further... :)

As I addressed all the points you mentioned, I will consider the change reviewed by you.

Best regards
Christoph