On Sun, 29 Mar 2026 10:39:43 GMT, Alan Bateman <[email protected]> wrote:
>> David Beaumont has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 12 commits: >> >> - Merge branch 'master' into jimage_preview_mode >> - rename jimage_exists to jimage_is_open >> - Feedback tweaks >> - Feedback tweaks >> - More feedback tweaks >> - Updated copyright >> - Feedback changes >> - Merge branch 'master' into jimage_preview_mode >> - Merge branch 'master' into jimage_preview_mode >> - undo exploded image changes for now >> - ... and 2 more: https://git.openjdk.org/jdk/compare/7695b1f9...0e802079 > > src/java.base/share/classes/jdk/internal/jimage/ModuleReference.java line 37: > >> 35: >> 36: /** >> 37: * Represents the module entries stored in the buffer of {@code >> "/packages/xxx"} > > Can we rename this to something like a ModuleRef (to avoid confusion with > j.l.module.ModuleReference at the use-sites) and improve the class > description to make it clear why it is needed. Right now it says it > references "module entries" but it's more of a way to easily check if a > module has resources or what its flags are. > > I'm also a bit puzzled by the static method readNameOffsets as it feels out > of place, doesn't use ModuleReference. The class doesn't say it "*references* module entries", it says (cut & paste from the docs) it "*Represents* the module entries...". It the representation of the data from which the nodes you see inside package subdirectories are created (which is handled differently to all other nodes). I tweaked to comments a bit, but since the class renaming is significant, I'd like to see if we can make sure we're happy with any new name before I do it. `readNameOffsets` uses the encapsulated constants. If it moved then the constants must either be made public or duplicated. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/29414#discussion_r3008507821
