On Wed, 18 Oct 2023 01:40:18 GMT, Alex Menkov <amen...@openjdk.org> wrote:
>> All test cases in getclfld007 had 1 (or 0) field in test classes/interfaces.
>> The change adds several fields in one of the test classes to verify order of
>> the returned fields (as described by GetClassFields spec: "in the order they
>> occur in the class file").
>> Field order in the class file is not guaranteed to be the same as in the
>> source, so information about expected fields and expected order is extracted
>> by ASM (it parses class file sequentially).
>> This allows to drop hardcoded field name/type in native part.
>>
>> Additionally did some test cleanup:
>> - dropped "printdump" stuff (the test always logs reported fields);
>> - removed unused `generic` in native check() method, added deallocation of
>> `name` and `sig`
>
> Alex Menkov has updated the pull request incrementally with one additional
> commit since the last revision:
>
> get field order from class file
test/hotspot/jtreg/vmTestbase/nsk/jvmti/GetClassFields/getclfld007.java line
120:
> 118: return explorer.fieldNameAndSig;
> 119: }
> 120: }
Nit: I'd suggest to make get() an instance method. It will be a little bit
simpler and more natural.
It wiill look as below:
static void check(Class cls) throws Exception {
FieldExplorer explorer = new FieldExplorer(cls);
List<String> fields = explorer.get();
check(cls, fields.toArray(new String[0]));
}
. . .
List<String> get() throws Exception {
System.out.println("Class " + cls.getName());
try (InputStream classBytes = getClassBytes()) {
ClassReader classReader = new ClassReader(classBytes);
classReader.accept(this, 0);
}
return fieldNameAndSig;
}
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16131#discussion_r1364717745
