RFR: 8348611: Eliminate DeferredLintHandler and emit warnings after attribution [v5]

[Maurizio Cimadamore]

On Mon, 28 Jul 2025 22:05:44 GMT, Archie Cobbs <acobbs at openjdk.org> wrote:

>> This is a cleanup/refactoring of how lint warnings are logged and `@SuppressWarnings` annotations applied.
>> 
>> A central challenge with lint warnings is that warnings can be generated during any compiler phase, but whether a particular lint warning is suppressed via `@SuppressWarnings` can't be known until after attribution. For example, the parser doesn't have enough information to interpret and apply `@SuppressWarnings("text-blocks")` to text blocks, or `@SuppressWarnings("preview")` to preview lexical features; instead, the `DeferredLintHandler` is used to workaround this limitation.
>> 
>> In addition, several other factors complicate things:
>> * Knowing the current applicable `Lint` instance requires manually tracking it with each declaration visited and applying/removing the `@SuppressWarnings` annotation there, if any
>> * Some warnings are "suppressibly mandatory" (i.e., they are _emitted if not suppressed_ instead of _emitted if enabled_)
>> * Some warnings are "unsuppressibly mandatory" (e.g., the "internal proprietary API" warning)
>> * Some mandatory warnings are _aggregated_ into notes that are emitted at the end of compilation when not enabled
>> * Some warnings are _lint_ warnings, with a corresponding lint category, while others are just "plain" warnings
>> * Some lint warnings are suppressible via `@SuppressWarnings`, while others are only suppressible via `-Xlint:-foo` flags
>> * Speculative compilation requires holding log messages in purgatory until the speculation resolves, after which they are then either discarded or emitted. But this creates a tricky interaction with `DeferredLintHandler` because even after speculation is complete, we may still not yet know whether a warning should be suppressed.
>> 
>> Previously the logic to get all of this right was non-obviously woven around the code base. In particular, you needed to know somehow whether or not to use `DeferredLintHandler`, and in what "mode".
>> 
>> The overall goal of this PR is to simplify usage so that **no matter where you are in the compiler, you can just invoke `log.warning()` to log a warning** and (mostly) forget about all of the details listed above.
>
> Archie Cobbs has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Refactor LintMapper to clean up internal type hierarchy per review.

src/jdk.compiler/share/classes/com/sun/tools/javac/code/LintMapper.java line 183:

> 181:     private class FileInfo {
> 182: 
> 183:         final MappedDeclNode rootNode = new MappedDeclNode(rootLint);   // tree of this file's "interesting" declaration nodes

I still find the design a bit odd here. We create an "empty" root node here, but then in the constructor we set up all its children. Maybe we should create the full "node" from the original `JCCompilationUnit` tree (in a single shot) ? Also, I can't quite figure out if we end up adding both a `Decl` and a `MappedDecl` as children of the same parent decl? Perhaps, if the only difference between mapped and non-mapped decl is that the former have a Lint object, just having a single decl class with a mutable Lint field that is only set once might work?

Meta-comment -- I also realize how a lot of this could probably be avoided if we could just stash the Lint directly in the corresponding tree? (e.g. if we could do that, then we'd just need something to "find" the most specific tree given a position and see if it has a `Lint` set. We'd probably still need a map from source files to compilation unit trees)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24584#discussion_r2240599648