On Mon, 26 Jan 2026 10:13:29 GMT, Jan Lahoda <[email protected]> wrote:
>> Liam Miller-Cushon has updated the pull request with a new target base due
>> to a merge or a rebase. The incremental webrev excludes the unrelated
>> changes brought in by the merge/rebase. The pull request contains seven
>> additional commits since the last revision:
>>
>> - Merge remote-tracking branch 'origin/master' into JDK-8372948
>> - Merge remote-tracking branch 'origin/master' into JDK-8372948
>> - Fix DiagnosticGetEndPosition on windows
>> - Debug DiagnosticGetEndPosition.java on windows
>> - Update assertion for
>> test/langtools/tools/javac/diags/DiagnosticGetEndPosition.java
>> - Merge remote-tracking branch 'origin/master' into JDK-8372948
>> - 8372948: Store end positions directly in JCTree
>
> test/langtools/tools/javac/api/TestJavacTask_Lock.java line 72:
>
>> 70: fm = comp.getStandardFileManager(null, null, null);
>> 71: try {
>> 72: MethodKind first = MethodKind.CALL;
>
> I think it would be good to preserve the behavior (i.e. prevent `parse`
> followed by `call` or `parse`). Otherwise, I am not sure if there may not be
> some a bit surprising effects (like calling `parse`, then `analyze` then
> `parse` again and `analyze` again might lead to some weird errors). If there
> would be a reason to relax this restriction, it would probably merit its own
> PR and own investigation of effects.
Thanks, I had meant to look into this more if there was interest in the overall
change, so this seems like a good time to revisit it.
Currently it works because a repeated call to `PARSE`, or calling `CALL` after
`PARSE`, will fail with `IllegalStateException: endPosTable already set`. I
reverted the workaround in the test and added an explicit check to replace the
dependency on the `EndPosTable`:
+ if (used.get())
+ throw new IllegalStateException();
I also noticed that there's no validation to disallow repeated calls to
`ANALZYE`, I wonder if there should be?
> test/langtools/tools/javac/diags/DiagnosticGetEndPosition.java line 127:
>
>> 125: int line = (int) d.getLineNumber();
>> 126: int col = (int) d.getColumnNumber();
>> 127: String substring = implCode.split("\\R")[line -
>> 1].substring(col - 1, col);
>
> Nit - there could be a check that `d.getEndPosition() - d.getStartPosition()`
> is equal to `1`, that would make the test a bit stricter.
>
> But this is a great change, I think!
Thanks, done!
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28610#discussion_r2727533728
PR Review Comment: https://git.openjdk.org/jdk/pull/28610#discussion_r2727533953
