On Thu, 22 Jan 2026 11:55:15 GMT, Liam Miller-Cushon <[email protected]> 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
