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

[Archie Cobbs]

On Fri, 30 May 2025 10:34:38 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> Archie Cobbs has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains eight commits:
>> 
>>  - Revert TreeInfo.endPos() refactoring except for "bracePos" renaming.
>>  - Merge branch 'master' into JDK-8350212
>>  - Merge branch 'master' into JDK-8350212 to fix conflict.
>>  - Update copyrights.
>>  - Refactoring/cleanup for handling of ending positions.
>>  - Add missing variable decl end position.
>>  - Add end position for variables coming from variableDeclaratorId().
>>  - Track source end positions of declarations that support @SuppressWarnings.
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java line 2063:
> 
>> 2061:             make_at(tree.pos());
>> 2062:             T result = super.translate(tree);
>> 2063:             if (endPosTable != null && result != null && result != tree) {
> 
> I wonder how this extra check for `result != null` was added. I mean, of course calling `replaceTree` with a null parameter seems suspicious -- should we add more formal null checks in the various end pos table methods? (it's ok if we do that in a separate PR)

I added this check to fix NPE's (e.g., in `tools/javac/warnings/suppress/PackageInfo.java`) that started to occur after the refactoring of `EndPosTable.replaceTree()`, which assumes that the replacement is not null.

Of couse a null node can't have an ending position, but it does happen sometimes that nodes are replaced with nulls. In that case, it makes sense for `replaceTree()` to just remove the old tree from `EndPosTable`. I've updated the patch accordingly in 205f77e9d53.

> Why was this added to the interface? 

It wasn't strictly necessary, but as long as `EndPosTable` is going to be a proper interface, it seemed like the right thing to do.

The other cleanups were based on the following observations:

1. There is no need for the `EndPosTable` to refer back to the parser. This reference was so it could have access to `parser.S` and `parser.token` in order to infer the ending position for the node in `to()` and `toP()`, but that calculation should just be done by the parser itself, before ever accessing `EndPosTable` (via what is now a single method, `storeEnd()`). The `EndPosTable` should simply be a standalone repository for the mappings from node to ending position. But really this change made the new subclass `MinimalEndPosTable` more straightforward to implement, which is what prompted the refactoring in the first place.
1. Having `attach()` and `storeEnd()` return the node instead of `void` makes the code more fluent.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23669#discussion_r2116571327
PR Review Comment: https://git.openjdk.org/jdk/pull/23669#discussion_r2116571098