On Wed, 8 Oct 2025 11:35:15 GMT, David Beaumont <[email protected]> wrote:
>> src/java.base/share/native/libjimage/imageFile.hpp line 253: >> >>> 251: // Set on a "normal" (non-preview) location if a preview >>> version of >>> 252: // it exists in the same module. >>> 253: FLAGS_HAS_PREVIEW_VERSION = 0x1, >> >> To avoid confusion, the value and spelling of the flag should agree across >> all modules. >> In ImageLocation and jimage/ModulesReference. > > ModulesReference are different flags, with different semantics. That's why I > made sure they were private and provided helper methods for reading/writing > the package directory offsets (because I did trip over using the wrong > constants at one point). > > I agree it's weird/annoying to have this similar-but-different set of flags, > but the ModuleReference flags are completely encapsulated in one place, so > cannot be accidentally confused with, or misused in place of the > ImageLocation flags. > > These flags have a requirement of being zero for almost all entries to reduce > image file size, while the ModulesReference flags need to additive for > merging (and are not stored in the same part of the jimage file). > > I could change flag ordering to make the values of the "has preview version" > flags match, but since these sets of flags must never be mistaken for each > other, I'm not sure that's beneficial (if anything it might foster the idea > that they can be used interchangeably in some way). If they have the same spelling, they will cause confusion if they have different semantics and usages. All of imageFile, jimage, and the file system provider and classloader are in a single scope of operation. ------------- PR Review Comment: https://git.openjdk.org/valhalla/pull/1618#discussion_r2417809579
