RFR: 8258897: wrong translation of capturing local classes inside nested lambdas [v3]

[Jan Lahoda]

On Tue, 29 Dec 2020 04:53:10 GMT, Jia Yanwei <github.com+3896345+hltj at openjdk.org> wrote:

>> **Issue:**
>> `javac` crashes with a NPE when compiling such code:
>> - a local class captured a reference type local variable of a enclosed lambda (which contains multiple local variables with different types)
>> - instance the local class in a nested lambda
>> 
>> Affects all versions from Java 8 to 16-ea, here is a example output with OpenJDK 15.0.1:
>> 
>> An exception has occurred in the compiler (15.0.1). Please file a bug against the Java compiler via the Java bug reporting page (http://bugreport.java.com) after checking the Bug Database (http://bugs.java.com) for duplicates. Include your program, the following diagnostic, and the parameters passed to the Java compiler in your report. Thank you.
>> java.lang.NullPointerException: Cannot read field "sym" because "this.lvar[1]" is null
>>     at jdk.compiler/com.sun.tools.javac.jvm.Code.emitop0(Code.java:571)
>>     at jdk.compiler/com.sun.tools.javac.jvm.Items$LocalItem.load(Items.java:399)
>>     at jdk.compiler/com.sun.tools.javac.jvm.Gen.genArgs(Gen.java:889)
>>     at jdk.compiler/com.sun.tools.javac.jvm.Gen.visitNewClass(Gen.java:1942)
>>     at jdk.compiler/com.sun.tools.javac.tree.JCTree$JCNewClass.accept(JCTree.java:1800)
>>     at jdk.compiler/com.sun.tools.javac.jvm.Gen.genExpr(Gen.java:864)
>>     at jdk.compiler/com.sun.tools.javac.jvm.Gen.visitExec(Gen.java:1723)
>>     at jdk.compiler/com.sun.tools.javac.tree.JCTree$JCExpressionStatement.accept(JCTree.java:1532)
>>     at jdk.compiler/com.sun.tools.javac.jvm.Gen.genDef(Gen.java:597)
>>     at jdk.compiler/com.sun.tools.javac.jvm.Gen.genStat(Gen.java:632)
>>     at jdk.compiler/com.sun.tools.javac.jvm.Gen.genStat(Gen.java:618)
>>     at jdk.compiler/com.sun.tools.javac.jvm.Gen.genStats(Gen.java:669)
>>     at jdk.compiler/com.sun.tools.javac.jvm.Gen.visitBlock(Gen.java:1084)
>>     at jdk.compiler/com.sun.tools.javac.tree.JCTree$JCBlock.accept(JCTree.java:1047)
>>     at jdk.compiler/com.sun.tools.javac.jvm.Gen.genDef(Gen.java:597)
>>     at jdk.compiler/com.sun.tools.javac.jvm.Gen.genStat(Gen.java:632)
>>     at jdk.compiler/com.sun.tools.javac.jvm.Gen.genMethod(Gen.java:954)
>>     at jdk.compiler/com.sun.tools.javac.jvm.Gen.visitMethodDef(Gen.java:917)
>>     at jdk.compiler/com.sun.tools.javac.tree.JCTree$JCMethodDecl.accept(JCTree.java:893)
>>     at jdk.compiler/com.sun.tools.javac.jvm.Gen.genDef(Gen.java:597)
>>     at jdk.compiler/com.sun.tools.javac.jvm.Gen.genClass(Gen.java:2395)
>>     at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.genCode(JavaCompiler.java:756)
>>     at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.generate(JavaCompiler.java:1644)
>>     at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.generate(JavaCompiler.java:1612)
>>     at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.compile(JavaCompiler.java:973)
>>     at jdk.compiler/com.sun.tools.javac.main.Main.compile(Main.java:317)
>>     at jdk.compiler/com.sun.tools.javac.main.Main.compile(Main.java:176)
>>     at jdk.compiler/com.sun.tools.javac.Main.compile(Main.java:59)
>>     at jdk.compiler/com.sun.tools.javac.Main.main(Main.java:45)
>> printing javac parameters to: /tmp/javac.20201126_141249.args
>> 
>> The minimal reproducing code is:
>> 
>>  java
>> class Main {
>>     static Runnable runnable = () -> {
>>         boolean b0 = false;
>>         String s0 = "hello";
>> 
>>         class Local {
>>             int i = s0.length();
>>         }
>> 
>>         Runnable dummy = () -> new Local();
>>     };
>> }
>> 
>> If the captured variable is a primitive type, the code will compile but cause a runtime crash at startup. e.g.:
>> 
>>  java
>> public class CaptureInt {
>>     static Runnable runnable = () -> {
>>         boolean b0 = false;
>>         int i0 = 5;
>> 
>>         class Local {
>>             int i = i0 + 2;
>>         }
>> 
>>         Runnable dummy = () -> new Local();
>>     };
>> 
>>     public static void main(String args[]) {
>>     }
>> }
>> 
>> **Reason & Experimental solution:**
>> 
>> During compilation, the captured variable was correctly synthetized as a synthetic argument in the synthetic methods for the nested lambda after `desugar()`. 
>> But the synthetic argument was not used when loading free variables, and it always use the original symbol for the original variable as if not in a nested lambda.
>> My experimental solution is substituting the symbols in free variables that were captured as synthetic arguments in nested lambda.
>> I'm not sure whether the code style as well as the solution itself are appropriate. Welcome any advice and better solution.
>
> Jia Yanwei has updated the pull request incrementally with three additional commits since the last revision:
> 
>  - fix 8258897: wrong translation of capturing local classes inside nested lambdas
>  - rename test case directory name
>  - revert the experimental fix in 9dec0d2

Seems sensible to me. Some recommendations on improving the text comment format are inline.

test/langtools/tools/javac/lambda/8258897/CaptureVariables.java line 3:

> 1: import java.util.function.Supplier;
> 2: 
> 3: /**

It would be nice to tweak the test to so that:
-the comment with `@test` is first, and imports follow
-`@test` is augmented with `/nodynamiccopyright/`
-`@bug` is added
-`@summary` is added

(applies to both tests)

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

Marked as reviewed by jlahoda (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/1479