Review Request JDK-8186050: StackFrame should provide the method signature

[Peter Levart]

Hi Mandy,

On 09/21/2017 05:15 PM, mandy chung wrote:
> Hi Peter,
>
> Updated webrev:
> http://cr.openjdk.java.net/~mchung/jdk10/webrevs/8186050/webrev.03/
>
> On 9/3/17 7:02 AM, Peter Levart wrote:
>>
>>> Updated webrev:
>>> http://cr.openjdk.java.net/~mchung/jdk10/webrevs/8186050/webrev.02/
>>
>> That's what I had in mind, yes.
>>
>> Looking at the method names, I have a second thought about the too 
>> general StackFrame::getDescriptor(). Not looking at the javadocs, one 
>> could ask: "what is a descriptor of a stack frame?". I don't know, 
>> maybe getMethodDescriptor() would be more to the point or even 
>> getMethodTypeDescriptor() (since it is a descriptor of method 
>> parameter and return types, not containing method name). What do you 
>> and others think?
>
> Descriptor is specified in JVMS 4.3:
>     A /descriptor/ is a string representing the type of a field or method.
>
> I initially had getMethodDescriptor() while "Method" is kinda 
> redundant since StackFrame represents a method invocation. Descriptor 
> in this context is for a method and never be a field. Hence I like 
> Remi's suggestion to rename it to getDescriptor.

Ok, I buy it.

>>
>> Although it is not expected for StackFrame interface to be 
>> implemented by custom classes, it is a public interface. I have seen 
>> 3rd party code implementing JDK interface that was not meant to be 
>> implemented by custom classes just because the interface seemed 
>> appropriate. To keep binary compatibility with such potential 
>> implementations, those two new methods could be default methods 
>> throwing UOE.
>>
> Having a second thought, while it's rarely to have custom StackFrame 
> implementation, I agree making the new methods to be default method 
> that would help migration for tests or other use.
>> nit: while you are at it, you could remove the redundant "static" 
>> modifier from the StackWalker.StackFrame interface declaration.
>>
>
> Done

Just two more things...

1st:

I was I little confused reading this part of javadoc of getDescriptor():

  152          * Returns the <i>descriptor</i> of the method type for
  153          * this stack frame.  A method descriptor is a string 
representing the
  154          * types of parameters that the method takes, and a return 
descriptor,
  155          * representing the type of the value (if any) that the 
method returns
  156          * as defined by <cite>The Java™ Virtual Machine 
Specification</cite>.

Wouldn't it be better to say:

  152          * Returns the <i>descriptor</i> of the method type for
  153          * this stack frame.  A method descriptor is a string 
representing the
  154          * types of parameters that the method takes, and the 
method's return type,

  156          * as defined by <cite>The Java™ Virtual Machine 
Specification</cite>.

I think there is a slight difference between "method return type" and 
"the type of value (if any) that the method returns". The former is a 
property of the method, the later is a property of the returned value 
(primitive or reference or none) in a particular invocation of the 
method and may be different (a subtype).


2nd:

I don't know in what circumstance may the MemberName.getMethodType() or 
.getMethodDescriptor() return null, but the MemberName code is written 
as follows:

  169         if (type == null) {
  170             expandFromVM();
  171             if (type == null) {
  172                 return null;
  173             }
  174         }

Is there a real circumstance when this may happen? Should 
StackWalker.StackFrame.getMethodType() / .getDescriptor() document that 
situation or maybe transform it into an exception or error?

Regards, Peter