RFR: 8341782: Allow lambda capture of basic for() loop variables as with enhanced for()

[Maurizio Cimadamore]

On Wed, 9 Oct 2024 15:38:09 GMT, Archie Cobbs <acobbs at openjdk.org> wrote:

> * We would need a new flag that says "we are currently in the for() loop body" and not, for example, in the step, where we should leave the flag alone.

Well, my idea for doing that was to just set the flag on the loop variables before scanning the loop body (as you seem to do in the patch you show)

> 
> * We would also need to ensure we don't get confused with nested `for()` loops. So the aforementioned flag can't just be a boolean (because otherwise which `for()` loop are we talking about?). For example, a symbol address range instead.

This I don't get. If you have a loop nested inside another loop, you want assignments in the inner loop to affect whether the outer loop variable is considered a "loop capturable variable" or not. So I'm not sure - it seems to me `letInit` should happily remove flags for any variable it sees marked with `FOR_LOOP_BODY_MAY_CAPTURE` (as that is surely a variable of a for loop and we must be in that loop's body for the variable to have that flag set?)

>> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java line 3486:
>> 
>>> 3484:             // Collect the for() loop iteration variables with the FOR_LOOP_BODY_MAY_CAPTURE flag
>>> 3485:             HashSet<VarSymbol> capturableForLoopVars = null;
>>> 3486:             if (tree.init != null) {
>> 
>> This is a bit unfortunate. In principle, a variable has `FOR_LOOP_BODY_MAY_CAPTURE` should just be  `EFFECTIVELY_FINAL` - at least in the context of the for loop body. I suppose there's still a problem that we'd have to unset `EFFECTIVELY_FINAL` after the loop body... but I wonder, do we care? After all, we have already flow-analyzed the other for-loop parts (condition and step), so perhaps we just set `EFFECTIVELY_FINAL` before scanning the loop body, and leave it there?
>
> We can't really mess with `EFFECTIVELY_FINAL` at all, because we need to preserve the current behavior in the init, condition, and step, so that you still get an error in this example:
> 
> for (int i = 0;
>   i++ + ((IntSupplier)() -> i).getAsInt() < 3; ) {      // error - i is not effectively final
>     ((IntSupplier)() -> i).getAsInt();                  // ok - i is effectively final in the body
> }
> 
> Put another way, I don't see how we could avoid having two separate "effectively final" flags - one for the loop body, and the original one for everything else, at least, not without combining `AssignAnalyzer` and `CaptureAnalyzer`.

True. The sequencing between the two visitors is problematic here.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21415#discussion_r1794242314
PR Review Comment: https://git.openjdk.org/jdk/pull/21415#discussion_r1794244820