RFR [9] 8141615: Add new public methods to sun.reflect.ConstantPool

[Konstantin Shefov]

Coleen,
Thanks for review

On 11/24/2015 07:33 PM, Coleen Phillimore wrote:
>
> I have a couple preliminary comments.  First, there are no tests added 
> with all this new functionality.  Tests should be added with the 
> functionality changeset, not promised afterward. 
I will add tests.
> Secondly, I do not like that JDK code knows the implementation details 
> of the JVM's constant pool tags.  These should be only for internal use.
The package "sun.reflect" is for internal use only, so it shouldn’t be a 
problem.
> My third comment is that I haven't looked carefully at this constant 
> pool implementation but it seems very unsafe if the class is redefined 
> and relies on an implementation detail in the JVM that can change.   I 
> will have more comments once I look more at the jvmti specification.
>
> thanks,
> Coleen
>
> On 11/24/15 9:48 AM, Konstantin Shefov wrote:
>> Hello
>>
>> Please, review modified webrev.
>>
>> I have added methods
>> getNameAndTypeRefIndexAt(int index) - to get name and type entry 
>> index from a method, field or indy entry index;
>> getClassRefIndexAt(int index) - to get class entry index from a 
>> method or field entry index;
>>
>> I removed previously added method
>> getInvokedynamicRefInfoAt(int index) - as it is no more needed now.
>>
>> New webrev:
>> Webrev hotspot: 
>> http://cr.openjdk.java.net/~kshefov/8141615/hotspot/webrev.01
>> Webrev jdk: http://cr.openjdk.java.net/~kshefov/8141615/jdk/webrev.01
>>
>> Thanks
>> -Konstantin
>>
>> On 11/18/2015 02:11 PM, Konstantin Shefov wrote:
>>> Remi,
>>>
>>> Thanks for reviewing. Your suggestion sounds sensibly.
>>> May be it also has sense to make a method 
>>> "getMethodRefNameAndTypeIndex(int index)" to acquire name-and-type 
>>> entry index for methods also.
>>>
>>> -Konstantin
>>>
>>> On 11/18/2015 12:04 AM, Remi Forax wrote:
>>>> Hi Konstantin,
>>>> Technically, getInvokedynamicRefInfoAt should be 
>>>> getNameAndTypeOfInvokedynamicRefInfoAt because it only extract the 
>>>> nameAndType value of the InvokeDynamicRef.
>>>>
>>>> In term of API, I think it's better to decompose the API, i.e. to 
>>>> have a method
>>>>    int getInvokedynamicRefNameAndTypeIndex(int index)
>>>> that returns the nameAndType index and to reuse 
>>>> getNameAndTypeRefInfoAt(index) to get the corresponding array of 
>>>> Strings.
>>>>
>>>> cheers,
>>>> Rémi
>>>>
>>>> ----- Mail original -----
>>>>> De: "Christian Thalinger" <christian.thalinger at oracle.com>
>>>>> À: "Konstantin Shefov" <konstantin.shefov at oracle.com>
>>>>> Cc: "hotspot-dev developers" <hotspot-dev at openjdk.java.net>, 
>>>>> core-libs-dev at openjdk.java.net
>>>>> Envoyé: Lundi 16 Novembre 2015 23:41:45
>>>>> Objet: Re: RFR [9] 8141615: Add new public methods to 
>>>>> sun.reflect.ConstantPool
>>>>>
>>>>> [CC'ing core-libs-dev]
>>>>>
>>>>>> On Nov 13, 2015, at 4:55 AM, Konstantin Shefov
>>>>>> <konstantin.shefov at oracle.com> wrote:
>>>>>>
>>>>>> Hello
>>>>>>
>>>>>> Please review an enhancement to add three new methods to
>>>>>> sun.reflect.ConstantPool class.
>>>>>> The following methods are suggested:
>>>>>>
>>>>>> public String[] getNameAndTypeRefInfoAt(int index) - returns string
>>>>>> representation of name and type from a NameAndType constant pool 
>>>>>> entry
>>>>>> with the specified index
>>>>>>
>>>>>> public String[] getInvokedynamicRefInfoAt(int index) - returns 
>>>>>> string
>>>>>> representation of name and type from an InvokeDynamic constant 
>>>>>> pool entry
>>>>>> with the specified index
>>>>>>
>>>>>> public Tag getTagAt(int index) - returns a Tag enum instance of a 
>>>>>> constant
>>>>>> pool entry with the specified index
>>>>>>
>>>>>> These three methods could be used for testing API working with 
>>>>>> constant
>>>>>> pool, e.g. JVMCI.
>>>>>>
>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8141615
>>>>>> Webrev hotspot:
>>>>>> http://cr.openjdk.java.net/~kshefov/8141615/hotspot/webrev.00
>>>>>> Webrev jdk: 
>>>>>> http://cr.openjdk.java.net/~kshefov/8141615/jdk/webrev.00
>>>>>>
>>>>>> Thanks
>>>>>> -Konstantin
>>>>>
>>>
>>
>