RFR: 8362885: A more formal way to mark javac's Flags that belong to a specific Symbol type only [v3]

[Chen Liang]

On Mon, 28 Jul 2025 06:50:47 GMT, Jan Lahoda <jlahoda at openjdk.org> wrote:

>> This PR proposes to improve handling of javac's `Flags` in two ways:
>> - for each flag, there's now an informational annotation specifying what is the target Symbol type. Only targets right now are `TypeSymbol`s, `MethodSymbol`s and `VarSymbol`s. If we ran out of flags for `TypeSymbol`s, we could split those to module/package/class/type variable, but it does not seem to be quite necessary yet. There's an auxiliary special `BLOCK`, which is for `JCBlock`.
>> - the manually handled `Flags.Flag` enum is replaced with autogenerated `FlagsEnum`
>> 
>> This is inspired by:
>> https://github.com/openjdk/jdk/pull/26181#pullrequestreview-2997428662
>> 
>> There may be some better to handle `Flags` eventually, but this hopefully improves the current situation at least somewhat, by providing more formal way to say the flags' target, and restricting the need to read comments and search for free flags.
>> 
>> As a side-effect of this annotation, the `test/langtools/tools/javac/flags/FlagsTest.java` now also prints which flags are free, for each Symbol type.
>> 
>> (I will remove the `build` label for now, until discussion on javac level is done, and will re-add it if we decide the goal to autogenerate the FlagsEnum makes sense.)
>
> Jan Lahoda has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Reverting runtime checks, as suggested.

make/langtools/tools/flagsgenerator/FlagsGenerator.java line 64:

> 62:             TypeElement clazz = (TypeElement) trees.getElement(new TreePath(new TreePath(cut), cut.getTypeDecls().get(0)));
> 63:             Map<Integer, List<String>> flag2Names = new TreeMap<>();
> 64:             Map<FlagTarget, Map<Integer, List<String>>> target2FlagBit2Fields = new HashMap<>();

Suggestion:

            Map<FlagTarget, Map<Integer, List<String>>> target2FlagBit2Fields = new EnumMap<>(FlagTarget.class);

make/langtools/tools/flagsgenerator/FlagsGenerator.java line 84:

> 82:                                            .forEach(target -> target2FlagBit2Fields.computeIfAbsent(target, _ -> new HashMap<>())
> 83:                                                                                    .computeIfAbsent(flagBit, _ -> new ArrayList<>())
> 84:                                                                                    .add(flagName));

Instead of a dedicated loop to verify no overlaps, we can make the nested map `Map<Integer, String>` and fail fast whenever we detect a conflict, like:

var oldFlagName = target2FlagBit2Fields.computeIfAbsent(target, _ -> new HashMap<>()).put(flagBit, flagName);
if (oldFlagName != null) {
    // Fail fast code
}

I personally don't see a reason to collect all conflicting fields for a flag if any of these conflicts already causes a failure.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26452#discussion_r2237874183
PR Review Comment: https://git.openjdk.org/jdk/pull/26452#discussion_r2237871781