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

[Maurizio Cimadamore]

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

>> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ThisEscapeAnalyzer.java line 265:
>> 
>>> 263: 
>>> 264:         // Short circuit if this calculation is unnecessary
>>> 265:         if (!lintMapper.lintAt(env.toplevel.sourcefile, env.tree.pos()).get().isEnabled(THIS_ESCAPE))
>> 
>> Question -- this code does the shortcircuiting using `lintMapper` -- but I saw code in `Check` that does shortcircuiting using `lint` (e.g. `Check::checkPotentiallyAmbiguousOverloads`). If such short-circuit operation are somewhat frequent (to avoid expensive computation logic) perhaps we should have a "blessed" (and less verbose) way to do it?
>> 
>> (Unfortunately, I'm aware that `checkPotentiallyAmbiguousOverloads` is called when `lintMapper` yet doesn't know whether lint warnings are suppressed or not -- this seems a problem?)
>
> The subtle difference between `ThisEscapeAnalyzer` and `Check::checkPotentiallyAmbiguousOverloads` is that `ThisEscapeAnalyzer` runs during the compiler's `WARN` phase, whereas `checkPotentiallyAmbiguousOverloads` runs during the `ATTR` phase, and only during the `WARN` phase it is guaranteed that `LintMapper.lintAt()` will always be able to return a non-empty value.
> 
> Side note: as you may remember we had an earlier discussion about the idea of moving warnings from the `ATTR` phase to the `WARN` phase - the main "pro" being that there is no longer any concern about whether the applicable `Lint` instance has been mapped yet, so you don't need to track it manually (like `Check` does). I played with trying to do this but it was too heavy of a lift at the time (and probably still is) but it's a potential long term idea. The downside however (as you pointed out) is that during `ATTR` you have all the information you need to compute the warnings already "in front of you", whereas if you deferred that warning calculation to the `WARN` you would have to find it again (but maybe most of it is available?). Anyway, for now this is all "for future study".

One thing that might help would be to maybe use the name `rootLint` for all lint variables we use to shortcircuit -- to make it clear that these checks are not really... 100% correct. Another possibility would be to have a centralized check in LintMapper which, if the tree has been attributed does a check similar to what `ThisEscapeAnalyzer` does, otherwise we just look at the root lint (as a stop gap). At least all clients that need shortcircuiting would use same logic, and the impl could do the best it can to support that (and maybe in the future we can see if we can improve on that front).

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

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