RFR: 8294461: wrong effectively final determination by javac [v2]

[Archie L. Cobbs]

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

Archie L. Cobbs has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains two additional commits since the last revision:

 - Merge branch 'master' into JDK-8294461
 - A for loop variable is not effectively final even if the loop never increments.

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/10856/files
  - new: https://git.openjdk.org/jdk/pull/10856/files/449a5efb..014d67d3

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=10856&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=10856&range=00-01

  Stats: 25053 lines in 338 files changed: 1115 ins; 23570 del; 368 mod
  Patch: https://git.openjdk.org/jdk/pull/10856.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/10856/head:pull/10856

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