<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">On 3/26/13 5:17 PM, Coleen Phillimore
wrote:<br>
</div>
<blockquote cite="mid:51523A83.1070703@oracle.com" type="cite">
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
<br>
<div class="moz-cite-prefix">On 03/26/2013 07:24 PM, <a
moz-do-not-send="true" class="moz-txt-link-abbreviated"
href="mailto:serguei.spitsyn@oracle.com">serguei.spitsyn@oracle.com</a>
wrote:<br>
</div>
<blockquote cite="mid:51522E2C.5010206@oracle.com" type="cite">
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
<div class="moz-cite-prefix">Copy usually means copying values
by pointers.<br>
Not clear what's wrong with the "set_annotations" as there is
no such function yet.<br>
</div>
</blockquote>
<br>
<br>
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.<br>
</blockquote>
<br>
Agreed, it'd be confusing too.<br>
Thank you for the clarification!<br>
<br>
Thanks,<br>
Serguei<br>
<br>
<blockquote cite="mid:51523A83.1070703@oracle.com" type="cite"> <br>
<blockquote cite="mid:51522E2C.5010206@oracle.com" type="cite">
<div class="moz-cite-prefix"> Probably, you find it confusing
too.<br>
But I'm not insisting on the copy_annotation_pointers()
either. :)<br>
Let's keep it as it is.<br>
</div>
</blockquote>
<br>
Thanks for the code review!<br>
Coleen<br>
<br>
<blockquote cite="mid:51522E2C.5010206@oracle.com" type="cite">
<div class="moz-cite-prefix"> <br>
Thanks,<br>
Serguei<br>
<br>
<br>
On 3/26/13 3:58 PM, Coleen Phillimore wrote:<br>
</div>
<blockquote cite="mid:51522828.6020003@oracle.com" type="cite">
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
<br>
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.<br>
<br>
Coleen<br>
<br>
<div class="moz-cite-prefix">On 03/26/2013 07:00 PM, <a
moz-do-not-send="true" class="moz-txt-link-abbreviated"
href="mailto:serguei.spitsyn@oracle.com">serguei.spitsyn@oracle.com</a>
wrote:<br>
</div>
<blockquote cite="mid:5152287F.9040801@oracle.com" type="cite">
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
<div class="moz-cite-prefix">Coleen,<br>
<br>
This does not look like a clone or copy, it just sets the
value?<br>
<meta http-equiv="content-type" content="text/html;
charset=ISO-8859-1">
<pre><meta http-equiv="content-type" content="text/html; charset=ISO-8859-1"><span class="new"> 366 void ConstMethod::copy_annotations(ConstMethod* cm) {</span>
<span class="new"> 367 if (cm->has_method_annotations()) {</span>
<span class="new"> 368 assert(has_method_annotations(), "should be allocated already");</span>
<span class="new"> 369 set_method_annotations(cm->method_annotations());</span>
<span class="new"> 370 }
...
</span></pre>
<span class="new">Do we have to actually clone the
annotations?</span><br>
<span class="new">If not, then the name "copy_annotations"
is wrong.</span><br>
<span class="new">It must be "set_annotations".</span><br>
<span class="new"></span><br>
The test fixes look Ok.<br>
<br>
<span class="new">Thanks,</span><br>
<span class="new">Serguei</span><br>
<br>
<br>
On 3/26/13 3:11 PM, Coleen Phillimore wrote:<br>
</div>
<blockquote cite="mid:51521D22.4060508@oracle.com"
type="cite">Summary: Neglected to copy the annotations in
clone_with_new_data when they were moved to ConstMethod. <br>
<br>
open webrev at <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ecoleenp/8009531/">http://cr.openjdk.java.net/~coleenp/8009531/</a>
<br>
bug link at <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://bugs.sun.com/view_bug.do?bug_id=8009531">http://bugs.sun.com/view_bug.do?bug_id=8009531</a>
<br>
<br>
Also. please review JDK test modified to test that this
crash is fixed (will check in in two weeks). <br>
<br>
open webrev at <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ecoleenp/8009531_jdk">http://cr.openjdk.java.net/~coleenp/8009531_jdk</a>
<br>
<br>
Thanks, <br>
Coleen <br>
<br>
<br>
<br>
<br>
<br>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>