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

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

Reply via email to