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

[Archie Cobbs]

On Wed, 3 Apr 2024 23:24:21 GMT, Archie Cobbs <acobbs at openjdk.org> wrote:

> 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 t...

This pull request has now been integrated.

Changeset: 192ec387
Author:    Archie Cobbs <acobbs at openjdk.org>
Committer: Vicente Romero <vromero at openjdk.org>
URL:       https://git.openjdk.org/jdk/commit/192ec387bc074b25465decf598a4dd87651cbcbb
Stats:     86 lines in 2 files changed: 71 ins; 13 del; 2 mod

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

Reviewed-by: vromero

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

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