RFR: 8372948: Store end positions directly in JCTree [v6]
Maurizio Cimadamore
mcimadamore at openjdk.org
Mon Jan 26 11:22:57 UTC 2026
On Thu, 22 Jan 2026 11:55:15 GMT, Liam Miller-Cushon <cushon at openjdk.org> wrote:
>> This change adds a field to `JCTree` to store end positions, instead of using a separate `EndPosTable` map. See also [this compiler-dev@ thread](https://mail.openjdk.org/pipermail/compiler-dev/2025-November/032254.html).
>>
>> I performed the refactoring in stages, preserving existing semantics at each step.
>>
>> There are two known places where this changes existing behaviour that are reflected in changes to tests:
>>
>> * `test/langtools/tools/javac/api/TestJavacTask_Lock.java` - this test asserts that calling `JavacTask#parse` first and then calling `#call` or `#parse` second will fail. The assertion that the test is currently expecting is thrown when the `EndPosTable` gets set a second time, and this change means that no longer results in an exception. If desired `JavacTask#parse` could be updated to explicitly check if it is called twice and fail, instead of indirectly relying on the `EndPosTable` for that.
>>
>> * `test/langtools/tools/javac/diags/DiagnosticGetEndPosition.java` - there's a comment that 'ideally would be "0", but the positions are not fully set yet', and with the new approach the end position is available to the test, so it resolves the comment. Also the test logic didn't handle platform specific line ending variations correctly, so I updated it to work on windows now that end positions are present.
>
> 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
src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java line 2058:
> 2056: T result = super.translate(tree);
> 2057: if (result != null && result != tree) {
> 2058: result.endpos = tree.endpos;
No more ad-hoc end pos management stuff like `replaceTree` :-)
src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java line 113:
> 111: private Names names;
> 112:
> 113: protected int errorEndPos = Position.NOPOS;
Using a simple field for error end pos makes the code more readable!
src/jdk.compiler/share/classes/com/sun/tools/javac/parser/ParserFactory.java line 92:
> 90: }
> 91:
> 92: public JavacParser newParser(CharSequence input, boolean keepDocComments, boolean keepLineMap) {
Unrelated to this PR, but I wonder how much value there is in having two different factories here where the only difference is the omission of a trailing boolean param
src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeInfo.java line 675:
> 673: case PREINC:
> 674: case PREDEC:
> 675: return getEndPos(((JCOperatorExpression) tree).getOperand(RIGHT));
Is this big switch still needed? When would an end position not be set in this new world?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28610#discussion_r2727231890
PR Review Comment: https://git.openjdk.org/jdk/pull/28610#discussion_r2727229660
PR Review Comment: https://git.openjdk.org/jdk/pull/28610#discussion_r2727223961
PR Review Comment: https://git.openjdk.org/jdk/pull/28610#discussion_r2727225292
More information about the compiler-dev
mailing list