> On Dec 11, 2015, at 1:54 AM, Konstantin Shefov <konstantin.she...@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.thalin...@oracle.com>
>>>>>>> À: "Konstantin Shefov" <konstantin.she...@oracle.com>
>>>>>>> Cc: "hotspot-dev developers" <hotspot-...@openjdk.java.net>,
>>>>>>> core-libs-dev@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.she...@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
>>>>>>>
>>>>>
>>>>
>>>
>>
>
