RFR: 8360037: Refactor ImageReader in preparation for Valhalla support [v12]

[Jaikiran Pai]

On Mon, 21 Jul 2025 11:41:21 GMT, David Beaumont <duke at openjdk.org> wrote:

>> src/java.base/share/classes/jdk/internal/jimage/ImageReader.java line 543:
>> 
>>> 541:         // As such, non-directory nodes are always complete.
>>> 542:         boolean isCompleted() {
>>> 543:             return true;
>> 
>> Seems like a risky default to have this be a concrete method/default that is not required to be overridden.
>> A subclass (not that many here) could forget to override and have a default wrong answer. YMMV.
>
> Hmm, fair. The protected constructor only exists because of ExplodedImage (and I had hoped to be able to get rid of that completely with this work, but sadly couldn't). I added a clear warning to not create other subtypes. I could move this to an abstract method, but honestly I don't think it fixes anything since "completeness" is a concept that only makes sense internally for the implementation in this class (it's package access, so cannot be seen elsewhere). Making it abstract would mean that it needs to be more visible, which I dislike.

In addition to what Roger noted, the default implementation of this `isCompleted()` method and the default implementation of `getLocation()` method a few lines below seem to contradict each other. `isCompleted()` by default returns `true` implying a resource node type but `getLocation()` by default throws an exception, implying a non-resource node type.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/26054#discussion_r2247917086