Integrated: 8294461: wrong effectively final determination by javac

[Archie L. Cobbs]

On Tue, 25 Oct 2022 15:36:05 GMT, Archie L. Cobbs <duke at openjdk.org> wrote:

> This bug involves DA/DU analysis and the concept of "effectively final".
> 
> Here's the test case, which the compiler currently but incorrectly accepts:
> 
>     for (int i = 0; i < 10; i++) {
>         Runnable r = () -> System.out.println(i);   // variable i is NOT effectively final
>         break;                                      // even though "i++" is never reached
>     }
> 
> 
> For the purposes of "effectively final", it doesn't matter whether an assignment statement that makes a variable no longer effectively final is actually reachable or not. In cases like the `i++` above, a statement can be actually unreachable yet not "unreachable" according to the JLS, and so it does not generate an "unreachable statement" compiler error (which would otherwise hide this bug).
> 
> JLS §4.12.4 states:
> 
>> A local variable declared by a statement and whose declarator has an initializer ... is effectively final if all of the following are true:
>> ...
>> • It never occurs as the operand of a prefix or postfix increment or decrement operator (§15.14, §15.15).
> 
> So clearly `i` is not effectively final.
> 
> However, the way we calculate effective finality does not involve actually looking for increment/decrement operators. Instead, it's determined during DA/DU analysis: non-final variables have an `EFFECTIVELY_FINAL` flag which is initialized to `true` and then cleared if/when we encounter contrary evidence - e.g., an assignment when not DU.
> 
> For simplicity, the DA/DU analysis works like this:
> * Variables with initializers are treated like variables without initializers followed by an assignment
> * Increment/decrement operators are treated as a normal read followed by a normal assignment.
> 
> These are reasonable. However, it means this clause of JLS §4.12.4 effectively applies:
> 
>> A local variable declared by a statement and whose declarator lacks an initializer is effectively final if all of the following are true:
>> ...
>> • Whenever it occurs as the left hand side in an assignment expression, it is definitely unassigned and not definitely assigned before the assignment...
> 
> The bug with the current code is that when we see an assignment to a variable with the `EFFECTIVELY_FINAL` flag still set, we clear the flag if the variable is not DU, but we are _not_ clearing the flag if the variable is DA, as required above. This happens with the `i++` statement because, by virtue of it being actually unreachable, `i` is both DA and DU before that statement.
> 
> This patch corrects that omission.

This pull request has now been integrated.

Changeset: b8ad6cd9
Author:    Archie L. Cobbs <archie.cobbs at gmail.com>
Committer: Vicente Romero <vromero at openjdk.org>
URL:       https://git.openjdk.org/jdk/commit/b8ad6cd98a7e4b577b888dc5f9d93c2e4d3bf177
Stats:     23 lines in 3 files changed: 17 ins; 2 del; 4 mod

8294461: wrong effectively final determination by javac

Reviewed-by: vromero

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

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