RFR: 8372948: Store end positions directly in JCTree [v6]
Liam Miller-Cushon
cushon at openjdk.org
Mon Jan 26 13:09:59 UTC 2026
On Mon, 26 Jan 2026 10:13:29 GMT, Jan Lahoda <jlahoda at openjdk.org> 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
More information about the compiler-dev
mailing list