RFR: 8359493: Refactor how aggregated mandatory warnings are handled in the compiler [v2]

[Maurizio Cimadamore]

On Mon, 23 Jun 2025 15:43:11 GMT, Archie Cobbs <acobbs at openjdk.org> wrote:

>> src/jdk.compiler/share/classes/com/sun/tools/javac/util/AbstractLog.java line 165:
>> 
>>> 163:      * @param flags      Any additional flags required
>>> 164:      */
>>> 165:     public void warning(Warning warningKey, DiagnosticFlag... flags) {
>> 
>> I'm not sure about these changes to the `warning` method. The reason being that this PR doesn't seem to require such changes. And note that, with this change, all calls to `Log.warning` become variadic -- meaning the compiler will have to at least allocate an empty array. I'm not sure if this is enough to cause a regression, but if not necessary, I'd prefer to revert these changes, also noting that `error` and `note` have variants that do not accept flags.
>
> (This comment also applies to your question about `mandatoryWarning()`).
> 
> This PR is a precursor to #24584 which is going to add another flag (`DEFAULT_ENABLED`). The goal here is to encapsulate all the site-specific peculiarities of logging in these flags to make the code clearer.
> 
> Let me know how you'd prefer to approach this. E.g., if you're worried about useless empty array allocations, we could just have another method override (so we'd have one that takes flags and one that doesn't). If you're worried about consistency in the API we can add a flags option to all of the other log methods. Etc. I'm happy to do it however makes the most sense.

I'd prefer to address the `DEFAULT_ENABLED` in a separate (and standalone, if possible) PR, to make sure we're really going the right direction about this. My general feeling is that all these properties are ultimately tied to the lint category --- so, once you pick a category, you kind of know whether it's aggregated, or defaultEnabled, or whatever -- modelling this as a flag makes it look as if you can also emit a preview warning w/o the aggregation behavior, which I'm not sure makes sense?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25810#discussion_r2161984764