On Fri, 18 Jul 2025 16:43:30 GMT, Roger Riggs <rri...@openjdk.org> 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 one commit:
>>
>> Resync after benchmark.
>
> 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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26054#discussion_r2218946364
