On Wed, 1 Mar 2023 15:00:41 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 four additional 
> commits since the last revision:
> 
>  - renamed all remaining ConcreteXyzEntry to XyzEntryImpl
>  - abstract implementations of RefEntry, RefsEntry and NamedEntry renamed to 
> AbstractRefEntry, AbstractRefsEntry and AbstractNamedEntry
>  - renamed ConcreteBootstrapMethodEntry to BootstrapMethodEntryImpl
>  - ConcreteEntry renamed to AbstractPoolEntry

src/java.base/share/classes/jdk/internal/classfile/impl/SplitConstantPool.java 
line 167:

> 165:             buf.patchInt(pos + 2, 4, attrLen - 6);
> 166:             buf.patchInt(pos + 6, 2, bsmSize);
> 167:             return true;

The if and else branch return true, factor out at the end of the method?

src/java.base/share/classes/jdk/internal/classfile/impl/SplitConstantPool.java 
line 339:

> 337:     }
> 338: 
> 339:     private<T> AbstractPoolEntry.Utf8EntryImpl tryFindUtf8(int hash, 
> String target) {

Unused type variable `T`

src/java.base/share/classes/jdk/internal/classfile/impl/SplitConstantPool.java 
line 488:

> 486:             return methodHandleEntry(refKind, reference);
> 487:         }
> 488:         return internalAdd(new 
> AbstractPoolEntry.MethodHandleEntryImpl(this, size, hash, refKind, 
> (AbstractPoolEntry.AbstractMemberRefEntry) reference), hash);

Break the long line (same for two following methods).

src/java.base/share/classes/jdk/internal/classfile/impl/TemporaryConstantPool.java
 line 56:

> 54: public final class TemporaryConstantPool implements ConstantPoolBuilder {
> 55: 
> 56:     private TemporaryConstantPool() {};

Suggestion:

    private TemporaryConstantPool() {}

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

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

Reply via email to