Merge pull request #4355 from jruby/truffle-fix-optional-assignment-s…

…pecs [Truffle] Fix optional assignment specs
jruby · Dec 2, 2016 · d94afea · d94afea
2 parents 04852a7 + 5dbb32d
commit d94afea
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 24 deletions.
diff --git a/spec/truffle/tags/language/optional_assignments_tags.txt b/spec/truffle/tags/language/optional_assignments_tags.txt
diff --git a/truffle/src/main/java/org/jruby/truffle/language/constants/ReadConstantNode.java b/truffle/src/main/java/org/jruby/truffle/language/constants/ReadConstantNode.java
@@ -10,6 +10,7 @@
 package org.jruby.truffle.language.constants;
 
 import com.oracle.truffle.api.frame.VirtualFrame;
+import com.oracle.truffle.api.nodes.NodeUtil;
 import com.oracle.truffle.api.source.SourceSection;
 import org.jcodings.specific.UTF8Encoding;
 import org.jruby.truffle.Layouts;
@@ -77,4 +78,8 @@ public Object isDefined(VirtualFrame frame) {
         }
     }
 
+    public RubyNode makeWriteNode(RubyNode rhs) {
+        return new WriteConstantNode(name, NodeUtil.cloneNode(moduleNode), rhs);
+    }
+
 }
diff --git a/truffle/src/main/java/org/jruby/truffle/parser/BodyTranslator.java b/truffle/src/main/java/org/jruby/truffle/parser/BodyTranslator.java
@@ -27,7 +27,6 @@
 import org.jruby.truffle.core.IsNilNode;
 import org.jruby.truffle.core.IsRubiniusUndefinedNode;
 import org.jruby.truffle.core.RaiseIfFrozenNode;
-import org.jruby.truffle.core.VMPrimitiveNodesFactory;
 import org.jruby.truffle.core.array.ArrayAppendOneNodeGen;
 import org.jruby.truffle.core.array.ArrayConcatNode;
 import org.jruby.truffle.core.array.ArrayDropTailNode;
@@ -2570,6 +2569,10 @@ public RubyNode visitNthRefNode(NthRefParseNode node) {
 
     @Override
     public RubyNode visitOpAsgnAndNode(OpAsgnAndParseNode node) {
+        return translateOpAsgnAndNode(node, node.getFirstNode().accept(this), node.getSecondNode().accept(this));
+    }
+
+    private RubyNode translateOpAsgnAndNode(ParseNode node, RubyNode lhs, RubyNode rhs) {
         /*
          * This doesn't translate as you might expect!
          *
@@ -2579,10 +2582,7 @@ public RubyNode visitOpAsgnAndNode(OpAsgnAndParseNode node) {
         final RubySourceSection sourceSection = translate(node.getPosition());
         final SourceSection fullSourceSection = sourceSection.toSourceSection(source);
 
-        final ParseNode lhs = node.getFirstNode();
-        final ParseNode rhs = node.getSecondNode();
-
-        final RubyNode andNode = new AndNode(lhs.accept(this), rhs.accept(this));
+        final RubyNode andNode = new AndNode(lhs, rhs);
         andNode.unsafeSetSourceSection(sourceSection);
 
         final RubyNode ret = new DefinedWrapperNode(context, fullSourceSection, context.getCoreStrings().ASSIGNMENT, andNode);
@@ -2591,11 +2591,33 @@ public RubyNode visitOpAsgnAndNode(OpAsgnAndParseNode node) {
 
     @Override
     public RubyNode visitOpAsgnConstDeclNode(OpAsgnConstDeclParseNode node) {
-        // TODO (eregon, 7 Nov. 2016): Is there any semantic difference?
-        if ("&&".equals(node.getOperator())) {
-            return visitOpAsgnAndNode(new OpAsgnAndParseNode(node.getPosition(), node.getFirstNode(), node.getSecondNode()));
-        } else {
-            return visitOpAsgnOrNode(new OpAsgnOrParseNode(node.getPosition(), node.getFirstNode(), node.getSecondNode()));
+        RubyNode lhs = node.getFirstNode().accept(this);
+        RubyNode rhs = node.getSecondNode().accept(this);
+
+        if (!(rhs instanceof WriteConstantNode)) {
+            rhs = ((ReadConstantNode) lhs).makeWriteNode(rhs);
+        }
+
+        switch (node.getOperator()) {
+            case "&&": {
+                return translateOpAsgnAndNode(node, lhs, rhs);
+            }
+
+            case "||": {
+                final RubyNode defined = new DefinedNode(context, translateSourceSection(source, lhs.getRubySourceSection()), lhs);
+                lhs = new AndNode(defined, lhs);
+
+                return translateOpAsgOrNode(node, lhs, rhs);
+            }
+
+            default: {
+                final RubySourceSection sourceSection = translate(node.getPosition());
+                final RubyCallNodeParameters callParameters = new RubyCallNodeParameters(context, sourceSection.toSourceSection(source), lhs, node.getOperator(), null, new RubyNode[] { rhs }, false, true);
+                final RubyNode opNode = context.getCoreMethods().createCallNode(callParameters);
+                final RubyNode ret = ((ReadConstantNode) lhs).makeWriteNode(opNode);
+
+                return addNewlineIfNeeded(node, ret);
+            }
         }
     }
 
@@ -2676,6 +2698,19 @@ public RubyNode visitOpAsgnNode(OpAsgnParseNode node) {
 
     @Override
     public RubyNode visitOpAsgnOrNode(OpAsgnOrParseNode node) {
+        RubyNode lhs = node.getFirstNode().accept(this);
+        RubyNode rhs = node.getSecondNode().accept(this);
+
+        // This is needed for class variables. Constants are handled separately in visitOpAsgnConstDeclNode.
+        if (node.getFirstNode().needsDefinitionCheck() && !(node.getFirstNode() instanceof InstVarParseNode)) {
+            RubyNode defined = new DefinedNode(context, translateSourceSection(source, lhs.getRubySourceSection()), lhs);
+            lhs = new AndNode(defined, lhs);
+        }
+
+        return translateOpAsgOrNode(node, lhs, rhs);
+    }
+
+    private RubyNode translateOpAsgOrNode(ParseNode node, RubyNode lhs, RubyNode rhs) {
         /*
          * This doesn't translate as you might expect!
          *
@@ -2685,16 +2720,6 @@ public RubyNode visitOpAsgnOrNode(OpAsgnOrParseNode node) {
         final RubySourceSection sourceSection = translate(node.getPosition());
         final SourceSection fullSourceSection = sourceSection.toSourceSection(source);
 
-        RubyNode lhs = node.getFirstNode().accept(this);
-        RubyNode rhs = node.getSecondNode().accept(this);
-
-        // I think this is only required for constants - not instance variables
-
-        if (node.getFirstNode().needsDefinitionCheck() && !(node.getFirstNode() instanceof InstVarParseNode)) {
-            RubyNode defined = new DefinedNode(context, translateSourceSection(source, lhs.getRubySourceSection()), lhs);
-            lhs = new AndNode(defined, lhs);
-        }
-
         final RubyNode ret = new DefinedWrapperNode(context, fullSourceSection, context.getCoreStrings().ASSIGNMENT,
                 new OrNode(lhs, rhs));