Skip to content
This repository has been archived by the owner. It is now read-only.

8267610: NPE at at jdk.compiler/com.sun.tools.javac.jvm.Code.emitop #59

Closed
wants to merge 3 commits into from
Closed
Changes from 1 commit
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -205,8 +205,11 @@ public void visitTypeTest(JCInstanceOf tree) {

JCExpression translatedExpr = translate(tree.expr);
Type principalType = principalType((JCPattern) tree.pattern);
VarSymbol actualVar = (currentValue.flags() & Flags.MATCH_BINDING) != 0
? (VarSymbol) TreeInfo.symbol(translatedExpr)
: currentValue;
result = makeBinary(Tag.AND,
makeTypeTest(make.Ident(currentValue), make.Type(principalType)),
makeTypeTest(make.Ident(actualVar), make.Type(principalType)),
(JCExpression) this.<JCTree>translate(tree.pattern));
if (currentValue != exprSym) {
result = make.at(tree.pos).LetExpr(make.VarDef(currentValue, translatedExpr),
@@ -227,10 +230,13 @@ public void visitBindingPattern(JCBindingPattern tree) {
BindingSymbol binding = (BindingSymbol) tree.var.sym;
Type castTargetType = principalType(tree);
VarSymbol bindingVar = bindingContext.bindingDeclared(binding);
VarSymbol actualVar = (currentValue.flags() & Flags.MATCH_BINDING) != 0
? bindingContext.getBindingFor((BindingSymbol) currentValue)
: currentValue;

if (bindingVar != null) {
JCAssign fakeInit = (JCAssign)make.at(TreeInfo.getStartPos(tree)).Assign(
make.Ident(bindingVar), convert(make.Ident(currentValue), castTargetType)).setType(bindingVar.erasure(types));
make.Ident(bindingVar), convert(make.Ident(actualVar), castTargetType)).setType(bindingVar.erasure(types));
LetExpr nestedLE = make.LetExpr(List.of(make.Exec(fakeInit)),
make.Literal(true));
nestedLE.needsCond = true;
@@ -0,0 +1,72 @@
/*
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

/*
* @test
* @bug 8267610
* @summary LambdaToMethod cannot capture pattern variables. So the TransPatterns should
* transform the pattern variables and symbols to normal variables and symbols.
* @compile --enable-preview -source ${jdk.version} LambdaCannotCapturePatternVariables.java
* @run main/othervm --enable-preview LambdaCannotCapturePatternVariables
*/

import java.util.function.Supplier;

public class LambdaCannotCapturePatternVariables {

public static void main(String[] args) {
var testVar = new LambdaCannotCapturePatternVariables();
testVar.testInstanceOfPatternVariable(Integer.valueOf(1));
testVar.testSwitchPatternVariable(Integer.valueOf(1));
testVar.test(Integer.valueOf(1));
}

public Integer testInstanceOfPatternVariable(Object x) {
if(x instanceof Number y) {
return ((Supplier<Integer>) (() -> {
return ((y instanceof Integer z) ? z : 1);
})).get();
}
return null;
}

public Integer testSwitchPatternVariable(Object x) {
switch (x) {
case Number n: {
return ((Supplier<Integer>) (() -> {
return ((n instanceof Integer i) ? i : 1);
})).get();
}
default: return null;
}
}

// Provided by the user
public Integer test(Object x) {
Integer bar = 1;
return ((x instanceof Number y) ?
((Supplier<Integer>) (() -> {
return ((y instanceof Integer z) ? z : bar);
})).get() : bar);
}
}
@@ -0,0 +1,114 @@
/*
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

/*
* @test
* @bug 8268748
* @summary Javac generates error opcodes when using nest pattern variables
* @library /tools/lib
* @modules jdk.compiler/com.sun.tools.javac.api
* jdk.compiler/com.sun.tools.javac.main
* jdk.jdeps/com.sun.tools.classfile
* @build toolbox.ToolBox toolbox.JavacTask
* @run main NestedPatternVariablesBytecode
*/

import java.nio.file.Path;
import java.util.Arrays;
import java.util.List;
import java.util.stream.StreamSupport;

import com.sun.tools.classfile.ClassFile;
import com.sun.tools.classfile.ConstantPoolException;
import com.sun.tools.classfile.Method;
import com.sun.tools.classfile.Attribute;
import com.sun.tools.classfile.Code_attribute;
import com.sun.tools.classfile.Instruction;

import toolbox.JavacTask;
import toolbox.TestRunner;
import toolbox.ToolBox;

public class NestedPatternVariablesBytecode extends TestRunner {
private static final String JAVA_VERSION = System.getProperty("java.specification.version");
private static final String TEST_METHOD = "test";

ToolBox tb;
ClassFile cf;

public NestedPatternVariablesBytecode() {
super(System.err);
tb = new ToolBox();
}

public static void main(String[] args) throws Exception {
NestedPatternVariablesBytecode t = new NestedPatternVariablesBytecode();
t.runTests();
}

@Test
public void testNestedPatternVariablesBytecode() throws Exception {
String code = """
class NestedPatterVariablesTest {
String test(Object o) {
if (o instanceof (CharSequence cs && cs instanceof String s)) {
return s;
}
return null;
}
}""";
Path curPath = Path.of(".");
new JavacTask(tb)
.options("--enable-preview", "-source", JAVA_VERSION)
.sources(code)
.outdir(curPath)
.run();

cf = ClassFile.read(curPath.resolve("NestedPatterVariablesTest.class"));
Method testMethod = Arrays.stream(cf.methods)
.filter(m -> isTestMethod(m))
.findAny()
.get();
Code_attribute code_attribute = (Code_attribute) testMethod.attributes.get(Attribute.Code);

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

This comment has been minimized.

@vicente-romero-oracle

vicente-romero-oracle Jun 24, 2021
Contributor

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?

This comment has been minimized.

@lgxbslgx

lgxbslgx Jun 24, 2021
Author Member

@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.

This comment has been minimized.

@vicente-romero-oracle

vicente-romero-oracle Jun 24, 2021
Contributor

@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

"ifeq", "aload_2", "checkcast", "astore_3", "aload_3", "areturn", "aconst_null", "areturn");
tb.checkEqual(expectedCode, actualCode);
}

boolean isTestMethod(Method m) {
try {
return TEST_METHOD.equals(m.getName(cf.constant_pool));
} catch (ConstantPoolException e) {
throw new IllegalStateException(e);
}
}

List<String> getCodeInstructions(Code_attribute code) {
return StreamSupport.stream(code.getInstructions().spliterator(), false)
.map(Instruction::getMnemonic)
.toList();
}
}