<div dir="ltr"><div dir="auto">Thanks for the peek. Omissions do happen a lot, but unfortunately many of them are hardly detected until the code is actually used in production. For example, I discovered the writeTo omission when I am using the API to patch class files to remove some attributes to test core reflection behavior, and same for SirYwell for the MethodParameters nullable name issue. </div><div dir="auto"><br></div><div dir="auto">Adam has already reviewed all these patches. Unfortunately, since Adam is not a JDK reviewer, these patches still need a JDK reviewer's review to be eligible for integration. Brian, as a reviewer, would you do us a favor? Especially for the first 3, since it currently impedes the regular usage of Classfile API and has no workaround.<div dir="auto"><br></div><div dir="auto">Chen</div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Apr 4, 2023, 10:49 AM Brian Goetz <<a href="mailto:brian.goetz@oracle.com" target="_blank">brian.goetz@oracle.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">I have been reviewing API and structural changes, Adam can review code changes.  I went through your list:<br>
<br>
> Since the Classfile API was added, a few patches have been submitted,<br>
> fixing various bugs in the API. Since Classfile API has no reviewer<br>
> closely tied to it, I wish JDK reviewers on this list can take a peek<br>
> at the following patches to facilitate the bug fixes to the Classfile<br>
> API:<br>
> <br>
> <a href="https://bugs.openjdk.org/browse/JDK-8304937" rel="noreferrer noreferrer" target="_blank">https://bugs.openjdk.org/browse/JDK-8304937</a> BufferedFieldBuilder.Model<br>
> missing writeTo(DirectClassBuilder)<br>
> <a href="https://github.com/openjdk/jdk/pull/13187" rel="noreferrer noreferrer" target="_blank">https://github.com/openjdk/jdk/pull/13187</a><br>
<br>
Yes, this does look like a straightforward omission to me.  Adam can review; please also check for similar omissions (other buffered builders, other kinds of elements.)<br>
<br>
<br>
> <a href="https://bugs.openjdk.org/browse/JDK-8304837" rel="noreferrer noreferrer" target="_blank">https://bugs.openjdk.org/browse/JDK-8304837</a> Classfile API throws IOOBE<br>
> for MethodParameters attribute without parameter names<br>
> <a href="https://github.com/openjdk/jdk/pull/13167" rel="noreferrer noreferrer" target="_blank">https://github.com/openjdk/jdk/pull/13167</a><br>
<br>
Yes, this is just a bug.  JVMS 4.7.24 allows nameIndex to be zero.<br>
<br>
> <a href="https://bugs.openjdk.org/browse/JDK-8304148" rel="noreferrer noreferrer" target="_blank">https://bugs.openjdk.org/browse/JDK-8304148</a> Remapping a class with<br>
> Invokedynamic constant loses static bootstrap arguments<br>
> <a href="https://github.com/openjdk/jdk/pull/13021" rel="noreferrer noreferrer" target="_blank">https://github.com/openjdk/jdk/pull/13021</a><br>
<br>
Yes, seems like a straightforward omission.<br>
<br>
> <a href="https://bugs.openjdk.org/browse/JDK-8304031" rel="noreferrer noreferrer" target="_blank">https://bugs.openjdk.org/browse/JDK-8304031</a> Classfile API cannot<br>
> encode Primitive Class as Condy<br>
> <a href="https://github.com/openjdk/jdk/pull/12996" rel="noreferrer noreferrer" target="_blank">https://github.com/openjdk/jdk/pull/12996</a><br>
<br>
Looks reasonable.<br>
<br>
<br>
So all of these look like straightforward patches; no objections from a specification or API perspective.  Please work with Adam for review, test coverage, etc.  <br>
<br>
<br>
</blockquote></div>