On Thu, 2 Mar 2023 14:31:06 GMT, Adam Sotona <asot...@openjdk.org> wrote:

>> This is root pull request with Classfile API implementation, tests and 
>> benchmarks initial drop into JDK.
>> 
>> Following pull requests consolidating JDK class files parsing, generating, 
>> and transforming 
>> ([JDK-8294957](https://bugs.openjdk.org/browse/JDK-8294957)) will chain to 
>> this one.
>> 
>> Classfile API development is tracked at:
>> https://github.com/openjdk/jdk-sandbox/tree/classfile-api-branch
>> 
>> Development branch of consolidated JDK class files parsing, generating, and 
>> transforming is at:
>> https://github.com/openjdk/jdk-sandbox/tree/classfile-api-dev-branch
>> 
>> Classfile API [JEP](https://bugs.openjdk.org/browse/JDK-8280389) and [online 
>> API 
>> documentation](https://htmlpreview.github.io/?https://raw.githubusercontent.com/openjdk/jdk-sandbox/classfile-api-javadoc-branch/doc/classfile-api/javadoc/java.base/jdk/internal/classfile/package-summary.html)
>>  is also available.
>> 
>> Please take you time to review this non-trivial JDK addition.
>> 
>> Thank you,
>> Adam
>
> Adam Sotona has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   StackMapFrameInfo extracted to top level from StackMapTableAttribute

src/java.base/share/classes/jdk/internal/classfile/impl/AbstractInstruction.java
 line 156:

> 154:         @Override
> 155:         public String toString() {
> 156:             return String.format("Store[OP=%s, slot=%d]", this.opcode(), 
> slot());

A suggestion. I believe the `toString` output for bound and unbound 
instructions should be identical and it can composed solely from methods on the 
public instruction interface. There are some differences e.g. `Increment` and 
`Inc` for the unbound and bound increment instruction respectively.

If those assumptions are correct i recommend placing a static `toString` method 
on all unbound instructions that also have bound instructions, accepting the 
public instruction interface as an argument. Then the overridden 
`Object::toString` method defers to those. Thereby there is no duplication.
(Alas we cannot override `toString` on the instruction interface itself).

-------------

PR: https://git.openjdk.org/jdk/pull/10982

Reply via email to