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

[Archie Cobbs]

On Wed, 30 Jul 2025 11:56:32 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

> Thanks for the comments -- mapped declarations are indeed sparse as you point out, and this might simplify the "position matching" process. However, the unmapped declarations are not sparse.

There number of unmapped declarations equals the number of top-level class, package, or module declarations. I'm describing this as "sparse" because it does not include any methods, variables or nested classes. But of course if you're considering only top-level declarations then it's not "sparse" with respect to just that set.

> It is true that, after Attr, we will go through the list of unmapped decls and remove them as we map them. But, after parsing the list of unmapped declaration does seem a copy of an AST.

It's only a copy of the AST to a depth of one... so really a list, not a tree... but regardless, I think you have a good point (see below)...

> Say you kept _mapped_ declaration as they are now. Can the list of _unmapped_ declaration just be replaced by the compilation unit tree? Or, maybe, a "list of declaration trees" to analyze?

To restate your question: What value does storing a list of `Decl`'s provide over just storing a list of the corresponding original `JCTree` objects? There are two reasons I did this:
1. If we just store the `JCTree`'s, then we also must retain a reference to every source file's `EndPosTable` (it's required by `MappedDeclBuilder`), which I was trying to avoid because, at least in some cases (depending on compiler configuration), those tables are (otherwise) discarded after parsing. So the new references to `EndPosTable` will increase compiler memory usage. But I may have been overreacting or misunderstanding that.
2. This is a minor detail, but the `JCTree` we see after parsing is not always the same `JCTree` we see after attribution (not sure why; it happens in test method `EdgeCases.testWrongPackageInExportedPackage()` for some reason). So even if we store the trees, we still have to compare them by position, not object equality, so in effect it's really the position we care about, not the tree...

> Does this mean we create a mapped decl for all toplevel decls in a file (even if their lint is not interesting) ? Why?

Yes, but! That special case is no longer required with the recent cleanups, so now we can fix this too :)

> My concrete suggestion here would be to...

Thanks, this makes things simpler and cleaner. Should all be done in 6a1289e9372.

Thanks again for your thoughtful review and comments!

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

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