On Mon, 3 Jul 2023 20:00:40 GMT, Mandy Chung <mch...@openjdk.org> wrote:
>> Oliver Kopp has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - Fix test
>> - Fix ArrayList initialization and off-by-one errors
>
> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java
> line 543:
>
>> 541: private static final int MAX_LOCAL_VARS = 256;
>> 542:
>> 543: private final int BUILDER_VAR = MAX_LOCAL_VARS + 1;
>
> why this one can't use the next available local variables? i.e. if no
> splitting, 1 is the next available slot (btw it looks like using 0 in the
> current implementation is a bug because this is an instance method). If
> module descriptors are done in multiple helper methods, 3 is the next
> available slot.
I tried that multiple times and always failed. With following hack, I
"debugged" the value of nextLocalVar at the beginning of the split:
Files.writeString(Path.of("C:\\temp\\test.log"), "nextlocavar" +
nextLocalVar, StandardOpenOption.CREATE, StandardOpenOption.APPEND);
The values are always 4. I did not dare to overwrite these "magic" slots 1, 2,
3. Despite using IntelliJ's call hierarchy feature, I could not find out where
in the code this happens or for what reasons.
I moved `BUILDER_VAR` and `DEDUP_LIST_VAR` two "away".
I think, using `0` was "intentional", because `this` was not needed in the
method and thus, this slot could be reused.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14408#discussion_r1251281097
