RFR: 8361292: Rename ModuleEntry::module() to module_oop()

Coleen Phillimore coleenp at openjdk.org
Wed Jul 2 17:54:40 UTC 2025


On Wed, 2 Jul 2025 17:33:40 GMT, Ioi Lam <iklam at 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


More information about the serviceability-dev mailing list