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

[Vicente Romero]

On Thu, 24 Jun 2021 16:08:20 GMT, Vicente Romero <vromero at openjdk.org> wrote:

>> 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?

> @vicente-romero-oracle Thanks for your review.
> 
> In this test, if the bug is not fixed which means the `aload_2` would be `aload_1`, the code runs well too.
> The `aload_1` or `aload_2` don't effect the result so that we can't check it by executing it.
> There are some similar situations in the past when we want to verify the class files.
> These tests would also fail if we change the logic of the code generation.
> 
> Another way is that only verify if the `aload_2` is at the right place instead of verify all the instruments.
> But it also need to be revisited and revised if the logic of the code generation is changed in the future.
> It seems a unavoidable issue about verifing the class files.

I see, thanks for the clarification

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

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