Fix --coverage together with multiple assignment using []=

* Fixes #2367
* RubyCallNode#assign should just replace the last value, not add an argument.
* Correctly set the number of arguments for AttrAssignParseNode before translating it.
  Previously, it could miss one argument and then CoreMethodAssumptions would not see the right arity.

Author: eregon

src/main/java/org/truffleruby/core/inlined/InlinedIndexSetNode.java

@@ -12,23 +12,27 @@
 import com.oracle.truffle.api.dsl.Bind;
 import org.truffleruby.RubyLanguage;
 import org.truffleruby.core.array.ArrayWriteNormalizedNode;
+import org.truffleruby.core.array.AssignableNode;
 import org.truffleruby.core.array.RubyArray;
 import org.truffleruby.language.dispatch.RubyCallNodeParameters;
+import org.truffleruby.language.literal.NilLiteralNode;
 import org.truffleruby.language.methods.LookupMethodOnSelfNode;
 
 import com.oracle.truffle.api.dsl.Cached;
 import com.oracle.truffle.api.dsl.Specialization;
 import com.oracle.truffle.api.frame.VirtualFrame;
 import com.oracle.truffle.api.profiles.ConditionProfile;
 
-public abstract class InlinedIndexSetNode extends TernaryInlinedOperationNode {
+public abstract class InlinedIndexSetNode extends TernaryInlinedOperationNode implements AssignableNode {
 
     protected static final String METHOD = "[]=";
 
     public InlinedIndexSetNode(RubyLanguage language, RubyCallNodeParameters callNodeParameters) {
         super(language, callNodeParameters);
     }
 
+    protected abstract Object execute(VirtualFrame frame, Object receiver, Object index, Object value);
+
     @Specialization(
             guards = {
                     "lookupNode.lookupProtected(frame, array, METHOD) == coreMethods().ARRAY_INDEX_SET",
@@ -55,4 +59,16 @@ protected int normalize(RubyArray array, int index, ConditionProfile denormalize
         return index;
     }
 
+    @Override
+    public void assign(VirtualFrame frame, Object value) {
+        final Object receiver = getReceiver().execute(frame);
+        final Object index = getOperand1().execute(frame);
+        execute(frame, receiver, index, value);
+    }
+
+    @Override
+    public AssignableNode toAssignableNode() {
+        assert getOperand2() instanceof NilLiteralNode && ((NilLiteralNode) getOperand2()).isImplicit() : getOperand2();
+        return this;
+    }
 }

src/main/java/org/truffleruby/language/dispatch/RubyCallNode.java

@@ -13,7 +13,6 @@
 import org.truffleruby.RubyLanguage;
 import org.truffleruby.core.array.ArrayToObjectArrayNode;
 import org.truffleruby.core.array.ArrayToObjectArrayNodeGen;
-import org.truffleruby.core.array.ArrayUtils;
 import org.truffleruby.core.array.AssignableNode;
 import org.truffleruby.core.cast.BooleanCastNode;
 import org.truffleruby.core.cast.BooleanCastNodeGen;
@@ -22,6 +21,7 @@
 import org.truffleruby.language.RubyBaseNode;
 import org.truffleruby.language.RubyContextSourceNode;
 import org.truffleruby.language.RubyNode;
+import org.truffleruby.language.literal.NilLiteralNode;
 import org.truffleruby.language.methods.BlockDefinitionNode;
 import org.truffleruby.language.methods.InternalMethod;
 
@@ -88,7 +88,7 @@ public Object execute(VirtualFrame frame) {
             return nil;
         }
 
-        final Object[] executedArguments = executeArguments(frame, arguments.length);
+        final Object[] executedArguments = executeArguments(frame);
 
         final Object blockObject = executeBlock(frame);
 
@@ -99,27 +99,28 @@ public Object execute(VirtualFrame frame) {
     }
 
     @Override
-    public void assign(VirtualFrame frame, Object extraArgument) {
+    public void assign(VirtualFrame frame, Object value) {
+        assert getLastArgumentNode() instanceof NilLiteralNode &&
+                ((NilLiteralNode) getLastArgumentNode()).isImplicit() : getLastArgumentNode();
+
         final Object receiverObject = receiver.execute(frame);
         if (isSafeNavigation && nilProfile.profile(receiverObject == nil)) {
             return;
         }
 
-        final Object[] argumentsObjects;
-        final Object blockObject;
-        if (isSplatted) {
-            final Object[] executedArguments = executeArguments(frame, arguments.length);
+        final Object[] executedArguments = executeArguments(frame);
 
-            blockObject = executeBlock(frame);
+        final Object blockObject = executeBlock(frame);
 
+        final Object[] argumentsObjects;
+        if (isSplatted) {
             // The expansion of the splat is done after executing the block, for m(*args, &args.pop)
-            argumentsObjects = ArrayUtils.append(splat(executedArguments), extraArgument);
+            argumentsObjects = splat(executedArguments);
+            assert argumentsObjects[argumentsObjects.length - 1] == nil;
+            argumentsObjects[argumentsObjects.length - 1] = value;
         } else {
-            final Object[] executedArguments = executeArguments(frame, arguments.length + 1);
-            executedArguments[arguments.length] = extraArgument;
-
-            blockObject = executeBlock(frame);
-
+            assert executedArguments[arguments.length - 1] == nil;
+            executedArguments[arguments.length - 1] = value;
             argumentsObjects = executedArguments;
         }
 
@@ -152,8 +153,8 @@ private Object executeBlock(VirtualFrame frame) {
     }
 
     @ExplodeLoop
-    private Object[] executeArguments(VirtualFrame frame, int size) {
-        final Object[] argumentsObjects = new Object[size];
+    private Object[] executeArguments(VirtualFrame frame) {
+        final Object[] argumentsObjects = new Object[arguments.length];
 
         for (int i = 0; i < arguments.length; i++) {
             argumentsObjects[i] = arguments[i].execute(frame);
@@ -193,6 +194,10 @@ public boolean hasLiteralBlock() {
         return hasLiteralBlock;
     }
 
+    private RubyNode getLastArgumentNode() {
+        return arguments[arguments.length - 1];
+    }
+
     @Override
     public AssignableNode toAssignableNode() {
         return this;

src/main/java/org/truffleruby/language/globals/WriteGlobalVariableNode.java

@@ -87,6 +87,8 @@ public Object isDefined(VirtualFrame frame, RubyLanguage language, RubyContext c
     @Override
     public AssignableNode toAssignableNode() {
         // Cannot reassign a @NodeChild
-        return WriteGlobalVariableNodeGen.create(name, null);
+        final WriteGlobalVariableNode node = WriteGlobalVariableNodeGen.create(name, null);
+        node.copySourceSection(this);
+        return node;
     }
 }

src/main/java/org/truffleruby/parser/BodyTranslator.java

@@ -1767,6 +1767,9 @@ protected AssignableNode toAssignableNode(ParseNode node) {
             return ((AssignableNode) translated).toAssignableNode();
         } else if (node instanceof AttrAssignParseNode) {
             final AttrAssignParseNode attrAssignParseNode = (AttrAssignParseNode) node;
+            // The AttrAssignParseNode does not have a value yet, add a sentinel nil,
+            // so the RubyCallNode is aware of the actual number of runtime arguments.
+            setRHS(attrAssignParseNode, NilImplicitParseNode.NIL);
 
             final RubyNode translated = attrAssignParseNode.accept(this);
             return ((AssignableNode) translated).toAssignableNode();