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

[Christian Thalinger]

> On Dec 11, 2015, at 1:54 AM, 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 <http://cr.openjdk.java.net/~kshefov/8141615/jdk/webrev.02>

These newlines look ridiculous especially when it’s not even needed:

+  // Returns a class reference index for a method or a field.
+  public int      getClassRefIndexAt
+                                     (int index) { return getClassRefIndexAt0 (constantPoolOop, index); }

Either fix the indent or just add them like regular methods should look like.

> 
> 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
>>>>>>> 
>>>>> 
>>>> 
>>> 
>> 
>