RFR (s): 8009531: Crash when redefining class with annotated method

[serguei.spitsyn at oracle.com]

On 3/26/13 5:17 PM, Coleen Phillimore wrote:
>
> On 03/26/2013 07:24 PM, serguei.spitsyn at oracle.com wrote:
>> Copy usually means copying values by pointers.
>> Not clear what's wrong with the "set_annotations" as there is no such 
>> function yet.
>
>
> There are set_x_annotations and I wouldn't want someone to think 
> set_annotations just sets one sort.  I don't know, it's just opinion.

Agreed, it'd be confusing too.
Thank you for the clarification!

Thanks,
Serguei

>
>> Probably, you find it confusing too.
>> But I'm not insisting on the copy_annotation_pointers() either. :)
>> Let's keep it as it is.
>
> Thanks for the code review!
> Coleen
>
>>
>> Thanks,
>> Serguei
>>
>>
>> On 3/26/13 3:58 PM, Coleen Phillimore wrote:
>>>
>>> It copies the pointers.  I can't change it to set_annotations 
>>> because there are already set_ functions.  O could change to 
>>> copy_annotation_pointers() if you insist.
>>>
>>> Coleen
>>>
>>> On 03/26/2013 07:00 PM, serguei.spitsyn at oracle.com wrote:
>>>> Coleen,
>>>>
>>>> This does not look like a clone or copy, it just sets the value?
>>>>   366 void ConstMethod::copy_annotations(ConstMethod* cm) {
>>>>   367   if (cm->has_method_annotations()) {
>>>>   368     assert(has_method_annotations(), "should be allocated already");
>>>>   369     set_method_annotations(cm->method_annotations());
>>>>   370   }
>>>>   ...
>>>>
>>>> Do we have to actually clone the annotations?
>>>> If not, then the name "copy_annotations" is wrong.
>>>> It must be "set_annotations".
>>>>
>>>> The test fixes look Ok.
>>>>
>>>> Thanks,
>>>> Serguei
>>>>
>>>>
>>>> On 3/26/13 3:11 PM, Coleen Phillimore wrote:
>>>>> Summary: Neglected to copy the annotations in clone_with_new_data 
>>>>> when they were moved to ConstMethod.
>>>>>
>>>>> open webrev at http://cr.openjdk.java.net/~coleenp/8009531/
>>>>> bug link at http://bugs.sun.com/view_bug.do?bug_id=8009531
>>>>>
>>>>> Also. please review JDK test modified to test that this crash is 
>>>>> fixed (will check in in two weeks).
>>>>>
>>>>> open webrev at http://cr.openjdk.java.net/~coleenp/8009531_jdk
>>>>>
>>>>> Thanks,
>>>>> Coleen
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20130326/809b26c2/attachment-0001.html