Permalink
Browse files

[IR] Fixed long-standing bug passing splats as call args.

  • Loading branch information...
1 parent e26bd7f commit f15ecdc6ee9ea9f52399da87201121c3c46688aa @subbuss subbuss committed Jan 31, 2012
@@ -669,19 +669,22 @@ protected Variable getValueInTemporaryVariable(IRScope s, Operand val) {
return copyAndReturnValue(s, val);
}
- protected void buildCallArgNode(List<Operand> argsList, Node args, IRScope s) {
+ // Return the last argument in the list -- AttrAssign needs it
+ protected Operand buildCallArgs(List<Operand> argsList, Node args, IRScope s) {
+ // unwrap newline nodes to get their actual type
+ args = skipOverNewlines(s, args);
switch (args.getNodeType()) {
case ARGSCATNODE: {
- ArgsCatNode n = (ArgsCatNode)args;
- argsList.add(new Splat(build(n.getFirstNode(), s)));
- argsList.add(new Splat(build(n.getSecondNode(), s)));
- break;
+ CompoundArray a = (CompoundArray)build(args, s);
+ argsList.add(new Splat(a));
+ return a.getAppendedArg();
}
- case ARGSPUSHNODE: {
- ArgsPushNode n = (ArgsPushNode)args;
- argsList.add(new Splat(build(n.getFirstNode(), s)));
- argsList.add(build(n.getSecondNode(), s));
- break;
+ case ARGSPUSHNODE: {
+ ArgsPushNode ap = (ArgsPushNode)args;
+ Operand v1 = build(ap.getFirstNode(), s);
+ Operand v2 = build(ap.getSecondNode(), s);
+ argsList.add(new Splat(new CompoundArray(v1, v2, true)));
+ return v2;
}
case ARRAYNODE: {
ArrayNode arrayNode = (ArrayNode)args;
@@ -690,33 +693,30 @@ protected void buildCallArgNode(List<Operand> argsList, Node args, IRScope s) {
if (children.size() == 1) {
// skipOverNewlines is required because the parser inserts a NewLineNode in between!
Node child = skipOverNewlines(s, children.get(0));
- Operand childOperand = build(child, s);
if (child instanceof SplatNode) {
// SSS: If the only child is a splat, the splat is supposed to get through
// as an array without being expanded into the call arg list.
//
- // In 1.9 mode, foo=*1 is supposed to store [1], but foo(*1) is supposed to store 1.
- // The AST for the former is: ArrayNode(Splat19Node(..))
- // The AST for the latter is: Splat19Node(..)
- //
- // In 1.8 mode, foo=[*1] is supposed to store [1]
- // The AST for this is: ArrayNode(SplatNode(..))
+ // The AST for the foo([*1]) is: ArrayNode(Splat19Node(..))
+ // The AST for the foo(*1) is: Splat19Node(..)
//
- // Why the special case?
- //
- // Since splats in call args are always expanded, we convert the splat
+ // Since a lone splat in call args is always expanded, we convert the splat
// into a compound array: *n --> args-cat([], *n)
- Splat splat = (Splat)childOperand;
- childOperand = new CompoundArray(new Array(), splat.getArray());
+ SplatNode splat = (SplatNode)child;
+ Variable splatArray = getValueInTemporaryVariable(s, build(splat.getValue(), s));
+ argsList.add(new CompoundArray(new Array(), splatArray));
+ return new Splat(splatArray);
+ } else {
+ Operand childOperand = build(child, s);
+ argsList.add(childOperand);
+ return childOperand;
}
- argsList.add(childOperand);
} else {
// explode array, it's an internal "args" array
for (Node n: children) {
argsList.add(build(n, s));
}
}
- break;
} else {
// use array as-is, it's a literal array
argsList.add(build(arrayNode, s));
@@ -728,16 +728,13 @@ protected void buildCallArgNode(List<Operand> argsList, Node args, IRScope s) {
break;
}
}
+
+ return argsList.isEmpty() ? Nil.NIL : argsList.get(argsList.size() - 1);
}
public List<Operand> setupCallArgs(Node args, IRScope s) {
List<Operand> argsList = new ArrayList<Operand>();
- if (args != null) {
- // unwrap newline nodes to get their actual type
- args = skipOverNewlines(s, args);
- buildCallArgNode(argsList, args, s);
- }
-
+ if (args != null) buildCallArgs(argsList, args, s);
return argsList;
}
@@ -962,10 +959,11 @@ public Operand buildArgsPush(final ArgsPushNode node, IRScope s) {
private Operand buildAttrAssign(final AttrAssignNode attrAssignNode, IRScope s) {
Operand obj = build(attrAssignNode.getReceiverNode(), s);
- List<Operand> args = setupCallArgs(attrAssignNode.getArgsNode(), s);
+ List<Operand> args = new ArrayList<Operand>();
+ Node argsNode = attrAssignNode.getArgsNode();
+ Operand lastArg = (argsNode == null) ? Nil.NIL : buildCallArgs(args, argsNode, s);
s.addInstr(new AttrAssignInstr(obj, new MethAddr(attrAssignNode.getName()), args.toArray(new Operand[args.size()])));
- Operand lastArg = args.get(args.size()-1);
- return (lastArg instanceof Splat) ? ((Splat)lastArg).getArray() : lastArg;
+ return lastArg;
}
public Operand buildAttrAssignAssignment(Node node, IRScope s, Operand value) {
@@ -27,10 +27,11 @@ public ManyArgBlockSplatCallAdapter(CallSite callSite, Operand[] args, Operand c
@Override
protected IRubyObject[] prepareArguments(ThreadContext context, IRubyObject self, Operand[] args, DynamicScope currDynScope, Object[] temp) {
List<IRubyObject> argList = new ArrayList<IRubyObject>();
- for (int i = 0; i < args.length; i++) {
+ int numArgs = args.length;
+ for (int i = 0; i < numArgs; i++) {
IRubyObject rArg = (IRubyObject) args[i].retrieve(context, self, currDynScope, temp);
- if (args[i] instanceof Splat) {
- argList.addAll(Arrays.asList(((RubyArray) rArg).toJavaArray()));
+ if (numArgs == 1 && args[i] instanceof Splat) {
+ argList.addAll(Arrays.asList(((RubyArray)rArg).toJavaArray()));
} else {
argList.add(rArg);
}

0 comments on commit f15ecdc

Please sign in to comment.