RFR: 8350212: Track source end positions of declarations that support @SuppressWarnings [v4]

[Archie Cobbs]

On Thu, 10 Apr 2025 16:30:03 GMT, Jan Lahoda <jlahoda at openjdk.org> wrote:

>> Archie Cobbs has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Refactoring/cleanup for handling of ending positions.
>
> In the sketch I was doing a few days back, I was peeking at forcing some kind of `EndPosTable` argument to `TreeInfo.endPos`, but I shied away as it seemed to complicate things. Here I see it is not as bad as I thought, but still it spreads a bit.
> 
> I see the value in avoiding special field and code path for the AST nodes for which the end pos is mandatory, in addition to the optional EndPosTable.
> 
> I think I would still slightly prefer the specialized fields, esp. if the impact on mostly unrelated outside code (like TransPatterns and Gen) could be eliminated. I would like to investigate if we can drop the EndPosTable altogether, although that is unlikely to happen before JDK 25 RPD1 (maybe 2). But, there is, of course, a possibility that the removal of EndPosTable won't happen, at which point we would have the duplication.
> 
> But, I can live with a patch long the lines here adding EndPosTable to `TreeInfo.endPos` if needed. Should we drop the EndPosTable as some point, we can drop the new EndPosTable argument. If not, we would preserve it. It is not what I would do, but I don't think either of the solutions is clearly wrong.

@lahodaj, thanks for your thoughts.

I agree it's not ideal either way. The thing that kindof pushes it in the `EndPosTable` direction for me is this: As long as we have `DiagnosticListener`'s and `-Xjcov`, we have a (sometimes) requirement to track the ending positions of _every_ node. So as long as we are required to have code that does that, why not just use it? Then we have only one mechanism for doing something, instead of two.

Practically speaking: @mcimadamore are you OK to proceed with the current approach?

In any case, it's not a tragedy if we change our minds in the future - removing `EndPosTable` or switching back to the previous approach is easy to do (and I'm happy to volunteer if needed).

Thanks.

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

PR Comment: https://git.openjdk.org/jdk/pull/23669#issuecomment-2795122929