<html><head><meta http-equiv="content-type" content="text/html; charset=utf-8"></head><body dir="auto"><div><br></div><div>On Oct 4, 2012, at 5:12 PM, John Rose &lt;<a href="mailto:john.r.rose@oracle.com">john.r.rose@oracle.com</a>&gt; wrote:<br><br></div><blockquote type="cite"><div><div><div>On Oct 4, 2012, at 3:51 PM, Christian Thalinger wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><a href="http://cr.openjdk.java.net/~twisti/8000263">http://cr.openjdk.java.net/~twisti/8000263</a><br><br>8000263: JSR 292: signature types may appear to be unloaded</blockquote><br></div><div>Good work. &nbsp;I very much like the 'is_public' assertion in 'check_wk_pre_link_klasses'.</div><div><br></div><div>I mechanically checked that the whitespace change in systemDictionary.hpp is mixed with no other effects,</div><div>besides changing some occurrences of Pre or Pre_JSR292 to Pre_Link or Pre?</div><div><br></div><div>Consider changing Opt to Pre for LambdaForm, if you agree that is a reasonable cleanup. &nbsp;That type is no longer optional.</div></div></blockquote><div><br></div>I tried that but running Queens fails then because we are still using 7 as JDK.<div><br><blockquote type="cite"><div><div><br></div>I think the following code would be simpler and more directly correct in 'sharpen_unsafe_type':<div>&nbsp; &nbsp;if (sharpened_klass != NULL &amp;&amp; !sharpened_klass-&gt;is_loaded())</div><div><div>&nbsp; &nbsp; &nbsp;return NULL;</div></div></div></blockquote><div><br></div>Hmm. &nbsp;Returning null is okay here? &nbsp;Have to take a look again.</div><div><br><blockquote type="cite"><div><div><br class="Apple-interchange-newline">Issue: &nbsp;Are intrinsics still properly recognized, even though 'find_well_known_klass' is restricted?</div></div></blockquote><div><br></div>Why wouldn't they? &nbsp;But I will check that.</div><div><br><blockquote type="cite"><div><div><br></div><div>The function vmIntrinsics::method_for bothers me still.</div><div>Maybe add a FIXME comment pointing out that it doesn't work for all intrinsics. &nbsp;Or, add a boolean parameter:</div><div><br></div><div>&nbsp;&nbsp;Klass* SystemDictionary::find_well_known_klass(Symbol* class_name, bool link_all = false) {</div><div>&nbsp; &nbsp; ... if (option == Pre_Link ||&nbsp;link_all) ...</div><div>&nbsp; }</div><div><br></div><div>Or, find a way to get rid of&nbsp;vmIntrinsics::method_for, since it is not used much.</div></div></blockquote><div><br></div>Ah, forgot about it. &nbsp;I will remove it and send a new webrev.<div><br></div><div>-- Chris</div><div><br><blockquote type="cite"><div><div><br></div><div>Thanks,</div><div>— John</div></div></blockquote></div></div></body></html>