RFR: 8348427: DeferredLintHandler API should use JCTree instead of DiagnosticPosition [v3]
Archie Cobbs
acobbs at openjdk.org
Thu Feb 6 02:41:54 UTC 2025
> The purpose of `DeferredLintHandler` is to allow `@SuppressWarnings` to be applied to warnings that are generated before `@SuppressWarnings` annotations themselves have been processed. The way this currently works is that warning callbacks are kept in a `HashMap` keyed by the innermost containing module, package, class, method, or variable declarations (in the form of a `JCModuleDecl`, `JCPackageDecl`, `JCClassDecl`, `JCMethodDecl`, or `JCVariableDecl`). Later, when the compiler executes the attribution phase and the lint categories suppressed at each declaration are known, the corresponding warning callbacks are provided with an appropriately configured `Lint` instance and "flushed".
>
> However, the `DeferredLintHandler` API uses `DiagnosticPosition` instead of `JCTree` for registering and flushing deferred warnings. This opens the door for bugs where warnings are registered to an object which is not a declaration, and therefore ignored.
>
> In fact, this occurs once in the code ([here](https://github.com/openjdk/jdk/blob/48ece0721489c1b357aaa81e89fe59f486079d15/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Annotate.java#L679)) where a `JCExpression` is being passed, although in this case the bug appears to be harmless (because annotation values can't contain any type of declaration).
>
> The API should be tighted up, and furthermore an assertion should be added to verify that the JCTree being passed is actually a declaration supporting `@SuppressWarnings`.
>
> In addition, there is a design flaw in the API: it's not possible to obtain the current immediate mode `Lint` object, so if an immediate mode `Lint` object is pushed/popped more than once, the second `Lint` object will overwrite the first.
>
> To fix this, the API should be adjusted so the stack of current declarations and/or immediate mode `Lint` objects is managed by the `DeferredLintHandler` itself, by providing a `push()` and `pop()` methods in the API.
Archie Cobbs has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains seven commits:
- Eliminate code branch that never occurs.
- Clean up imports.
- Merge branch 'master' into JDK-8348427 to resolve conflict.
- Merge branch 'master' into JDK-8348427 to fix conflict.
- Refactor the DeferredLintHandler API to maintain the "stack" itself.
- Add Javadoc to document a subtley in the API.
- Update DeferredLintHandler API to use JCTree instead of DiagnosticPosition.
-------------
Changes: https://git.openjdk.org/jdk/pull/23281/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=23281&range=02
Stats: 210 lines in 9 files changed: 50 ins; 34 del; 126 mod
Patch: https://git.openjdk.org/jdk/pull/23281.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/23281/head:pull/23281
PR: https://git.openjdk.org/jdk/pull/23281
More information about the compiler-dev
mailing list