changeset 51264:9e35bb8ec76c lworld

8207341: [Lworld] Javac generates bad code for value constructors when expressions with side effects are involved
author sadayapalam
date Mon, 16 Jul 2018 20:28:32 +0530
parents 19b069125c1d
children 54f37cb89cb3
files src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/TransValues.java test/langtools/tools/javac/valhalla/lworld-values/SideEffectTest.java
diffstat 2 files changed, 107 insertions(+), 0 deletions(-) [+]
line wrap: on
line diff
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/TransValues.java	Mon Jul 16 10:34:51 2018 +0200
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/TransValues.java	Mon Jul 16 20:28:32 2018 +0530
@@ -86,6 +86,11 @@
     private Types types;
     private Names names;
 
+    /* Is an assignment undergoing translation just an assignment statement ?
+       Or is also a value ??
+    */
+    private boolean requireRVal;
+
     // class currently undergoing translation.
     private JCClassDecl currentClass;
 
@@ -113,6 +118,29 @@
         names = Names.instance(context);
     }
 
+    @SuppressWarnings("unchecked")
+    public <T extends JCTree> T translate(T tree, boolean requireRVal) {
+        boolean priorRequireRVal = this.requireRVal;
+        try {
+            this.requireRVal = requireRVal;
+            if (tree == null) {
+                return null;
+            } else {
+                tree.accept(this);
+                JCTree tmpResult = this.result;
+                this.result = null;
+                return (T)tmpResult; // XXX cast
+            }
+        } finally {
+             this.requireRVal = priorRequireRVal;
+        }
+    }
+
+    @Override
+    public <T extends JCTree> T translate(T tree) {
+        return translate(tree, true);
+    }
+
     public JCClassDecl translateTopLevelClass(JCClassDecl classDecl, TreeMaker make) {
         try {
             this.make = make;
@@ -271,6 +299,9 @@
             if (isInstanceFieldAccess(symbol)) {
                 final JCIdent facHandle = make.Ident(currentMethod.factoryProduct);
                 result = make.Assign(facHandle, make.WithField(make.Select(facHandle, symbol), translate(tree.rhs)).setType(currentClass.type)).setType(currentClass.type);
+                if (requireRVal) {
+                    result = make.Select(make.Parens((JCExpression) result).setType(currentClass.type), symbol);
+                }
                 return;
             }
         }
@@ -278,6 +309,16 @@
     }
 
     @Override
+    public void visitExec(JCExpressionStatement tree) {
+        if (constructingValue()) {
+            tree.expr = translate(tree.expr, false);
+            result = tree;
+        } else {
+            super.visitExec(tree);
+        }
+    }
+
+    @Override
     public void visitIdent(JCIdent ident) {
         if (constructingValue()) {
             Symbol symbol = ident.sym;
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/langtools/tools/javac/valhalla/lworld-values/SideEffectTest.java	Mon Jul 16 20:28:32 2018 +0530
@@ -0,0 +1,66 @@
+/*
+ * Copyright (c) 2018, 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 8207341
+ * @summary Test value constructor code with side effects.
+ * @run main/othervm -XX:+EnableValhalla SideEffectTest
+ */
+
+
+public class SideEffectTest {
+
+	static __ByValue class V {
+
+        static String output = "";
+
+        int x;
+
+		V() {
+            foo(x = 1234);
+		}
+
+		V(int x) {
+            int l = 1234; 
+            foo(l += this.x = x);
+		}
+
+        static void foo(int x) {
+            output += x;
+        }
+	}
+
+	public static void main(String[] args) {
+		V v = new V();
+        if (!v.output.equals("1234"))
+            throw new AssertionError("Broken");
+        if (!v.toString().equals("[value class SideEffectTest$V, 1234]"))
+            throw new AssertionError("Broken");
+		v = new V(8765);
+        if (!v.output.equals("12349999"))
+            throw new AssertionError("Broken");
+        if (!v.toString().equals("[value class SideEffectTest$V, 8765]"))
+            throw new AssertionError("Broken");
+	}
+}
\ No newline at end of file