RFR (M) JDK-8237766: Enhance signature API to include ResolvingSignatureStream

[Lois Foltan]

On 2/28/2020 10:07 AM, coleen.phillimore at oracle.com wrote:
>
> Hi Lois,  This looks good.  Can I change my mind about the 
> klass_if_loaded name, can you make it as_klass_if_loaded instead (a 
> change of 3 additional characters)?  I don't need to see another 
> webrev if you make that change.

Yes, I like that better as well.  Will make the change before pushing.
Thanks!
Lois

> Thanks,
> Coleen
>
> On 2/28/20 8:47 AM, Lois Foltan wrote:
>> On 2/27/2020 7:37 PM, coleen.phillimore at oracle.com wrote:
>>>
>>> http://cr.openjdk.java.net/~lfoltan/bug_jdk8237766.0/webrev/src/hotspot/share/oops/method.cpp.frames.html 
>>>
>>>
>>> 1683 // These two methods are static since a GC may move the Method
>>>
>>> Not in this change but can you remove this obsolete comment.
>>
>> Done.
>>
>>>
>>> http://cr.openjdk.java.net/~lfoltan/bug_jdk8237766.0/webrev/src/hotspot/share/runtime/signature.cpp.frames.html 
>>>
>>>
>>> Can you make line 422 shorter?  And I think the signature should be 
>>> the first parameter since that's the main parameter.
>>
>> Done. Changed both ResolvingSignatureStream constructors that have a 
>> signature parameter.  It is now the first parameter.
>>
>>>
>>> 452 Klass* ResolvingSignatureStream::as_loaded_klass(TRAPS) {
>>>
>>>
>>> I think as_loaded_klass as a name doesn't really say that you're not 
>>> going to load the class.  Can you rename klass_if_loaded()? There's 
>>> a klass_at_if_loaded which does the same sort of thing in the 
>>> constant pool.
>>
>> Done.
>>
>>>
>>> This change looks good to me apart from these minor requests.
>>
>> Thank you for the review!  Updated webrev at: 
>> http://cr.openjdk.java.net/~lfoltan/bug_jdk8237766.1/webrev/
>> Lois
>>
>>>
>>> Thanks,
>>> Coleen
>>>
>>> On 2/25/20 11:41 AM, Lois Foltan wrote:
>>>> Please review the following enhancement to introduce a 
>>>> ResolvingSignatureStream class into the signature API support. This 
>>>> consolidates numerous places in the code that manually walk through 
>>>> the differing parts of a signature and request to resolve or find 
>>>> the respective underlying types. ResolvingSignatureStream now 
>>>> provides this capability in order to eliminate the code duplication 
>>>> and redundancy.
>>>>
>>>> open webrev at: 
>>>> http://cr.openjdk.java.net/~lfoltan/bug_jdk8237766.0/webrev/
>>>> bug link: https://bugs.openjdk.java.net/browse/JDK-8237766
>>>> contributed-by: Lois Foltan, John Rose
>>>>
>>>> Testing: hs-tier1-3 (all platforms), hs-tier4-6 (linux only)
>>>>
>>>> Thanks,
>>>> Lois
>>>
>>
>