RFR: 8347958: Minor compiler cleanups relating to MandatoryWarningHandler [v2]

Archie Cobbs acobbs at openjdk.org
Wed Feb 5 22:19:12 UTC 2025


On Wed, 5 Feb 2025 22:06:32 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>>> Now the handler starts off empty, and the first report determines the kind. But the same kind is then used for all deferred logging... 
>> 
>> You are correct - which is why the Javadoc comment "Instances assume all warnings will be in the same LintCategory" was added.
>> 
>> So in summary, I replaced what was previously an assertion of an invariant with a new API requirement of that invariant (which, currently, is satisfied). Admittedly it was a close call - the benefit being the elimination of an instance field and constructor parameter.
>> 
>> But keeping the assertion is a more conservative approach so I'll do that instead. Updated in 87e44347f65.
>
>> Admittedly it was a close call - the benefit being the elimination of an instance field and constructor parameter.
> 
> To clarify my thought process a bit: while the change made the impl simpler (one less field, one less constructor parameter) it also made it more "magic", in the sense that we lost the hard constraint that a MWH can only refer to _one_ lint category. So, overall, at least in my eyes, the overall balance was negative.

I agree, less magic is better :) Thanks for your review - very helpful as always.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23167#discussion_r1943748681


More information about the compiler-dev mailing list