RFR: 8334037: Local class creation in lambda in pre-construction context crashes javac
Maurizio Cimadamore
mcimadamore at openjdk.org
Fri Jun 21 17:40:36 UTC 2024
A long, long time ago, we used to have [two competing translation strategies](https://hg.openjdk.org/lambda/lambda/langtools/rev/a7e6203a332d?revcount=240) for lambda expressions. One strategy was to translate lambdas to anonymous inner classes. The other was to translate them as invokedynamic, pointing to a synthetic method containing the lambda body.
The simpler inner class traslation strategy predates the more complex indy-based one. Since that translation strategy was based on inner classes, it made a lot of sense to do that translation _before_ `Lower`, so that any anon class generated by javac could then be translated correctly (and take into account issues such as local variable capture).
When we added `LambdaToMethod`, it seemed natural to add it in the same step (e.g. before `Lower`) - after all, users could toggle between the two translation strategies with a switch, so it made sense to keep them close together.
But, as we got rid of the inner class translation strategy, and doubled down on `LambdaToMethod` we have started to see some artifacts. For instance, if a lambda expression contains a local class declaration, `LambdaToMethod` has to [guess](https://github.com/openjdk/jdk/blob/master/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java#L1480) which captured variables might get captured later in `Lower`, and re-captured them in the lambda. Now, guessing the full set of local captures is not that easy, as the set depends on the state in which `Lower` is in: more specifically, sometimes `Lower` might decide to also capture the enclosing this constructor parameter, when in early-construction context. Of course `LambdaToMethod` didn't know any of this, and over the years several [workarounds](https://github.com/openjdk/jdk/blob/master/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java#L1244) have been introduced.
Unfortunately, JEP 482 is in collision course with the current compiler pipeline: `Lower` is the only place in the compiler which knows how to correctly translate references to enclosing instances. Trying to mimic what `Lower` is doing in `LambdaToMethod`, doesn't scale, as the [many bugs](https://bugs.openjdk.org/issues/?jql=labels%20%3D%20javac-pre-capture) that were discovered in this area show.
For this reason, instead of keep playing whack-a-mole, I think the time has come to finally move `LambdaToMethod` _after_ `Lower`. This move makes `LambdaToMethod` the _last_ compiler step (before `Gen`).
The move is relatively straightforward, but there are some caveats:
* the new `LambdaToMethod` deals with separate classes, not a single toplevel one (as it now occurs _after_ lowering);
* `Lower` needs a to override `visitLambda`, so that it can properly lower all the lambda return expressions;
* the code to desugars complex method references into lambdas has been moved from `LambdaToMethod` to Lower (so that we can fixup references to inner classes etc.) - this means that `LambdaToMethod` now only has to worry about lambdas and *simple* method references;
* the lambda deserialization method generated by `LambdaToMethod` contains string in switch, and implicitly requires boxing, so we need to explictly lower that (given `Lower` has already ran, we need to trigger the lowering of the method manually)
* some code generated by `Lower` is not stable (this is a pre-existing issue), so lambda deduplication fails in new ways
This PR addresses all the issues above and removes all the workaround we have accumulated over the years to make up for the fact that `LambdaToMethod` runs at the wrong time. In doing so, we fix many issues related to JEP 482 (thanks to @archiecobbs for the tests which I've included here).
-------------
Commit messages:
- Remove spurious changes in `Gen`
- Remove spurious changes
- Add tests
- Merge branch 'master' into cleanup_capture
- Add comment
- Use LetExpr to force eager computaton of method reference receiver.
- More unused code cleanup
- Fix remaining dedup issues
- Fix dedup - part 1
- Restore desugaring of method reference when using -source 14
- ... and 7 more: https://git.openjdk.org/jdk/compare/7b3a96d5...75a52ffb
Changes: https://git.openjdk.org/jdk/pull/19836/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19836&range=00
Issue: https://bugs.openjdk.org/browse/JDK-8334037
Stats: 1516 lines in 15 files changed: 765 ins; 715 del; 36 mod
Patch: https://git.openjdk.org/jdk/pull/19836.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19836/head:pull/19836
PR: https://git.openjdk.org/jdk/pull/19836
More information about the compiler-dev
mailing list