RFR: 8336786: VerifyError with lambda capture and enclosing instance references

[Maurizio Cimadamore]

On Fri, 19 Jul 2024 16:13:46 GMT, Liam Miller-Cushon <cushon at openjdk.org> wrote:

> Hi, please consider this fix for [JDK-8336786](https://bugs.openjdk.org/browse/JDK-8336786). After [JDK-8334037](https://bugs.openjdk.org/browse/JDK-8334037) the handling of capturing `this` in lambdas does not handle synthetic `this` variables declared in supertypes in different packages. This change adjusts the lowering of `this` references to be owned by the enclosing class, instead of the superclass, so they are handle correctly be the capture logic in `capturedDecl` in `LambdaToMethod`.

Marked as reviewed by mcimadamore (Reviewer).

The fix looks reasonable. I came up with something slightly different:
https://github.com/openjdk/jdk/compare/master...mcimadamore:jdk:lambda_encl_capture?expand=1

(perhaps some of the changes in LambdaToMethod from that fix would still be appreciated, such as the extra assert, and the removal of the subclass test, as I don't think that ever does anything).

Where I'm more unsure is in noting that Lower has two modes of capturing an enclosing `this`: a "precise" one, which calls `isMemberOf` and a more fuzzy one, which calls `isSubclass` on the owner. Inner class creation is the only case where we seem to use the fuzzy match.

But the lambda code _always_ uses membership. Effectively, the old `LambdaToMethod` code in `visitNewClass`, attempted to scan all the enclosing scopes, looking a proper owner of the to-be captured `this`. Failing that, we would just go ahead and add the captured enclosing `this` anyway.

I'm sure these little discrepancies are all accidents waiting to happen, in the sense that probably there are examples where Lower and LambdaToMethod would pick different enclosing `this`. This is kind of an unavoidable consequence of having the capture logic being defined in two unrelated places. So, not sure we've seen the end of this, but your fix seems a good step forward.

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

PR Review: https://git.openjdk.org/jdk/pull/20259#pullrequestreview-2188680615
PR Comment: https://git.openjdk.org/jdk/pull/20259#issuecomment-2239623408