Classfile API patch review request

- liangchenblue at gmail.com
Tue Apr 4 17:20:46 UTC 2023


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.

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.

Chen

On Tue, Apr 4, 2023, 10:49 AM Brian Goetz <brian.goetz at oracle.com> wrote:

> I have been reviewing API and structural changes, Adam can review code
> changes.  I went through your list:
>
> > Since the Classfile API was added, a few patches have been submitted,
> > fixing various bugs in the API. Since Classfile API has no reviewer
> > closely tied to it, I wish JDK reviewers on this list can take a peek
> > at the following patches to facilitate the bug fixes to the Classfile
> > API:
> >
> > https://bugs.openjdk.org/browse/JDK-8304937 BufferedFieldBuilder.Model
> > missing writeTo(DirectClassBuilder)
> > https://github.com/openjdk/jdk/pull/13187
>
> 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.)
>
>
> > https://bugs.openjdk.org/browse/JDK-8304837 Classfile API throws IOOBE
> > for MethodParameters attribute without parameter names
> > https://github.com/openjdk/jdk/pull/13167
>
> Yes, this is just a bug.  JVMS 4.7.24 allows nameIndex to be zero.
>
> > https://bugs.openjdk.org/browse/JDK-8304148 Remapping a class with
> > Invokedynamic constant loses static bootstrap arguments
> > https://github.com/openjdk/jdk/pull/13021
>
> Yes, seems like a straightforward omission.
>
> > https://bugs.openjdk.org/browse/JDK-8304031 Classfile API cannot
> > encode Primitive Class as Condy
> > https://github.com/openjdk/jdk/pull/12996
>
> Looks reasonable.
>
>
> 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.
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/classfile-api-dev/attachments/20230404/f85093ca/attachment-0001.htm>


More information about the classfile-api-dev mailing list