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
