RFR: 8344079: Minor fixes and cleanups to compiler lint-related code [v2]

[Archie Cobbs]

On Tue, 3 Dec 2024 15:17:02 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>>> See the first suggestion in this comment... would that work?
>> 
>> I like that idea. So, one concrete proposal might be:
>> 
>> 1. have LintWarning as a subclass of Warning
>> 2. have CompilerProperties reflect the lint categories, by having different nested classes where LintWarnings factories and fields would appear (one nested class per lint category)
>> 3. don't change `AbstractLog` - that will still offer a `log` method that takes a `Warning` that can be used for both regular warning and lint warnings (in case the lint warning is not really suppressible)
>> 4. Add a method in `lint` to "logIfEnabled". This method accepts a `LintWarning`, not a warning. So we get some static type checking.
>> 
>> (That said, if lint warnings are to be reported using (4), what would be the added benefit of (2) ? )
>
> There's also another issue with `logIfEnabled`. While that seems to be the majority of cases, there is a non-trivial amount of uses where the is-enabled check occurs at some outer level, at which point the inner code just warns w/o a check (see all the serial warnings inside `Check` -- but there's several other cases too). So, not sure how much that infrastructure would be worth adding.

> So, one concrete proposal might be:
> 
> - have LintWarning as a subclass of Warning
> - don't change `AbstractLog` - that will still offer a `log` method that takes a `Warning` that can be used for both regular warning and lint warnings (in case the lint warning is not really suppressible)

I'm with you except for these two items. I believe that every warning associated with a lint category is always suppressible at least via the `-Xlint:-foo` command line flag. Do you have any specific counter-examples in mind?

If that's true, then we should be able to go a little further:

1. Have `LintWarning` be a sibling class, not a subclass, of `Warning`
2. Have `CompilerProperties` reflect the lint categories, by having different nested classes where `LintWarning` factories and fields would appear (one nested class per lint category)
3. Change `AbstractLog` to add methods taking a `LintWarning`; these will automatically add the "[foo]" prefix to the warning text that is emitted
4. Add a convenience instance method `Lint.logIfEnabled()` which accepts a `LintWarning`, does the enablement check, and then (if the category is enabled) invokes the new `Log` method added in the previous step.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22056#discussion_r1867933843