RFR: 8329595: spurious variable "might not have been initialized" on static final field

Archie Cobbs acobbs at openjdk.org
Thu Apr 4 02:17:30 UTC 2024


This bug causes the compiler to generate a bogus error `variable NAME might not have been initialized` for the following program:

public class StaticFinalNestedClass {
    public class Inner {
        public static final String NAME;
        static {
            try {
                NAME = "bob";
            } catch (Exception e) {
                throw new Error(e);
            }
        }
    }
}

An interesting fact is that if you make the class `Inner` a `static` inner class, then the bug disappears. The oddness of that fact is indicative of the oddness of the underlying problem, some of which I probably contributed to and which I'll try to summarize...

First some background. The various flow analyzers are written to recurse through the entire AST including nested classes in one go. So for example if there is an inner class, the analysis of the outer class is paused while the inner class is analyzed, and then the former state is restored and the outer class analysis proceeds. This causes a bit of chaos in situations where the analyzer needs to "jump around" for whatever reason. For example, when `super()` is encountered while analyzing a constructor, we need to analyze any field initializers because that's when they execute.

As a result, there are several cases where the same code gets visited more than once, which is not harmful but is obviously inefficient. But it also leads to this bug.

So what is the actual bug? The bug is that (one of the times) the inner class static initializer was being checked for uninitialized variables was when the outer class constructor was being analyzed, and that analysis recursed into the inner class. Because the outer class constructor was being analyzed, and no other method had been entered yet, the `isConstructor` flag was still set to `true`, so the code that marks all fields as initialized when an exception is thrown from a constructor or static initializer (because the instance or class is unusable so don't bother complaining about uninitialized fields) was only marking the instance fields.

In commit 12e983a72e72 I did some refactoring to create a method `forEachInitializer()` for analyzing initializers, but the existing behavior of recursing into nested classes was preserved in order to not break things. With this commit, in order to impose some order, `forEachInitializer()` is being changed to not recurse into nested classes. Instead, the various bits of code that use `forEachInitializer()` and also need to recurse into nested classes now do that explicitly themselves (and the rest do not, eliminating some redundancy). This change also makes it easier to fix the actual bug - the bug fix is to simply remove a workaround for the previous redundant recursion. I've broken the changes into two corresponding commits for clarity.

There was also a separate bug in `forEachInitializer()`. It takes a boolean `isStatic` parameter (for whether you want to visit instance initializers or static initializers). The bug is that it would only recurse into nested classes with the same staticness as `isStatic`. That's explains the odd observation about the static-ness of class `Inner` in the given test case. Of course, now that `forEachInitializer()` no longer recurses into nested classes that problem goes away as well.

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

Commit messages:
 - Don't report unintialized fields when a static initializer throws.
 - Refactor so forEachInitializer() doesn't recurse into nested classes.

Changes: https://git.openjdk.org/jdk/pull/18610/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18610&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8329595
  Stats: 86 lines in 2 files changed: 71 ins; 13 del; 2 mod
  Patch: https://git.openjdk.org/jdk/pull/18610.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18610/head:pull/18610

PR: https://git.openjdk.org/jdk/pull/18610


More information about the compiler-dev mailing list