RFR: 8362885: A more formal way to mark javac's Flags that belong to a specific Symbol type only
Archie Cobbs
acobbs at openjdk.org
Thu Jul 24 15:27:54 UTC 2025
On Thu, 24 Jul 2025 07:21:12 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.)
This looks like a really nice improvement for maintainability.
**?** Would it make more sense (and/or be simpler?) to move the conflict-checking logic into `FlagsGenerator`?
It somehow seems odd to have a tool which can knowingly generate an invalid `FlagsEnum.java` file... and then defer the check for that until later, maybe, in a unit test... when we could just make the build fail instead.
**?** Is it worth adding some simple runtime check for correct `FlagTarget` usage?
For example (just brainstorming):
* For each `FlagTarget` have `FlagsGenerator` output e.g. `public static final long METHOD_FLAGS = 0x0301930420520017L;`
* At appropriate location(s) in the code, add `Assert.check(this.flags_field & ~METHOD_FLAGS) == 0);`
(Of course even better would be to check this at compile time using stronger typing (e.g., by replacing `long flags_field` with some `EnumSet`, etc.) but that's probably way beyond this first step.)
-------------
PR Comment: https://git.openjdk.org/jdk/pull/26452#issuecomment-3113908635
More information about the compiler-dev
mailing list