[jdk17] RFR: 8267610: NPE at at jdk.compiler/com.sun.tools.javac.jvm.Code.emitop [v2]

[Vicente Romero]

On Wed, 23 Jun 2021 18:34:58 GMT, Guoxiong Li <gli at openjdk.org> wrote:

>> Hi all,
>> 
>> Currently, the class TransPatterns sometimes doesn't transform the `pattern variables` and `pattern symbols` to the normal variables and symbols, especially the places where the pattern variables are used.
>> The following phases, such as LambdaToMethod, Lower and Gen, may crash or generate some wrong results.
>> 
>> The known issues are [JDK-8267610](https://bugs.openjdk.java.net/browse/JDK-8267610) and [JDK-8268748](https://bugs.openjdk.java.net/browse/JDK-8268748).
>> 
>> **JDK-8267610 is an issue that pattern symbol causes the compiler to crash.**
>> 
>> During transforming the InstanceOfTree (JCInstanceOf) , the `BindingSymbol`  instead of the `VarSymbol` is used to make the new `JCIdent` and `JCBinary` trees. At the phase LambdaToMethod, the compiler can't capture this variable so that the lambda method has uncorrect parameters. So at the phase Gen, the compiler crashes because of NPE.
>> 
>> **JDK-8268748 is an issue that pattern symbol causes that the compiler generates wrong bytecodes.**
>> 
>> When transforming the BindingPatternTree (JCBindingPattern), the `BindingSymbol` is also handled uncorrectly and used to make the new `JCIdent` and `JCAssign` trees. At the phase Gen, the compiler find the wrong variables, so that the wrong bytecodes are generated.
>> 
>> These two issues are similar and influence each other. So I solve them at one patch.
>> The lines 208-212 are to solve JDK-8267610 with the test `LambdaCannotCapturePatternVariables`. 
>> The lines 233-239 are to solve JDK-8268748 with the test `NestedPatternVariablesBytecode`.
>> 
>> If lines 208-212 are not included, the test  `NestedPatternVariablesBytecode` can't pass. 
>> If lines 233-239 are not included, the test `LambdaCannotCapturePatternVariables` can't pass. 
>> So I put them together.
>> 
>> Thanks for taking the time to review.
>> 
>> Best Regards,
>> -- Guoxiong.
>
> Guoxiong Li 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 three additional commits since the last revision:
> 
>  - Merge remote-tracking branch 'origin/master' into JDK-8267610
>  - Set currentValue after translating tree.expr
>  - JDK-8267610 and JDK-8268748

test/langtools/tools/javac/patterns/NestedPatternVariablesBytecode.java line 96:

> 94:         List<String> actualCode = getCodeInstructions(code_attribute);
> 95:         List<String> expectedCode = Arrays.asList(
> 96:                 "aload_1", "instanceof", "ifeq", "aload_1", "checkcast", "astore_2", "aload_2", "instanceof",

not sure I like this code pattern check, if we change the generated code for whatever reason we will need to revisit this test. Is there other way of testing the same? like executing a code and checking its result(s) or throwing an exception if something unexpected happened?

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

PR: https://git.openjdk.java.net/jdk17/pull/59