On Mon, 2 Oct 2023 13:40:40 GMT, Maurizio Cimadamore <mcimadam...@openjdk.org>
wrote:
>> Adam Sotona has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> fixed javadoc typo
>
> src/java.base/share/classes/java/lang/classfile/components/CodeRelabeler.java
> line 71:
>
>> 69: * Creates a new instance of CodeRelabeler using provided {@link
>> java.util.function.BiFunction}
>> 70: * to re-label the code.
>> 71: * @param mapFunction function remapping labels
>
> I found this a bit hard to read. Maybe expand to "function for remapping
> labels in the source code model", or something similar
Fixed, thanks.
> src/java.base/share/classes/java/lang/classfile/constantpool/ConstantPoolBuilder.java
> line 158:
>
>> 156: */
>> 157: default ClassEntry classEntry(ClassDesc classDesc) {
>> 158: if (requireNonNull(classDesc).isPrimitive()) {
>
> It is not clear to me as to whether all the methods in the API deal with null
> correctly. What we did for FFM, assuming that the classfile API is also
> null-hostile by default, was to add a single test which would try to call all
> API methods and looking for NPEs:
>
> https://github.com/openjdk/jdk/blob/2637e8ddc4ffe102418139f501fc0be8e9c5317b/test/jdk/java/foreign/TestNulls.java#L80
>
> This test has saved us many times when adding/changing API methods.
Good point, thanks.
I've create new RFE https://bugs.openjdk.org/browse/JDK-8317356 because the
test would not be so simple to cover all ClassFile API in different situations
(for example different builders implementations based on the actual context).
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15706#discussion_r1342777997
PR Review Comment: https://git.openjdk.org/jdk/pull/15706#discussion_r1342775179
