RFR: 8344079: Minor fixes and cleanups to compiler lint-related code [v2]
Maurizio Cimadamore
mcimadamore at openjdk.org
Mon Dec 2 17:59:37 UTC 2024
On Mon, 2 Dec 2024 17:53:53 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:
>> Changing the key is possible. I even prototyped that - but ran out of steam when I realized that all compiler negative tests required some fixing. So I'm a bit wary of going down that path.
>>
>> IMHO, the issue with stuff like:
>>
>>
>> if (lint.isEnabled(LintCategory.DEP_ANN))
>> log.warn(LintCategory.DEP_ANN, pos, Warnings.MissingDeprecatedAnnotation));
>>
>>
>> Is that compiler code should even bother to do this! Ideally, you might want to have a single routine:
>>
>>
>> log.warnIfEnabled(DEP_ANN, pos, Warnings.MissingDeprecatedAnnotation)
>>
>>
>> This could test whether `DEP_ANN` is enabled _and_ also check that the provided warning key has the correct category set
>
>> `log.warnIfEnabled(DEP_ANN, pos, Warnings.MissingDeprecatedAnnotation)`
>
> Or, even better:
>
> `log.warnIfEnabled(pos, Warnings.MissingDeprecatedAnnotation)`
>
> (e.g. just use the warning key option for the `isEnabled` check)
Another observation: it would be better if we had two distinct set of diagnostics; warnings and lint warnings. If one couldn't be passed in place of the other that will force clients to use the correct `log` method. Which might make it easier to check that all the places issuing a lint warning also check as to whether the warning is enabled.
(that said, we can't check for enable-ment in all cases - e.g. for lint warnings in the tokenizer, so there will always be some ad-hocness somewhere, I think. I think we win if we can find something that works in most cases, but we can't make everything statically type-checkable, not without significant cost and effort).
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22056#discussion_r1866336502
More information about the compiler-dev
mailing list