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