RFR: 8350212: Track source end positions of declarations that support @SuppressWarnings [v3]
Jan Lahoda
jlahoda at openjdk.org
Tue Apr 8 14:59:14 UTC 2025
On Fri, 4 Apr 2025 21:42:35 GMT, Archie Cobbs <acobbs at openjdk.org> wrote:
>> This PR is a sub-task split off from [JDK-8224228](https://bugs.openjdk.org/browse/JDK-8224228), which seeks to add `@SuppressWarnings` support for lexical features.
>>
>> Lexical features don't know about classes, members or symbols, so their source file positions must be matched to the source file character offset range of the innermost containing `JCModuleDecl`, `JCPackageDecl`, `JCClassDecl`, `JCMethodDecl`, or `JCVariableDecl`. That means we need the end positions of all such declarations to be available.
>>
>> The parser doesn't normally store lexical end positions unless explicitly requested, and we don't want to mandate it for performance reasons.
>>
>> Instead, we can just add an `int endPos` field to these five AST nodes. This field can be populated as part of the normal parsing of these node types, which already supports the (optional) tracking of end positions.
>
> Archie Cobbs has updated the pull request incrementally with one additional commit since the last revision:
>
> Add missing variable decl end position.
The new calls to `storeEnd` seem to partly duplicate existing `to`/`toP` calls. That feels a bit unfortunate - as we may be adding new places that create AST node for declarations, and one sometimes forgets to do `to`/`toP` - and forgetting `storeEnd` might lead to weird problems.
I tried to re-write this part by putting the the set logic directly into the end pos tables here:
https://github.com/archiecobbs/jdk/compare/JDK-8350212...lahodaj:jdk:JDK-8350212?expand=1
Notes:
- I believe the fact that implictly declared classes don't have an end position was intentional at that time. I don't have an issue with changing that, just pointing it out.
- for blocks, `endpos` is actually pointing at the start of the closing brace, not the end (I think, at least). So, I've renamed the field, but kept the semantics.
What do you think?
-------------
PR Comment: https://git.openjdk.org/jdk/pull/23669#issuecomment-2786735601
More information about the compiler-dev
mailing list