[PATCH] 8147527: Non-optimal code generated for postfix unary operators

[bsrbnd]

Hi,

2016-11-07 22:48 GMT+01:00 bsrbnd <bsrbnd at gmail.com>:
> Hi,
>
> Another solution would be to clone the nodes as the unary boxed
> operation is expanded.
> This way, there is no reused tree at all and Lower.visitSelect() has
> no need to be modified.
> It's probably the easiest solution which is also the one that makes
> most sense (I think).
> Below is a fix including the optimization.
>
Of course, a deep copy of the tree would be a conceptually better
solution but more dangerous for a simple fix.
Unfortunately, "TreeCopier" loses some information like symbols, etc...
For this reason, I wrote a new class "TreeCloner" extending
"TreeCopier" which performs a full cloning of the nodes while copying
the tree.
I've only implemented the necessary methods required by this issue, as
shown in the following patch (all javac tests have been run).
What do you think of all these solutions?

Thanks,
Bernard

diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java
b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java
@@ -2218,7 +2218,7 @@
                 return builder.build(rval);
         }
         Name name = TreeInfo.name(rval);
-        if (name == names._super)
+        if (name == names._super || name == names._this)
             return builder.build(rval);
         VarSymbol var =
             new VarSymbol(FINAL|SYNTHETIC,
@@ -3205,7 +3205,7 @@
                                                                       newTag,

tree.type,

tree.rhs.type);
-                        JCExpression expr = lhs;
+                        JCExpression expr = new TreeCloner(make).copy(lhs);
                         if (expr.type != tree.type)
                             expr = make.TypeCast(tree.type, expr);
                         JCBinary opResult = make.Binary(newTag, expr,
tree.rhs);
@@ -3292,7 +3292,7 @@
                                     ? make.TypeCast(tree.arg.type, tmp1)
                                     : tmp1;
                                 JCExpression update = makeAssignop(opcode,
-                                                             lhs,
+                                                             new
TreeCloner(make).copy(lhs),
                                                              make.Literal(1));
                                 return makeComma(update, tmp2);
                             }
diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeCloner.java
b/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeCloner.java
new file mode 100644
--- /dev/null
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeCloner.java
@@ -0,0 +1,45 @@
+package com.sun.tools.javac.tree;
+
+import com.sun.source.tree.*;
+import com.sun.tools.javac.tree.JCTree.*;
+import com.sun.tools.javac.util.DefinedBy;
+import com.sun.tools.javac.util.DefinedBy.Api;
+
+public class TreeCloner extends TreeCopier<Void> {
+    public TreeCloner(TreeMaker M) {
+        super(M);
+    }
+
+    @DefinedBy(Api.COMPILER_TREE)
+    public JCTree visitIdentifier(IdentifierTree node, Void p) {
+        return (JCIdent)((JCIdent)node).clone();
+    }
+
+    @DefinedBy(Api.COMPILER_TREE)
+    public JCTree visitMemberSelect(MemberSelectTree node, Void p) {
+        JCFieldAccess t = (JCFieldAccess)((JCFieldAccess)node).clone();
+        t.selected = copy(t.selected, p);
+        return t;
+    }
+
+    @DefinedBy(Api.COMPILER_TREE)
+    public JCTree visitTypeCast(TypeCastTree node, Void p) {
+        JCTypeCast t = (JCTypeCast)((JCTypeCast)node).clone();
+        t.clazz = copy(t.clazz, p);
+        t.expr = copy(t.expr, p);
+        return t;
+    }
+
+    @DefinedBy(Api.COMPILER_TREE)
+    public JCTree visitArrayAccess(ArrayAccessTree node, Void p) {
+        JCArrayAccess t = (JCArrayAccess)((JCArrayAccess)node).clone();
+        t.indexed = copy(t.indexed, p);
+        t.index = copy(t.index, p);
+        return t;
+    }
+
+    @DefinedBy(Api.COMPILER_TREE)
+    public JCTree visitLiteral(LiteralTree node, Void p) {
+        return (JCLiteral)((JCLiteral)node).clone();
+    }
+}