RFR: 8354216: Small cleanups relating to Log.DiagnosticHandler
Archie Cobbs
acobbs at openjdk.org
Wed Apr 9 17:40:47 UTC 2025
On Wed, 9 Apr 2025 17:26:21 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:
>> This is split off as a sub-task of [JDK-8224228](https://bugs.openjdk.org/browse/JDK-8224228), which seeks to add `@SuppressWarnings` support for lexical features.
>>
>> There are a few of small refactoring cleanups possible relating to the nested class `Log.DiagnosticHandler`.
>>
>> First, this class is currently declared as a static inner class with an explicit field reference to its outer class (`Log`) instance. Although protected and non final, this field really should never be changed. Therefore, there is no reason not to just make `DiagnosticHandler` a non-static inner class. This will slightly simplify the code and eliminate an obscure corner case by which a bug could possibly occur someday.
>>
>> Secondly, the deferred diagnostics are currently stored in a linked list, and in cases where they must be sorted before being reported, they have to be copied into an array first. A simpler and more space efficient approach would be to just use an `ArrayList`. Then the sorting variant of the method can just sort the list in place and delegate directly to the non-sorting variant.
>>
>> Finally, the code can be simplified by replacing a null `filter` predicate with an always true predicate at construction time instead of at filter time.
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/util/Log.java line 184:
>
>> 182: /** Report all deferred diagnostics in the specified order. */
>> 183: public void reportDeferredDiagnostics(Comparator<JCDiagnostic> order) {
>> 184: deferred.sort(order);
>
> So, here there would seem to be a difference in behavior, in that `sort` has a permanent side-effect on `deferred`. But I think this is fine, because at the end of this method we end up setting `deferred = null` anyway, right?
Yes, that's my thought too - we are indeed changing the list, but who cares because we're about to discard it. Of course the discarding is done indirectly by `reportDeferredDiagnostics()`, not by this method.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24553#discussion_r2035834713
More information about the compiler-dev
mailing list