8009170: Regression: javac generates redundant bytecode in assignop involving arrays
Reviewed-by: mcimadamore
--- a/langtools/src/share/classes/com/sun/tools/javac/comp/Lower.java Thu Mar 07 10:04:28 2013 +0000
+++ b/langtools/src/share/classes/com/sun/tools/javac/comp/Lower.java Thu Mar 07 10:12:13 2013 +0000
@@ -3158,38 +3158,59 @@
}
public void visitAssignop(final JCAssignOp tree) {
+ JCTree lhsAccess = access(TreeInfo.skipParens(tree.lhs));
final boolean boxingReq = !tree.lhs.type.isPrimitive() &&
tree.operator.type.getReturnType().isPrimitive();
- // boxing required; need to rewrite as x = (unbox typeof x)(x op y);
- // or if x == (typeof x)z then z = (unbox typeof x)((typeof x)z op y)
- // (but without recomputing x)
- JCTree newTree = abstractLval(tree.lhs, new TreeBuilder() {
- public JCTree build(final JCTree lhs) {
- JCTree.Tag newTag = tree.getTag().noAssignOp();
- // Erasure (TransTypes) can change the type of
- // tree.lhs. However, we can still get the
- // unerased type of tree.lhs as it is stored
- // in tree.type in Attr.
- Symbol newOperator = rs.resolveBinaryOperator(tree.pos(),
- newTag,
- attrEnv,
- tree.type,
- tree.rhs.type);
- JCExpression expr = (JCExpression)lhs;
- if (expr.type != tree.type)
- expr = make.TypeCast(tree.type, expr);
- JCBinary opResult = make.Binary(newTag, expr, tree.rhs);
- opResult.operator = newOperator;
- opResult.type = newOperator.type.getReturnType();
- JCExpression newRhs = boxingReq ?
- make.TypeCast(types.unboxedType(tree.type),
- opResult) :
+ if (boxingReq || lhsAccess.hasTag(APPLY)) {
+ // boxing required; need to rewrite as x = (unbox typeof x)(x op y);
+ // or if x == (typeof x)z then z = (unbox typeof x)((typeof x)z op y)
+ // (but without recomputing x)
+ JCTree newTree = abstractLval(tree.lhs, new TreeBuilder() {
+ public JCTree build(final JCTree lhs) {
+ JCTree.Tag newTag = tree.getTag().noAssignOp();
+ // Erasure (TransTypes) can change the type of
+ // tree.lhs. However, we can still get the
+ // unerased type of tree.lhs as it is stored
+ // in tree.type in Attr.
+ Symbol newOperator = rs.resolveBinaryOperator(tree.pos(),
+ newTag,
+ attrEnv,
+ tree.type,
+ tree.rhs.type);
+ JCExpression expr = (JCExpression)lhs;
+ if (expr.type != tree.type)
+ expr = make.TypeCast(tree.type, expr);
+ JCBinary opResult = make.Binary(newTag, expr, tree.rhs);
+ opResult.operator = newOperator;
+ opResult.type = newOperator.type.getReturnType();
+ JCExpression newRhs = boxingReq ?
+ make.TypeCast(types.unboxedType(tree.type), opResult) :
opResult;
- return make.Assign((JCExpression)lhs, newRhs).setType(tree.type);
- }
- });
- result = translate(newTree);
+ return make.Assign((JCExpression)lhs, newRhs).setType(tree.type);
+ }
+ });
+ result = translate(newTree);
+ return;
+ }
+ tree.lhs = translate(tree.lhs, tree);
+ tree.rhs = translate(tree.rhs, tree.operator.type.getParameterTypes().tail.head);
+
+ // If translated left hand side is an Apply, we are
+ // seeing an access method invocation. In this case, append
+ // right hand side as last argument of the access method.
+ if (tree.lhs.hasTag(APPLY)) {
+ JCMethodInvocation app = (JCMethodInvocation)tree.lhs;
+ // if operation is a += on strings,
+ // make sure to convert argument to string
+ JCExpression rhs = (((OperatorSymbol)tree.operator).opcode == string_add)
+ ? makeString(tree.rhs)
+ : tree.rhs;
+ app.args = List.of(rhs).prependList(app.args);
+ result = app;
+ } else {
+ result = tree;
+ }
}
/** Lower a tree of the form e++ or e-- where e is an object type */
--- a/langtools/test/tools/javac/7167125/DiffResultAfterSameOperationInnerClasses.java Thu Mar 07 10:04:28 2013 +0000
+++ b/langtools/test/tools/javac/7167125/DiffResultAfterSameOperationInnerClasses.java Thu Mar 07 10:12:13 2013 +0000
@@ -34,27 +34,60 @@
private int j = 1;
public String s1 = "Hi, ";
private String s2 = "Hi, ";
+ public int arr1[] = new int[]{1};
+ public int arr2[] = new int[]{1};
public static void main(String[] args) {
- InnerClass inner =
- new DiffResultAfterSameOperationInnerClasses().new InnerClass();
- if (!inner.test()) {
+ DiffResultAfterSameOperationInnerClasses theTest =
+ new DiffResultAfterSameOperationInnerClasses();
+ InnerClass inner = theTest.new InnerClass();
+ if (!inner.test1()) {
+ throw new AssertionError("Different results after same calculation");
+ }
+
+ theTest.resetVars();
+ if (!inner.test2()) {
throw new AssertionError("Different results after same calculation");
}
}
+ void resetVars() {
+ i = 1;
+ j = 1;
+ s1 = "Hi, ";
+ s2 = "Hi, ";
+ arr1[0] = 1;
+ arr2[0] = 1;
+ }
+
class InnerClass {
- public boolean test() {
+ public boolean test1() {
i += i += 1;
j += j += 1;
+ arr1[0] += arr1[0] += 1;
+ arr2[0] += arr2[0] += 1;
+
s1 += s1 += "dude";
s2 += s2 += "dude";
- System.out.println("s1 = " + s1);
- System.out.println("s2 = " + s2);
+ return (i == j && i == 3 &&
+ arr1[0] == arr2[0] && arr2[0] == 3 &&
+ s1.equals(s2) && s1.endsWith("Hi, Hi, dude"));
+ }
+
+ public boolean test2() {
+ (i) += (i) += 1;
+ (j) += (j) += 1;
+
+ (arr1[0])+= (arr1[0]) += 1;
+ (arr2[0])+= (arr2[0]) += 1;
+
+ (s1) += (s1) += "dude";
+ (s2) += (s2) += "dude";
return (i == j && i == 3 &&
+ arr1[0] == arr2[0] && arr2[0] == 3 &&
s1.equals(s2) && s1.endsWith("Hi, Hi, dude"));
}
}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/8009170/RedundantByteCodeInArrayTest.java Thu Mar 07 10:12:13 2013 +0000
@@ -0,0 +1,71 @@
+/*
+ * Copyright (c) 2013, 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 8009170
+ * @summary Regression: javac generates redundant bytecode in assignop involving
+ * arrays
+ * @run main RedundantByteCodeInArrayTest
+ */
+
+import java.io.File;
+import java.io.IOException;
+
+import com.sun.tools.classfile.Attribute;
+import com.sun.tools.classfile.ClassFile;
+import com.sun.tools.classfile.Code_attribute;
+import com.sun.tools.classfile.Code_attribute.InvalidIndex;
+import com.sun.tools.classfile.ConstantPool;
+import com.sun.tools.classfile.ConstantPoolException;
+import com.sun.tools.classfile.Descriptor.InvalidDescriptor;
+import com.sun.tools.classfile.Method;
+
+public class RedundantByteCodeInArrayTest {
+ public static void main(String[] args)
+ throws IOException, ConstantPoolException, InvalidDescriptor, InvalidIndex {
+ new RedundantByteCodeInArrayTest()
+ .checkClassFile(new File(System.getProperty("test.classes", "."),
+ RedundantByteCodeInArrayTest.class.getName() + ".class"));
+ }
+
+ void arrMethod(int[] array, int p, int inc) {
+ array[p] += inc;
+ }
+
+ void checkClassFile(File file)
+ throws IOException, ConstantPoolException, InvalidDescriptor, InvalidIndex {
+ ClassFile classFile = ClassFile.read(file);
+ ConstantPool constantPool = classFile.constant_pool;
+
+ //lets get all the methods in the class file.
+ for (Method method : classFile.methods) {
+ if (method.getName(constantPool).equals("arrMethod")) {
+ Code_attribute code = (Code_attribute) method.attributes
+ .get(Attribute.Code);
+ if (code.max_locals > 4)
+ throw new AssertionError("Too many locals for method arrMethod");
+ }
+ }
+ }
+}