On Wed, 2 Jul 2025 17:33:40 GMT, Ioi Lam <ik...@openjdk.org> wrote:
> A module has both a Java and a C++ representation
>
> - C++: `ModuleEntry`
> - Java: `java.lang.Module`
>
> In C++, we have the following two methods
>
> - `ModuleEntry* InstanceKlass::module()`
> - `oop ModuleEntry::module()`
>
> This can lead to confusing code like this:
>
>
> InstanceKlass* ik = ...;
> oop module = ik->module()->module()
>
>
> Proposal:
>
> - Leave `InstanceKlass::module()` as is -- there's another function with the
> same style: `PackageEntry* InstanceKlass::package()`
> - Rename `ModuleEntry::module()` to `ModuleEntry::module_oop()`, so the above
> example can be more readable:
>
>
> InstanceKlass* ik = ...;
> oop module = ik->module()->module_oop()
I like this change other than precond(). ModuleEntry vs java.lang.Module oops
is clearer with a different method name. I've been confused by this too.
src/hotspot/share/runtime/reflection.cpp line 555:
> 553: } else {
> 554: oop module_oop = module_to->module_oop();
> 555: precond(module_oop != nullptr);
I don't like precond() when you could have a useful assert message instead.
src/hotspot/share/runtime/reflection.cpp line 582:
> 580: } else {
> 581: oop module_oop = module_from->module_oop();
> 582: precond(module_oop != nullptr);
same here. "Module oop should be non-null in ModuleEntry"
-------------
Marked as reviewed by coleenp (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/26102#pullrequestreview-2980107062
PR Review Comment: https://git.openjdk.org/jdk/pull/26102#discussion_r2180644851
PR Review Comment: https://git.openjdk.org/jdk/pull/26102#discussion_r2180646204
