RFR: 8345911: Enhance error message when IncompatibleClassChangeError is thrown for sealed class loading failures [v3]
David Holmes
dholmes at openjdk.org
Fri Dec 13 02:37:38 UTC 2024
On Thu, 12 Dec 2024 13:43:32 GMT, Alan Bateman <alanb at openjdk.org> wrote:
>> David Holmes has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Updated additional tests
>
> src/hotspot/share/oops/instanceKlass.cpp line 234:
>
>> 232: "and sealed class %s is in module \"%s\" for loader %s",
>> 233: k->external_name(), k->module()->name_as_C_string(), k->module()->loader_data()->loader_name_and_id(),
>> 234: this->external_name(), this->module()->name_as_C_string(), this->module()->loader_data()->loader_name_and_id());
>
> This looks quite good except that the word "for" in "for loader %s" looks a bit strange. In API docs we speak of "the ClassLoader for a module", not the other way around. Class loaders optionally have a name so I suppose the "loader %s" should put quotes around the name like it does for the module name.
I will change to "with loader %s".
Note that `loader_name_and_id()` already puts (single) quotes around the name of the loader. I will use single-quotes for the module name too.
> src/hotspot/share/oops/instanceKlass.cpp line 240:
>
>> 238:
>> 239: if (!k->is_public() && !is_same_class_package(k)) {
>> 240: ss.print("Failed same run-time package check: non-public subclass %s is in package \"%s\" with classloader %s,"
>
> Since the checking is past the same module check at this point then it might be simpler to say that it fails the same package check.
Ah I see what you are saying. As they are known to be in the same module then they are known to have the same loader. But "runtime package" is only significant for different loaders with the same package name - which can't be the case here. So it is simply not the same package. Will fix.
Thanks for looking at this @AlanBateman !
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22703#discussion_r1883162736
PR Review Comment: https://git.openjdk.org/jdk/pull/22703#discussion_r1883164193
More information about the hotspot-dev
mailing list