Skip to content
Permalink
Browse files
8260593: javac can skip a temporary local variable when pattern match…
…ing over a local variable

Reviewed-by: vromero
  • Loading branch information
Jan Lahoda committed Feb 8, 2021
1 parent deb0544 commit d0a8f2f737cdfc1ae742d47f2dd4f2bbc03f4398
Show file tree
Hide file tree
Showing 5 changed files with 220 additions and 15 deletions.
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2017, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2017, 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
@@ -26,13 +26,14 @@
package com.sun.tools.javac.comp;

import com.sun.tools.javac.code.Flags;
import com.sun.tools.javac.code.Kinds;
import com.sun.tools.javac.code.Kinds.Kind;
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.Symbol.BindingSymbol;
import com.sun.tools.javac.code.Symbol.VarSymbol;
import com.sun.tools.javac.code.Symtab;
import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.code.Types;
import com.sun.tools.javac.tree.JCTree;
import com.sun.tools.javac.tree.JCTree.JCAssign;
import com.sun.tools.javac.tree.JCTree.JCBinary;
import com.sun.tools.javac.tree.JCTree.JCConditional;
@@ -49,7 +50,6 @@
import com.sun.tools.javac.tree.JCTree.Tag;
import com.sun.tools.javac.tree.TreeMaker;
import com.sun.tools.javac.tree.TreeTranslator;
import com.sun.tools.javac.util.Assert;
import com.sun.tools.javac.util.Context;
import com.sun.tools.javac.util.ListBuffer;
import com.sun.tools.javac.util.Log;
@@ -68,6 +68,7 @@
import com.sun.tools.javac.tree.JCTree.JCLambda;
import com.sun.tools.javac.tree.JCTree.JCStatement;
import com.sun.tools.javac.tree.JCTree.LetExpr;
import com.sun.tools.javac.tree.TreeInfo;
import com.sun.tools.javac.util.List;
import java.util.HashMap;

@@ -152,15 +153,23 @@ public void visitTypeTest(JCInstanceOf tree) {
//E instanceof T N
//=>
//(let T' N$temp = E; N$temp instanceof T && (N = (T) N$temp == (T) N$temp))
Symbol exprSym = TreeInfo.symbol(tree.expr);
JCBindingPattern patt = (JCBindingPattern)tree.pattern;
VarSymbol pattSym = patt.var.sym;
Type tempType = tree.expr.type.hasTag(BOT) ?
syms.objectType
: tree.expr.type;
VarSymbol temp = new VarSymbol(pattSym.flags() | Flags.SYNTHETIC,
names.fromString(pattSym.name.toString() + target.syntheticNameChar() + "temp"),
tempType,
patt.var.sym.owner);
VarSymbol temp;
if (exprSym != null &&
exprSym.kind == Kind.VAR &&
exprSym.owner.kind.matches(Kinds.KindSelector.VAL_MTH)) {
temp = (VarSymbol) exprSym;
} else {
temp = new VarSymbol(pattSym.flags() | Flags.SYNTHETIC,
names.fromString(pattSym.name.toString() + target.syntheticNameChar() + "temp"),
tempType,
patt.var.sym.owner);
}
JCExpression translatedExpr = translate(tree.expr);
Type castTargetType = types.boxedTypeOrType(pattSym.erasure(types));

@@ -176,8 +185,10 @@ public void visitTypeTest(JCInstanceOf tree) {
nestedLE.setType(syms.booleanType);
result = makeBinary(Tag.AND, (JCExpression)result, nestedLE);
}
result = make.at(tree.pos).LetExpr(make.VarDef(temp, translatedExpr), (JCExpression)result).setType(syms.booleanType);
((LetExpr) result).needsCond = true;
if (temp != exprSym) {
result = make.at(tree.pos).LetExpr(make.VarDef(temp, translatedExpr), (JCExpression)result).setType(syms.booleanType);
((LetExpr) result).needsCond = true;
}
} else {
super.visitTypeTest(tree);
}
@@ -70,8 +70,8 @@ void run() throws Exception {
codeAttr.attributes.map.get(Attribute.RuntimeInvisibleTypeAnnotations);
RuntimeInvisibleTypeAnnotations_attribute annotations =
(RuntimeInvisibleTypeAnnotations_attribute) annoAttr;
String expected = "[@Annotations$DTA; pos: [LOCAL_VARIABLE, {start_pc = 35, length = 7, index = 1}, pos = -1], " +
"@Annotations$TA; pos: [LOCAL_VARIABLE, {start_pc = 56, length = 7, index = 1}, pos = -1]]";
String expected = "[@Annotations$DTA; pos: [LOCAL_VARIABLE, {start_pc = 31, length = 7, index = 1}, pos = -1], " +
"@Annotations$TA; pos: [LOCAL_VARIABLE, {start_pc = 50, length = 7, index = 1}, pos = -1]]";
String actual = Arrays.toString(annotations.annotations);
if (!expected.equals(actual)) {
throw new AssertionError("Unexpected type annotations: " +
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2017, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2017, 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
@@ -231,6 +231,11 @@ public void run() {
throw new AssertionError();
}

//binding in an anonymous class:
if (!(invokeOnce("") instanceof String s)) {
throw new AssertionError();
}

System.out.println("BindingsTest1 complete");
}

@@ -240,4 +245,13 @@ interface VoidPredicate {
static boolean id(boolean b) {
return b;
}
private static boolean invoked;
static Object invokeOnce(Object val) {
if (invoked) {
throw new IllegalStateException();
} else {
invoked = true;
return val;
}
}
}
@@ -0,0 +1,182 @@
/*
* 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 8260593
* @summary Verify that a temporary storage variable is or is not used as needed when pattern matching.
* @library /tools/lib /tools/javac/lib
* @modules
* jdk.compiler/com.sun.tools.javac.api
* jdk.compiler/com.sun.tools.javac.file
* jdk.compiler/com.sun.tools.javac.main
* jdk.compiler/com.sun.tools.javac.util
* @build toolbox.ToolBox toolbox.JavacTask
* @build combo.ComboTestHelper
* @compile LocalVariableReuse.java
* @run main LocalVariableReuse
*/

import combo.ComboInstance;
import combo.ComboParameter;
import combo.ComboTask;
import combo.ComboTestHelper;
import java.io.IOException;
import javax.tools.JavaFileObject;
import toolbox.ToolBox;

public class LocalVariableReuse extends ComboInstance<LocalVariableReuse> {
protected ToolBox tb;

LocalVariableReuse() {
super();
tb = new ToolBox();
}

public static void main(String... args) throws Exception {
new ComboTestHelper<LocalVariableReuse>()
.withDimension("CODE", (x, code) -> x.code = code, Code.values())
.run(LocalVariableReuse::new);
}

private Code code;

private static final String MAIN_TEMPLATE =
"""
public class Test {
#{CODE}
}
""";

@Override
protected void doWork() throws Throwable {
ComboTask task = newCompilationTask()
.withSourceFromTemplate(MAIN_TEMPLATE, pname -> switch (pname) {
case "CODE" -> code;
default -> throw new UnsupportedOperationException(pname);
});

task.withOption("-printsource");
task.generate(result -> {
for (JavaFileObject out : result.get()) {
try {
String actualDesugared = out.getCharContent(false).toString();
boolean hasTempVar = actualDesugared.contains("$temp");
if (hasTempVar != code.useTemporaryVariable) {
throw new AssertionError("Expected temporary variable: " + code.useTemporaryVariable +
", but got: " + actualDesugared);
}
} catch (IOException ex) {
throw new AssertionError(ex);
}
}
});
}

public enum Code implements ComboParameter {
LOCAL_VARIABLE(
"""
private boolean test() {
Object o = null;
return o instanceof String s && s.length() > 0;
}
""", false),
PARAMETER(
"""
private boolean test(Object o) {
return o instanceof String s && s.length() > 0;
}
""", false),
LAMBDA_PARAMETER(
"""
private void test(Object o) {
I i = o -> o instanceof String s && s.length() > 0;
interface I {
public boolean run(Object o);
}
""", false),
EXCEPTION(
"""
private boolean test() {
try {
throw new Exception();
} catch (Exception o) {
return o instanceof RuntimeException re && re.getMessage() != null;
}
}
""", false),
RESOURCE(
"""
private boolean test() throws Exception {
try (AutoCloseable r = null) {
return r instanceof java.io.InputStream in && in.read() != (-1);
} catch (Exception o) {
}
}
""", false),
FIELD("""
private Object o;
private boolean test() {
return o instanceof String s && s.length() > 0;
}
""",
true),
FINAL_FIELD("""
private final Object o;
private boolean test() {
return o instanceof String s && s.length() > 0;
}
""",
true),
ARRAY_ACCESS("""
private boolean test() {
Object[] o = null;
return o[0] instanceof String s && s.length() > 0;
}
""",
true),
METHOD_INVOCATION("""
private boolean test() {
return get() instanceof String s && s.length() > 0;
}
private Object get() {
return null;
}
""",
true),
;
private final String body;
private final boolean useTemporaryVariable;

private Code(String body, boolean useTemporaryVariable) {
this.body = body;
this.useTemporaryVariable = useTemporaryVariable;
}

@Override
public String expand(String optParameter) {
return body;
}
}

}
@@ -67,11 +67,9 @@ void checkClassFile(File file)
.get();
String expectedInstructions = """
aload_1
astore_3
aload_3
instanceof
ifeq
aload_3
aload_1
checkcast
astore_2
aload_2

1 comment on commit d0a8f2f

@openjdk-notifier
Copy link

@openjdk-notifier openjdk-notifier bot commented on d0a8f2f Feb 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.