From 8d822b11197efd5bf29358a8ef0caedd203adcfb Mon Sep 17 00:00:00 2001 From: "Archie L. Cobbs" Date: Thu, 3 Jul 2025 09:31:40 -0500 Subject: [PATCH 1/2] Remove assumptions about mandatoryness from the MandatoryWarningAggregator. --- .../classes/com/sun/tools/javac/util/Log.java | 12 ++++----- ...Aggregator.java => WarningAggregator.java} | 27 +++++++------------ 2 files changed, 15 insertions(+), 24 deletions(-) rename src/jdk.compiler/share/classes/com/sun/tools/javac/util/{MandatoryWarningAggregator.java => WarningAggregator.java} (90%) diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/util/Log.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/util/Log.java index 1825a897d7f..8c60de58bb8 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/util/Log.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/util/Log.java @@ -707,7 +707,7 @@ public class Log extends AbstractLog { // Mandatory Warnings - private final EnumMap aggregators = new EnumMap<>(LintCategory.class); + private final EnumMap aggregators = new EnumMap<>(LintCategory.class); private final EnumSet suppressedDeferredMandatory = EnumSet.noneOf(LintCategory.class); @@ -725,17 +725,17 @@ public class Log extends AbstractLog { aggregators.entrySet().stream() .filter(entry -> !suppressedDeferredMandatory.contains(entry.getKey())) .map(Map.Entry::getValue) - .map(MandatoryWarningAggregator::aggregationNotes) + .map(WarningAggregator::aggregationNotes) .flatMap(List::stream) .forEach(this::report); aggregators.clear(); } - private MandatoryWarningAggregator aggregatorFor(LintCategory lc) { + private WarningAggregator aggregatorFor(LintCategory lc) { return switch (lc) { - case PREVIEW -> aggregators.computeIfAbsent(lc, c -> new MandatoryWarningAggregator(this, Source.instance(context), c)); - case DEPRECATION -> aggregators.computeIfAbsent(lc, c -> new MandatoryWarningAggregator(this, null, c, "deprecated")); - default -> aggregators.computeIfAbsent(lc, c -> new MandatoryWarningAggregator(this, null, c)); + case PREVIEW -> aggregators.computeIfAbsent(lc, c -> new WarningAggregator(this, Source.instance(context), c)); + case DEPRECATION -> aggregators.computeIfAbsent(lc, c -> new WarningAggregator(this, null, c, "deprecated")); + default -> aggregators.computeIfAbsent(lc, c -> new WarningAggregator(this, null, c)); }; } diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/util/MandatoryWarningAggregator.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/util/WarningAggregator.java similarity index 90% rename from src/jdk.compiler/share/classes/com/sun/tools/javac/util/MandatoryWarningAggregator.java rename to src/jdk.compiler/share/classes/com/sun/tools/javac/util/WarningAggregator.java index b5da48f9183..2bb8d2e2754 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/util/MandatoryWarningAggregator.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/util/WarningAggregator.java @@ -42,19 +42,12 @@ import com.sun.tools.javac.util.JCDiagnostic.Warning; /** - * An aggregator for mandatory warnings, setting up a deferred diagnostic + * An aggregator for warnings, setting up a deferred diagnostic * to be printed at the end of the compilation if some warnings get suppressed * because the lint category is not enabled or too many warnings have already * been generated. * *

- * Note that the SuppressWarnings annotation can be used to suppress warnings - * about conditions that would otherwise merit a warning. Such processing - * is done when the condition is detected, and in those cases, no call is - * made on any API to generate a warning at all. In consequence, this handler only - * Returns to handle those warnings that JLS says must be generated. - * - *

* All warnings must be in the same {@link LintCategory} provided to the constructor. * *

This is NOT part of any supported API. @@ -62,12 +55,11 @@ import com.sun.tools.javac.util.JCDiagnostic.Warning; * This code and its internal interfaces are subject to change or * deletion without notice. */ -class MandatoryWarningAggregator { +class WarningAggregator { /** * The kinds of different deferred diagnostics that might be generated - * if a mandatory warning is suppressed because too many warnings have - * already been output. + * if a warning is suppressed because too many warnings have already been output. * * The parameter is a fragment used to build an I18N message key for Log. */ @@ -109,18 +101,18 @@ class MandatoryWarningAggregator { /** - * Create an aggregator for mandatory warnings. + * Create an aggregator for warnings. * * @param log The log on which to generate any diagnostics * @param source Associated source file, or null for none * @param lc The lint category for all warnings */ - public MandatoryWarningAggregator(Log log, Source source, LintCategory lc) { + public WarningAggregator(Log log, Source source, LintCategory lc) { this(log, source, lc, null); } /** - * Create an aggregator for mandatory warnings. + * Create an aggregator for warnings. * * @param log The log on which to generate any diagnostics * @param source Associated source file, or null for none @@ -128,7 +120,7 @@ class MandatoryWarningAggregator { * @param prefix A common prefix for the set of message keys for the messages * that may be generated, or null to infer from the lint category. */ - public MandatoryWarningAggregator(Log log, Source source, LintCategory lc, String prefix) { + public WarningAggregator(Log log, Source source, LintCategory lc, String prefix) { this.log = log; this.source = source; this.prefix = prefix != null ? prefix : lc.option; @@ -136,14 +128,13 @@ class MandatoryWarningAggregator { } /** - * Aggregate a mandatory warning and determine whether to emit it. + * Aggregate a warning and determine whether to emit it. * - * @param diagnostic the mandatory warning + * @param diagnostic the warning * @param verbose whether the warning's lint category is enabled * @return true if diagnostic should be emitted, otherwise false */ public boolean aggregate(JCDiagnostic diagnostic, boolean verbose) { - Assert.check(diagnostic.isMandatory()); Assert.check(diagnostic.getLintCategory() == lintCategory); JavaFileObject currentSource = log.currentSourceFile(); if (verbose) { From 2b16d65758ba90c6b7acaa55b64828d2f40e1f15 Mon Sep 17 00:00:00 2001 From: "Archie L. Cobbs" Date: Thu, 3 Jul 2025 10:03:15 -0500 Subject: [PATCH 2/2] Address review suggestions. --- .../propertiesparser/gen/ClassGenerator.java | 23 ++++++++++---- .../resources/templates.properties | 16 +++++----- .../sun/tools/javac/util/JCDiagnostic.java | 31 +++++++++---------- 3 files changed, 39 insertions(+), 31 deletions(-) diff --git a/make/langtools/tools/propertiesparser/gen/ClassGenerator.java b/make/langtools/tools/propertiesparser/gen/ClassGenerator.java index 10bc274e141..247537b4676 100644 --- a/make/langtools/tools/propertiesparser/gen/ClassGenerator.java +++ b/make/langtools/tools/propertiesparser/gen/ClassGenerator.java @@ -47,6 +47,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Map.Entry; import java.util.Properties; @@ -94,7 +95,8 @@ public class ClassGenerator { WILDCARDS_EXTENDS("wildcards.extends"), SUPPRESS_WARNINGS("suppress.warnings"), LINT_CATEGORY("lint.category"), - DIAGNOSTIC_FLAGS("diagnostic.flags"); + DIAGNOSTIC_FLAGS_EMPTY("diagnostic.flags.empty"), + DIAGNOSTIC_FLAGS_NON_EMPTY("diagnostic.flags.non-empty"); /** stub key (as it appears in the property file) */ String key; @@ -264,7 +266,8 @@ public class ClassGenerator { .filter(MessageLine::isDiagnosticFlags) .map(MessageLine::diagnosticFlags) .flatMap(Stream::of) - .map(s -> "\"" + s + "\"") + .map(s -> s.replace('-', '_')) + .map(s -> s.toUpperCase(Locale.ROOT)) .collect(Collectors.joining(", ")); String factoryName = factoryName(key); if (msgInfo.getTypes().isEmpty()) { @@ -272,13 +275,17 @@ public class ClassGenerator { String factoryField; if (lintCategory == null) { factoryField = StubKind.FACTORY_FIELD.format(k.keyClazz, factoryName, - !diagnosticFlags.isEmpty() ? StubKind.DIAGNOSTIC_FLAGS.format(diagnosticFlags) : "null", + diagnosticFlags.isEmpty() ? + StubKind.DIAGNOSTIC_FLAGS_EMPTY.format() : + StubKind.DIAGNOSTIC_FLAGS_NON_EMPTY.format(diagnosticFlags), "\"" + keyParts[0] + "\"", "\"" + Stream.of(keyParts).skip(2).collect(Collectors.joining(".")) + "\"", javadoc); } else { factoryField = StubKind.FACTORY_FIELD_LINT.format(k.keyClazz, factoryName, - !diagnosticFlags.isEmpty() ? StubKind.DIAGNOSTIC_FLAGS.format(diagnosticFlags) : "null", + diagnosticFlags.isEmpty() ? + StubKind.DIAGNOSTIC_FLAGS_EMPTY.format() : + StubKind.DIAGNOSTIC_FLAGS_NON_EMPTY.format(diagnosticFlags), StubKind.LINT_CATEGORY.format("\"" + lintCategory + "\""), "\"" + keyParts[0] + "\"", "\"" + Stream.of(keyParts).skip(2).collect(Collectors.joining(".")) + "\"", @@ -296,13 +303,17 @@ public class ClassGenerator { String methodBody; if (lintCategory == null) { methodBody = StubKind.FACTORY_METHOD_BODY.format(k.keyClazz, - !diagnosticFlags.isEmpty() ? StubKind.DIAGNOSTIC_FLAGS.format(diagnosticFlags) : "null", + diagnosticFlags.isEmpty() ? + StubKind.DIAGNOSTIC_FLAGS_EMPTY.format() : + StubKind.DIAGNOSTIC_FLAGS_NON_EMPTY.format(diagnosticFlags), "\"" + keyParts[0] + "\"", "\"" + Stream.of(keyParts).skip(2).collect(Collectors.joining(".")) + "\"", argNames.stream().collect(Collectors.joining(", "))); } else { methodBody = StubKind.FACTORY_METHOD_BODY_LINT.format(k.keyClazz, - !diagnosticFlags.isEmpty() ? StubKind.DIAGNOSTIC_FLAGS.format(diagnosticFlags) : "null", + diagnosticFlags.isEmpty() ? + StubKind.DIAGNOSTIC_FLAGS_EMPTY.format() : + StubKind.DIAGNOSTIC_FLAGS_NON_EMPTY.format(diagnosticFlags), StubKind.LINT_CATEGORY.format("\"" + lintCategory + "\""), "\"" + keyParts[0] + "\"", "\"" + Stream.of(keyParts).skip(2).collect(Collectors.joining(".")) + "\"", diff --git a/make/langtools/tools/propertiesparser/resources/templates.properties b/make/langtools/tools/propertiesparser/resources/templates.properties index 8488124da22..81a9be2552c 100644 --- a/make/langtools/tools/propertiesparser/resources/templates.properties +++ b/make/langtools/tools/propertiesparser/resources/templates.properties @@ -35,8 +35,9 @@ toplevel.decl=\ import com.sun.tools.javac.util.JCDiagnostic.Fragment;\n\ import com.sun.tools.javac.code.Lint.LintCategory;\n\ \n\ - import java.util.Locale;\n\ - import java.util.stream.Stream;\n\ + import java.util.EnumSet;\n\ + \n\ + import static com.sun.tools.javac.util.JCDiagnostic.DiagnosticFlag.*;\n\ \n\ public class {2} '{'\n\ {3}\n\ @@ -88,10 +89,9 @@ suppress.warnings=\ lint.category=\ LintCategory.get({0}).get() -diagnostic.flags=\n\ - ' 'Stream.of({0})\n\ - ' '.map(s -> s.replace(''-'', ''_''))\n\ - ' '.map(s -> s.toUpperCase(Locale.ROOT))\n\ - ' '.map(DiagnosticFlag::valueOf)\n\ - ' '.toArray(DiagnosticFlag[]::new) +diagnostic.flags.empty=\ + EnumSet.noneOf(DiagnosticFlag.class) + +diagnostic.flags.non-empty=\ + EnumSet.of({0}) diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/util/JCDiagnostic.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/util/JCDiagnostic.java index c7a889f9132..5d10626f944 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/util/JCDiagnostic.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/util/JCDiagnostic.java @@ -509,8 +509,8 @@ public class JCDiagnostic implements Diagnostic { /** The diagnostic kind (i.e. error). */ DiagnosticType type; - /** A set of diagnostic flags to be automatically added to newly created JCDiagnostics (if not null). */ - DiagnosticFlag[] flags; + /** A set of diagnostic flags to be automatically added to newly created JCDiagnostics. */ + Set flags; /** The diagnostic prefix (i.e. 'javac'); used to compute full resource key. */ String prefix; @@ -522,9 +522,9 @@ public class JCDiagnostic implements Diagnostic { /** The diagnostic arguments. */ Object[] args; - private DiagnosticInfo(DiagnosticType type, DiagnosticFlag[] flags, String prefix, String code, Object... args) { + private DiagnosticInfo(DiagnosticType type, Set flags, String prefix, String code, Object... args) { this.type = type; - this.flags = flags; + this.flags = flags != null ? flags : EnumSet.noneOf(DiagnosticFlag.class); this.prefix = prefix; this.code = code; this.args = args; @@ -540,11 +540,12 @@ public class JCDiagnostic implements Diagnostic { /** * Static factory method; build a custom diagnostic key using given kind, prefix, code and args. */ - public static DiagnosticInfo of(DiagnosticType type, DiagnosticFlag[] flags, String prefix, String code, Object... args) { + public static DiagnosticInfo of(DiagnosticType type, Set flags, + String prefix, String code, Object... args) { return of(type, flags, null, prefix, code, args); } - public static DiagnosticInfo of(DiagnosticType type, DiagnosticFlag[] flags, + public static DiagnosticInfo of(DiagnosticType type, Set flags, LintCategory lc, String prefix, String code, Object... args) { switch (type) { case ERROR: @@ -582,7 +583,7 @@ public class JCDiagnostic implements Diagnostic { } public boolean hasFlag(DiagnosticFlag flag) { - return flags != null && Arrays.asList(flags).contains(flag); + return flags.contains(flag); } } @@ -590,7 +591,7 @@ public class JCDiagnostic implements Diagnostic { * Class representing error diagnostic keys. */ public static final class Error extends DiagnosticInfo { - public Error(DiagnosticFlag[] flags, String prefix, String key, Object... args) { + public Error(Set flags, String prefix, String key, Object... args) { super(DiagnosticType.ERROR, flags, prefix, key, args); } } @@ -599,7 +600,7 @@ public class JCDiagnostic implements Diagnostic { * Class representing warning diagnostic keys. */ public static sealed class Warning extends DiagnosticInfo { - public Warning(DiagnosticFlag[] flags, String prefix, String key, Object... args) { + public Warning(Set flags, String prefix, String key, Object... args) { super(DiagnosticType.WARNING, flags, prefix, key, args); } } @@ -610,7 +611,7 @@ public class JCDiagnostic implements Diagnostic { public static final class LintWarning extends Warning { final LintCategory category; - public LintWarning(DiagnosticFlag[] flags, LintCategory category, String prefix, String key, Object... args) { + public LintWarning(Set flags, LintCategory category, String prefix, String key, Object... args) { super(flags, prefix, key, args); this.category = category; } @@ -624,7 +625,7 @@ public class JCDiagnostic implements Diagnostic { * Class representing note diagnostic keys. */ public static final class Note extends DiagnosticInfo { - public Note(DiagnosticFlag[] flags, String prefix, String key, Object... args) { + public Note(Set flags, String prefix, String key, Object... args) { super(DiagnosticType.NOTE, flags, prefix, key, args); } } @@ -633,7 +634,7 @@ public class JCDiagnostic implements Diagnostic { * Class representing fragment diagnostic keys. */ public static final class Fragment extends DiagnosticInfo { - public Fragment(DiagnosticFlag[] flags, String prefix, String key, Object... args) { + public Fragment(Set flags, String prefix, String key, Object... args) { super(DiagnosticType.FRAGMENT, flags, prefix, key, args); } } @@ -680,11 +681,7 @@ public class JCDiagnostic implements Diagnostic { this.position = pos; this.rewriter = rewriter; - if (diagnosticInfo.flags != null) { - for (DiagnosticFlag flag : diagnosticInfo.flags) { - this.flags.add(flag); - } - } + this.flags.addAll(diagnosticInfo.flags); } /**