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

[Konstantin Shefov]

Hi Igor

I have filed an RFE you asked about 
https://bugs.openjdk.java.net/browse/JDK-8145297.

-Konstantin

On 12/12/2015 05:10 AM, Igor Ignatyev wrote:
> Hi Konstantin,
>
> the fix and tests look good to me, but I think you have to wait for OK from Coleen.
>
> One minor thing —it looks like there are no tests for other s.r.CP methods, could you please file an RFE against core-libs/j.l.reflect to cover them?
>
> Thanks,
> — Igor
>
>> On Dec 11, 2015, at 2:54 PM, Konstantin Shefov <konstantin.shefov at oracle.com> wrote:
>>
>> Hello
>>
>> Please review the new version on the patch.
>>
>> New webrev:
>> Webrev hotspot: http://cr.openjdk.java.net/~kshefov/8141615/hotspot/webrev.02
>> Webrev jdk: http://cr.openjdk.java.net/~kshefov/8141615/jdk/webrev.02
>>
>> What has been changed:
>> 1. Added tests for the new added methods.
>> 2. Removed CP tag codes 100 - 105 from being passed to java and left only codes from the open JVM spec (https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.4-140).
>>
>> Thanks
>> -Konstantin
>>
>> On 11/27/2015 07:48 PM, Konstantin Shefov wrote:
>>> 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