RFR 8009382: Add JVM_Get{Field|Method}TypeAnnotations

[Rickard Bäckman]

Joel,

the change looks good.

/R

On Mar 26, 2013, at 11:32 AM, Joel Borggrén-Franck wrote:

> Hi,
> 
> Also including build-dev since this touches some makefiles.
> 
> Thanks Dan! see inline, also new webrev:
> 
> http://cr.openjdk.java.net/~jfranck/8009382/hotspot/webrev.02/
> 
> On 03/25/2013 04:17 PM, Daniel D. Daugherty wrote:
>> On 3/22/13 8:16 AM, Joel Borggrén-Franck wrote:
> 
>> make/bsd/makefiles/mapfile-vers-debug
>> make/linux/makefiles/mapfile-vers-debug
>> make/linux/makefiles/mapfile-vers-product
>> make/solaris/makefiles/mapfile-vers
>>     It looks like the other entries are in alpha order so these
>>     two new entries should also be added in alpha order.
>> 
> 
> Fixed. I also noticed I forgot to update one of the bsd makefiles.
> 
>> src/share/vm/prims/jvm.cpp
>>     line 1510: return NULL;
>>         nit: indent should be two spaces
>> 
>>     line 1529: assert(false, "cannot find method");
>>     line 1530: return NULL;  // robustness
>>         Normally "robustness" comments flag logic after an assert()
>>         to indicate what we do in product mode to deal with the "bad"
>>         situation without crashing. In this case, we don't try to use
>>         'm' after discovering that it is NULL so this could be simpler:
>> 
>>             Method* m = InstanceKlass::cast(k)->method_with_idnum(slot);
>>             assert(m != NULL, "cannot find method");
>>             return m;  // caller has to deal with NULL in product mode
>> 
>>          Yes, I realize you only touched the comment here, but it
>>          served to point out the messiness of the existing code.
>> 
> 
> Good suggestion. Fixed.
> 
>>     line 1551: return NULL;
>>     line 1565: return NULL;
>>     line 1579: return NULL;
>>         nit: indent should be two spaces
>> 
>>     line 1632: return NULL;
>>     line 1613: return NULL;
>>         nit: indent should be two spaces
> 
> Fixed all indent nits.
> 
>>> 
>>> A prototype of the jdk changes can be found here:
>>> http://cr.openjdk.java.net/~jfranck/8009382/jdk/webrev.00/
>> 
>> You should use a separate bug ID for the jdk repo changes. This will
>> prevent confusion between when HSX with these changes is integrated
>> into JDK8 and when the jdk repo changes are integrated into JDK8.
>> 
> 
> The JDK changes have a separate bug, sorry if this was unclear. The jdk changes shown are just a proof of concept, intended to highlight that my HotSpot changes have actually been run. I will send out a separate review request in the future for the JDK changes.
> 
> cheers
> /Joel