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

[Archie Cobbs]

On Mon, 23 Jun 2025 09:48:03 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> The compiler's handling of the aggregation of mandatory warnings into "notes" at the end of compilation can be refactored to simplify the code.
>> 
>> The `JCDiagnostic` class supports flags that alter how warnings are handled, e.g., `MANDATORY`, `NON_DEFERRABLE`, etc. So instead of having to log aggregated mandatory warnings through a separate channel (the `MandatoryWarningHandler`), these warnings could instead be logged just like any other warning, but with an `AGGREGATED` flag added. The actual aggregation can then be handled "behind the scenes" by the logging subsystem.
>> 
>> This will also make it easier to implement `@SuppressAnnotations` support for parser/tokenizer warnings which require aggregated mandatory warning notes such as warnings for preview features.
>
> 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.

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

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