On Tue, 14 Apr 2026 15:46:24 GMT, Alan Bateman <[email protected]> wrote:
>> David Beaumont has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Rename ModuleReference to ModuleLink
>> - tweak comments
>
> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageStringsWriter.java
> line 51:
>
>> 49: reserveString("class", ImageStrings.CLASS_STRING_OFFSET);
>> 50: reserveString("modules", ImageStrings.MODULES_STRING_OFFSET);
>> 51: reserveString("packages", ImageStrings.PACKAGES_STRING_OFFSET);
>
> At some point it would be good if we could refactor this to avoid the
> "this-escape" (the issue pre-dates your proposed change because of
> addString). A static helper method could be used to add the mapping before
> "this.stringToOffsetMap" is set.
It's a fiddle to this inside the constructor because it's also using the stream
in there.
The neatest way to do it is just to make the constructor private and use a
static factory method to construct the empty writer and then reserve the
strings. But this is basically identical in terms of operations performed to
the code that's there now.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/29414#discussion_r3092557924
