RFR: 8313422: test/langtools/tools/javac 147 test classes uses com.sun.tools.classfile library

[Adam Sotona]

On Tue, 1 Aug 2023 21:01:51 GMT, Qing Xiao <duke at openjdk.org> wrote:

> There are 147 tests to convert to the new Class-File API.

It is huge piece of work and it looks good.
Thanks!

test/langtools/tools/javac/annotations/typeAnnotations/referenceinfos/Driver.java line 201:

> 199:                     table.add(TypeAnnotation.LocalVarTargetInfo.of(null, null, d.lvarIndex()[idx]));
> 200:                 p = TypeAnnotation.TargetInfo.ofResourceVariable(table);
> 201:             }

Filling missing labels with null is not ideal.
All such unsupported cases should directly throw an exception to indicate invalid state of the test.
BTW silent acceptance of null labels by TypeAnnotation.TargetInfo factory methods is a bug.

test/langtools/tools/javac/diags/CheckResourceKeys.java line 501:

> 499:             ClassModel cm = Classfile.of().parse(in.readAllBytes());
> 500:             for (int i = 1; i < cm.constantPool().entryCount(); ++i) {
> 501:                 if (cm.constantPool().entryByIndex(i) instanceof Utf8Entry entry) {

The same iteration issue as in the RecordCompilationTests

test/langtools/tools/javac/modules/IncubatingTest.java line 276:

> 274:             }
> 275:             cb.with(res);
> 276:         });

This is the same transformation pattern (dropping and then end handler).

test/langtools/tools/javac/modules/JavaBaseTest.java line 249:

> 247:             }
> 248:             cb.with(modAttr2);
> 249:         });

The same case as in OpenModulesTest

test/langtools/tools/javac/modules/OpenModulesTest.java line 257:

> 255:             }
> 256:             classBuilder.with(newModule);
> 257:         });

FYI: this pattern can replaced with:

Classfile.of().transform(miClass, ClassTransform.dropping(ce -> ce instanceof ModuleAttribute).andThen(ClassTransform.endHandler(classBuilder -> classBuilder.with(newModule))));

Positive impact is re-used constant pool from the original classfile and so other attributes are not expanded. However we are missing `Classfile::transformTo` method in the API.

test/langtools/tools/javac/records/RecordCompilationTests.java line 1280:

> 1278:                     ClassModel classFile = Classfile.of().parse(fileEntry.toPath());
> 1279:                     for (int i = 1; i < classFile.constantPool().entryCount(); ++i) {
> 1280:                         if (classFile.constantPool().entryByIndex(i) instanceof FieldRefEntry fieldRefEntry) {

I've identified iteration over CP entries is problematic in the Classfile API yet, see:
https://mail.openjdk.org/pipermail/classfile-api-dev/2023-August/000399.html

Here the int i must be incremented by the entry::width or the iteration will fail on double-slot entries (long and double).

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

PR Comment: https://git.openjdk.org/jdk/pull/15112#issuecomment-1702760128
PR Review Comment: https://git.openjdk.org/jdk/pull/15112#discussion_r1313007137
PR Review Comment: https://git.openjdk.org/jdk/pull/15112#discussion_r1291109139
PR Review Comment: https://git.openjdk.org/jdk/pull/15112#discussion_r1291120151
PR Review Comment: https://git.openjdk.org/jdk/pull/15112#discussion_r1288215435
PR Review Comment: https://git.openjdk.org/jdk/pull/15112#discussion_r1288212904
PR Review Comment: https://git.openjdk.org/jdk/pull/15112#discussion_r1291100895