[Truffle] Starting to fix attribute assignment returning the RHS rath…

…er than the result of the method.
jruby · Dec 22, 2014 · 58ca80d · 58ca80d
1 parent f6a371a
commit 58ca80d
Showing 6 changed files with 131 additions and 3 deletions.
diff --git a/core/src/main/java/org/jruby/ast/visitor/AbstractNodeVisitor.java b/core/src/main/java/org/jruby/ast/visitor/AbstractNodeVisitor.java
@@ -583,4 +583,9 @@ public T visitZSuperNode(ZSuperNode node) {
         return defaultVisit(node);
     }
 
+    @Override
+    public T visitOther(Node node) {
+        return defaultVisit(node);
+    }
+
 }
diff --git a/core/src/main/java/org/jruby/ast/visitor/NodeVisitor.java b/core/src/main/java/org/jruby/ast/visitor/NodeVisitor.java
@@ -146,4 +146,5 @@ public interface NodeVisitor<T> {
     public T visitYieldNode(YieldNode iVisited);
     public T visitZArrayNode(ZArrayNode iVisited);
     public T visitZSuperNode(ZSuperNode iVisited);
+    public T visitOther(Node iVisited);
 }
diff --git a/core/src/main/java/org/jruby/truffle/translator/BodyTranslator.java b/core/src/main/java/org/jruby/truffle/translator/BodyTranslator.java
@@ -189,9 +189,65 @@ public RubyNode visitAttrAssignNode(org.jruby.ast.AttrAssignNode node) {
      * See translateDummyAssignment to understand what this is for.
      */
     public RubyNode visitAttrAssignNodeExtraArgument(org.jruby.ast.AttrAssignNode node, RubyNode extraArgument) {
-        final CallNode callNode = new CallNode(node.getPosition(), node.getReceiverNode(), node.getName(), node.getArgsNode(), null);
-        boolean isAccessorOnSelf = (node.getReceiverNode() instanceof org.jruby.ast.SelfNode);
-        return visitCallNodeExtraArgument(callNode, extraArgument, isAccessorOnSelf, false);
+        final SourceSection sourceSection = translate(node.getPosition());
+
+        // The last argument is the value we assign, and we need to return that as the whole result of this node
+
+        if (extraArgument == null) {
+            // Get that last argument out
+            final List<org.jruby.ast.Node> argChildNodes = new ArrayList<>(node.getArgsNode().childNodes());
+            final org.jruby.ast.Node valueNode = argChildNodes.get(argChildNodes.size() - 1);
+            argChildNodes.remove(argChildNodes.size() - 1);
+
+            // Evaluate the value and store it in a local variable
+            final FrameSlot frameSlot = environment.declareVar(environment.allocateLocalTemp("attrasgn"));
+            final WriteLocalVariableNode writeValue = WriteLocalVariableNodeFactory.create(context, sourceSection, frameSlot, valueNode.accept(this));
+
+            // Recreate the arguments array, reading that local instead of including the RHS for the last argument
+            argChildNodes.add(new ReadLocalDummyNode(sourceSection, frameSlot));
+            final org.jruby.ast.ArrayNode newArgsNode = new org.jruby.ast.ArrayNode(node.getPosition(), argChildNodes.get(0));
+            argChildNodes.remove(0);
+            for (org.jruby.ast.Node child : argChildNodes) {
+                newArgsNode.add(child);
+            }
+
+            final CallNode callNode = new CallNode(node.getPosition(), node.getReceiverNode(), node.getName(), newArgsNode, null);
+            boolean isAccessorOnSelf = (node.getReceiverNode() instanceof org.jruby.ast.SelfNode);
+            final RubyNode actualCall = visitCallNodeExtraArgument(callNode, null, isAccessorOnSelf, false);
+
+            return SequenceNode.sequence(context, sourceSection,
+                    writeValue,
+                    actualCall,
+                    ReadLocalVariableNodeFactory.create(context, sourceSection, frameSlot));
+        } else {
+            final RubyNode valueNode = extraArgument;
+
+            // Evaluate the value and store it in a local variable
+            final FrameSlot frameSlot = environment.declareVar(environment.allocateLocalTemp("attrasgn"));
+            final WriteLocalVariableNode writeValue = WriteLocalVariableNodeFactory.create(context, sourceSection, frameSlot, valueNode);
+
+            // Recreate the arguments array, reading that local instead of including the RHS for the last argument
+            final List<org.jruby.ast.Node> argChildNodes = new ArrayList<>();
+            if (node.getArgsNode() != null) {
+                argChildNodes.addAll(node.getArgsNode().childNodes());
+            }
+            argChildNodes.add(new ReadLocalDummyNode(sourceSection, frameSlot));
+            final org.jruby.ast.ArrayNode newArgsNode = new org.jruby.ast.ArrayNode(node.getPosition(), argChildNodes.get(0));
+            argChildNodes.remove(0);
+            for (org.jruby.ast.Node child : argChildNodes) {
+                newArgsNode.add(child);
+            }
+
+            final CallNode callNode = new CallNode(node.getPosition(), node.getReceiverNode(), node.getName(), newArgsNode, null);
+            boolean isAccessorOnSelf = (node.getReceiverNode() instanceof org.jruby.ast.SelfNode);
+            final RubyNode actualCall = visitCallNodeExtraArgument(callNode, null, isAccessorOnSelf, false);
+
+            return SequenceNode.sequence(context, sourceSection,
+                    writeValue,
+                    actualCall,
+                    ReadLocalVariableNodeFactory.create(context, sourceSection, frameSlot));
+        }
+
     }
 
     @Override
@@ -2325,4 +2381,14 @@ protected String getIdentifier() {
         }
     }
 
+    @Override
+    public RubyNode visitOther(Node node) {
+        if (node instanceof ReadLocalDummyNode) {
+            final ReadLocalDummyNode readLocal = (ReadLocalDummyNode) node;
+            return ReadLocalVariableNodeFactory.create(context, readLocal.getSourceSection(), readLocal.getFrameSlot());
+        } else {
+            throw new UnsupportedOperationException();
+        }
+    }
+
 }
diff --git a/core/src/main/java/org/jruby/truffle/translator/ReadLocalDummyNode.java b/core/src/main/java/org/jruby/truffle/translator/ReadLocalDummyNode.java
@@ -0,0 +1,53 @@
+/*
+ * Copyright (c) 2014 Oracle and/or its affiliates. All rights reserved. This
+ * code is released under a tri EPL/GPL/LGPL license. You can use it,
+ * redistribute it and/or modify it under the terms of the:
+ *
+ * Eclipse Public License version 1.0
+ * GNU General Public License version 2
+ * GNU Lesser General Public License version 2.1
+ */
+package org.jruby.truffle.translator;
+
+import com.oracle.truffle.api.frame.FrameSlot;
+import com.oracle.truffle.api.source.SourceSection;
+import org.jruby.ast.Node;
+import org.jruby.ast.NodeType;
+import org.jruby.ast.visitor.NodeVisitor;
+
+import java.util.List;
+
+public class ReadLocalDummyNode extends org.jruby.ast.Node {
+
+    final SourceSection sourceSection;
+    final FrameSlot frameSlot;
+
+    public ReadLocalDummyNode(SourceSection sourceSection, FrameSlot frameSlot) {
+        super(null);
+        this.sourceSection = sourceSection;
+        this.frameSlot = frameSlot;
+    }
+
+    @Override
+    public <T> T accept(NodeVisitor<T> visitor) {
+        return visitor.visitOther(this);
+    }
+
+    @Override
+    public List<Node> childNodes() {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public NodeType getNodeType() {
+        throw new UnsupportedOperationException();
+    }
+
+    public SourceSection getSourceSection() {
+        return sourceSection;
+    }
+
+    public FrameSlot getFrameSlot() {
+        return frameSlot;
+    }
+}
diff --git a/spec/truffle/tags/language/array_tags.txt b/spec/truffle/tags/language/array_tags.txt
@@ -0,0 +1,2 @@
+fails:The unpacking splat operator (*) unpacks arguments as if they were listed statically
+fails:The unpacking splat operator (*) unpacks the start and count arguments in an array slice assignment
diff --git a/spec/truffle/tags/language/send_tags.txt b/spec/truffle/tags/language/send_tags.txt
@@ -4,3 +4,4 @@ fails:Invoking a method allows []= with multiple *args and does not unwrap the l
 fails:Invoking a method allows []= with multiple *args
 fails:Invoking a method allows []= with *args in the [] expanded to individual arguments
 fails:Invoking a method expands an array to arguments grouped in parentheses which in turn takes rest arguments
+fails:allows []= with arguments after splat with *args in the [] and post args
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		fails:The unpacking splat operator (*) unpacks arguments as if they were listed statically
		fails:The unpacking splat operator (*) unpacks the start and count arguments in an array slice assignment