On Thu, 4 Sep 2025 12:53:06 GMT, Paul Hübner <[email protected]> wrote:
>> I added some logging and fixed the tests so that one doesn't rely on a
>> migrated class in java.base to throw a VerifyError (will make a parallel fix
>> to this test in mainline). The other, I changed the logging message that it
>> is looking for. More tests in this area will be added.
>> Tested locally.
>
> src/hotspot/share/classfile/classFileParser.cpp line 6240:
>
>> 6238:
>> _inline_layout_info_array->adr_at(fieldinfo.index())->set_klass(InlineKlass::cast(klass));
>> 6239: log_info(class, preload)("Preloading of class %s during
>> loading of class %s "
>> 6240: "(cause: field type not in
>> LoadableDescriptors attribute) succeeded",
>
> "field type not in LoadableDescriptors" yet "succeeded" seems contradictory.
> Correct me if I'm wrong, but I think successful preloading implies that this
> class was (potentially transitively) in a LoadableDescriptors attribute of
> some class being loaded.
>
> I'm not sure I follow why we suddenly introduce a log message here which is
> never checked in the tests. Could you elaborate?
I couldn't figure out how to write this test (even though we do reach this
code), or rather I didn't want to delay this change while figuring out how to
write a test because it was breaking the CI.
Even though this doesn't technically preload the class, I wanted the log
message to look like the other preloading messages, so kept "Preloading". And
I wanted to keep "field type". How about:
Preloading of class %s during loading of class $s (cause: field type where
LoadableDescriptors attribute is missing) succeeded
Although that's really long. Suggestions welcome!
-------------
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1544#discussion_r2322246353
