On Mon, 21 Jul 2025 11:41:21 GMT, David Beaumont <[email protected]> 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
