<div dir="ltr">Thanks for the review,<div><br></div><div>The update change is at: <a href="http://cr.openjdk.java.net/~cushon/8171132/webrev.01">http://cr.openjdk.java.net/~cushon/8171132/webrev.01</a><br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF"><pre>should probably be replaced with:<br></pre><pre><span class="gmail-m_-3832265415287047242new">Assert.check(var.type.tsym == syms.stringType.tsym)</span></pre></div></blockquote><div>Done.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF">On a related note, your convertType routine seems weak - it does all
    the required conversions - but without actively checking ranges and
    throwing errors if something bad is detected (e.g. a 'byte' field,
    whose constant value is 1000). Instead, I believe your code just
    truncates the part in excess, which, while acceptable (after all,
    the creator of such a bad classfile gets what he/she deserves) -
    could lead to some painful debugging; so an eager check/error would
    be preferrable, I think.<br></div></blockquote><div><br></div><div>I may have misunderstood Alex's message here: <a href="http://mail.openjdk.java.net/pipermail/compiler-dev/2016-November/010500.html">http://mail.openjdk.java.net/pipermail/compiler-dev/2016-November/010500.html</a></div><div><br></div>> javac must not reject Lib.class on this basis. However, javac should handle an out-of-band value in the CONSTANT_Integer c.p. entry as a quality-of-implementation detail.</div><div class="gmail_quote"><br></div><div class="gmail_quote">I agree that truncating could be surprising and hard to debug. So if an error is permitted by the spec I'm happy to make the suggested change, and the details you suggested sound good to me.</div></div></div></div>