Use a defensive temp var when pulling possibly-undefined opt args.

Fixes #3094.
jruby · Mar 4, 2016 · c9e46b7 · subbuss · Mar 4, 2016 · headius
1 parent c6e43c0
commit c9e46b7
Showing 2 changed files with 32 additions and 3 deletions.
diff --git a/core/src/main/java/org/jruby/ir/IRBuilder.java b/core/src/main/java/org/jruby/ir/IRBuilder.java
@@ -1733,7 +1733,12 @@ public Operand buildDAsgn(final DAsgnNode dasgnNode) {
         int depth = dasgnNode.getDepth();
         Variable arg = getLocalVariable(dasgnNode.getName(), depth);
         Operand  value = build(dasgnNode.getValueNode());
+
+        // no use copying a variable to itself
+        if (arg == value) return value;
+
         addInstr(new CopyInstr(arg, value));
+
         return value;
 
         // IMPORTANT: The return value of this method is value, not arg!
@@ -1909,6 +1914,7 @@ protected void receiveNonBlockArgs(final ArgsNode argsNode) {
         // Now for opt args
         if (opt > 0) {
             int optIndex = argsNode.getOptArgIndex();
+            Variable temp = createTemporaryVariable();
             for (int j = 0; j < opt; j++, argIndex++) {
                 // Jump to 'l' if this arg is not null.  If null, fall through and build the default value!
                 Label l = getNewLabel();
@@ -1917,10 +1923,12 @@ protected void receiveNonBlockArgs(final ArgsNode argsNode) {
                 Variable av = getNewLocalVariable(argName, 0);
                 if (scope instanceof IRMethod) addArgumentDescription(ArgumentType.opt, argName);
                 // You need at least required+j+1 incoming args for this opt arg to get an arg at all
-                addInstr(new ReceiveOptArgInstr(av, signature.required(), signature.pre(), j));
-                addInstr(BNEInstr.create(l, av, UndefinedValue.UNDEFINED)); // if 'av' is not undefined, go to default
-                build(n.getValue());
+                addInstr(new ReceiveOptArgInstr(temp, signature.required(), signature.pre(), j));
+                addInstr(BNEInstr.create(l, temp, UndefinedValue.UNDEFINED)); // if 'av' is not undefined, go to default
+                Operand defaultResult = build(n.getValue());
+                addInstr(new CopyInstr(temp, defaultResult));
                 addInstr(new LabelInstr(l));
+                addInstr(new CopyInstr(av, temp));
             }
         }
 
@@ -2752,7 +2760,12 @@ public Operand buildLiteral(LiteralNode literalNode) {
     public Operand buildLocalAsgn(LocalAsgnNode localAsgnNode) {
         Variable var  = getLocalVariable(localAsgnNode.getName(), localAsgnNode.getDepth());
         Operand value = build(localAsgnNode.getValueNode());
+
+        // no use copying a variable to itself
+        if (var == value) return value;
+
         addInstr(new CopyInstr(var, value));
+
         return value;
 
         // IMPORTANT: The return value of this method is value, not var!

diff --git a/spec/compiler/general_spec.rb b/spec/compiler/general_spec.rb
@@ -1068,5 +1068,21 @@ def a; __callee__; end
         expect(y).to eq(1)
       end
     end
+
+    it "handles circular optional argument assignment" do
+      begin
+        verbose = $VERBOSE
+        $VERBOSE = nil
+
+        run('def foo(a = a, b = b, c = c); [a.inspect, b.inspect, c.inspect]; end; foo') do |x|
+          expect(x).to eq(["nil", "nil", "nil"])
+        end
+        run('def foo; yield; end; foo {|a = a, b = b, c = c|; [a.inspect, b.inspect, c.inspect]}') do |x|
+          expect(x).to eq(["nil", "nil", "nil"])
+        end
+      ensure
+        $VERBOSE = verbose
+      end
+    end
   end
 end