RFR: 8347474: Options singleton is used before options are parsed

Archie Cobbs acobbs at openjdk.org
Sun Jan 12 22:55:08 UTC 2025


On Sun, 12 Jan 2025 22:50:14 GMT, Archie Cobbs <acobbs at openjdk.org> wrote:

> This PR proposes some cleanup/refactoring relating to startup ordering issues in the compiler.
> 
> Please see the follow-up comment for a more complete description (trying to keep the first comment short).

**Issue A**: In several places in the compiler, the `Options` singleton is queried before it's been populated. Most of the time this is checking for undocumented flags like `-XDonlySyntaxErrorsUnrecoverable`, so unless you are actually using these flags, this does not cause a problem. But on the flip side, if you ever do use these flags, they could be ignored.

**Issue B**: Several classes would like to use the root `Lint` instance but can't due to initialization ordering issues. An example is `BaseFileManager`, which uses `Options.isLintSet()` instead. But that method not only duplicates `Lint.isEnabled()` but also is buggy - it doesn't correctly mirror `Lint`'s  behavior for certain categories like `"preview"`, `"dep-ann"`, etc.

In the bigger picture, the compiler has a bunch of singletons like `Log`, `Options`, `Lint`, `Names`, etc. that are somewhat inter-dependent and being wired up and initialized at the same time, which creates a classic startup ordering puzzle. With no explicitly enforced ordering, it's easy for problems to occur. To complicate things further, the ordering can change depending on which API is used to invoke the compiler; some test cases even test individual singletons, forcing them to be "first".

The `Options` singleton is something of a poster child because so many other singletons query it as part of their own initialization. For example, `Log` depends on flags like `-Xmaxerrs`, which come from `Options`, but the processing of options requires a `Log` for any required output, etc.

To address the situation, this PR proposes the following:

* Define two states for `Options`:
  * _Uninitialized_ means the instance is empty (unpopulated)
  * _Initialized_ means it is (or has started being) populated
* Provide two options for querying `Options`:
  * `Options.get()`:
    * If initialized, behave the same as today
    * If uninitialized, return null/false (same as today) but also later, if/when initialized, `Assert.check()` that the correct answer was given¹. This provides backward-compatible behavior while also auto-detecting **Issue A** bugs when they occur.
  * `Options.whenReady(Runnable)`:
    * If initialized, invokes the `Runnable` immediately
    * If uninitialized, registers a listener to invoke the `Runnable` if/when initialized
    * `Options` already has a simple listener notification mechanism, so we piggy-back on that
* "Robustify" a few low-level singletons such as `Log`, `JCDiagnostic`, `Lint`, etc., allowing them to function properly both before and after `Options` is populated
* Update several singletons (such as `BaseFileManager`) to use `Lint` directly, replacing previous homebrew logic

¹ Two alternatives would be: (a) just log a warning, or (b) only assert if some flag `-XDverifyOptions` was given. But an ignored flag is by definition a bug, so an assertion seems appropriate here.

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

PR Comment: https://git.openjdk.org/jdk/pull/23056#issuecomment-2585950556


More information about the compiler-dev mailing list