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

[Peter Levart]

Hi Mandy,

On 09/03/2017 04:52 AM, mandy chung wrote:
>
> On 9/2/17 2:57 AM, Peter Levart wrote:
>> Hi Mandy,
>>
>> The API looks fine to me.
>>
>> Note that there is an opportunity for a follow-up optimization of the 
>> StackFrameInfo::getDescriptor() case. When MemberName's 'type' field 
>> is filled by native expandFromVM() it is usually filled with the 
>> descriptor string. MemberName::getMethodType() then parses this 
>> string into a MemberType, resolving all the types. So when 
>> StackFrameInfo::getDescriptor() is called, the descriptor string is 
>> 1st parsed into MethodType and then formatted back to the descriptor. 
>> By introducing new method into package-private MemberName - say 
>> getMethodDescriptorString(), this intermediate conversion could often 
>> be avoided (for example, if getMethodDescriptorString() was called 
>> before getMethodType() on an instance of MethodName).
>>
> Good suggestion.
>
> 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?

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.

nit: while you are at it, you could remove the redundant "static" 
modifier from the StackWalker.StackFrame interface declaration.

Regards, Peter

>
> Thanks
> Mandy
>> Regards, Peter
>>
>> On 09/01/2017 07:39 AM, mandy chung wrote:
>>> Updated webrev:
>>> http://cr.openjdk.java.net/~mchung/jdk10/webrevs/8186050/webrev.01/index.html 
>>>
>>>
>>> This introduces two new methods, StackFrame::getMethodType and 
>>> StackFrame::getDescriptor.
>>>
>>> Mandy
>>>
>>> On 8/30/17 12:25 AM, Remi Forax wrote:
>>>> Hi Mandy,
>>>> thanks for taking care of this.
>>>>
>>>> In my opinion, we should provide both getMethodType() and 
>>>> getDescriptor(),
>>>> getDescriptor() is handy for logging (finding the right overload 
>>>> when line numbers are not present) and getMethodType() is the one 
>>>> you whant if you want to inspect the runtime view of the stack 
>>>> frames (and by example interact with java.lang.invoke). For me, 
>>>> it's the same reason that give us getDeclaringClass() and 
>>>> getClassName() in the current API.
>>>>
>>>> So getDescriptor() can be called with no restriction but 
>>>> getMethodType() requires RETAIN_CLASS_REFERENCE.
>>>>
>>>> regards,
>>>> Rémi
>>>>
>>>> ----- Mail original -----
>>>>> De: "mandy chung" <mandy.chung at oracle.com>
>>>>> À: "core-libs-dev" <core-libs-dev at openjdk.java.net>
>>>>> Envoyé: Mardi 29 Août 2017 00:57:28
>>>>> Objet: Review Request JDK-8186050: StackFrame should provide the 
>>>>> method signature
>>>>> Method signature is missing in the StackFrame API. This proposes 
>>>>> to add
>>>>> StackFrame::getMethodDescriptor method to return the method 
>>>>> descriptor
>>>>> in a stack frame.
>>>>>
>>>>> Webrev at:
>>>>> http://cr.openjdk.java.net/~mchung/jdk10/webrevs/8186050/webrev.00/index.html 
>>>>>
>>>>>
>>>>> There are a couple options how to present the method signature in the
>>>>> API level:
>>>>> 1. Class<?>[] getParameterTypes() and Class<?> getReturnTypes() 
>>>>> similiar
>>>>> to what java.lang.reflect.Method has.
>>>>> 2. java.lang.invoke.MethodType
>>>>> 3. a String representation (i) comma-separated list of the method's
>>>>> formal parameter types (ii) bytecode method descriptor as 
>>>>> specified in JVMS
>>>>>
>>>>> Returning Class<?> instance should require to add a new StackWalker
>>>>> option to access to the parameter types and return type for option #1
>>>>> and #2. StackFrame::getDeclaringClass requires the stack walker to 
>>>>> have
>>>>> the RETAIN_CLASS_REFERENCE capability.
>>>>>
>>>>> Option #2 returning MethodType is handy while java.lang would 
>>>>> reference
>>>>> a type in java.lang.invoke.
>>>>>
>>>>> Option #3 requires the caller to parse the return string and call
>>>>> Class.forName to get the Class<?> instance. OTOH
>>>>> MethodType::fromMethodDescriptorString method that returns MethodType
>>>>> from a bytecode method descriptor string.
>>>>>
>>>>> Method signature is for information for typical cases. Getting 
>>>>> Class<?>
>>>>> for the parameter types and return type would be a niche case. I 
>>>>> think
>>>>> returning the method descriptor string is a good option - keep the 
>>>>> API
>>>>> simple and can use MethodType::fromMethodDescriptorString to get back
>>>>> the types if needed.
>>>>>
>>>>> thanks
>>>>> Mandy
>>>
>>
>