On Mon, 26 Jan 2026 16:29:45 GMT, Maurizio Cimadamore <[email protected]> 
wrote:

>> I think it's ok to start simple and refine as we go. As part of this change 
>> it would be perhaps helpful to at least simplify the method so that 
>> "unreachable cases" are removed?
>
> (longer term, (as discussed offline with @lahodaj ) I think it would be nice 
> to have end positions set up correctly by parser for all trees, since now it 
> doesn't cost extra to do so)

> As part of this change it would be perhaps helpful to at least simplify the 
> method so that "unreachable cases" are removed?

I double checked that `test/langtools/tools/javac/tree/TreePosTest.java` is 
exercising all of the cases in the switch here except for `TOPLEVEL` and 
`ERRONEOUS`, which I think my be gaps in test coverage rather than truly dead 
code. Do you see more unreachable cases here that I'm missing?

> (longer term, (as discussed offline with @lahodaj ) I think it would be nice 
> to have end positions set up correctly by parser for all trees, since now it 
> doesn't cost extra to do so)

That sounds like a good direction to me!

I was also wondering how feasible it would be to make the `pos`/`endpos` fields 
field final, instead of having the parser set them after the instances are 
constructed, but I haven't looked closely at how much refactoring that would 
take.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/28610#discussion_r2728355597

Reply via email to