Merge pull request #7688 from enebo/weird_binding

Correct Binding#dup to properly propagate values it captures

core/src/main/java/org/jruby/RubyBinding.java

@@ -105,7 +105,12 @@ public IRubyObject initialize(ThreadContext context) {
 
         return this;
     }
-
+
+    @JRubyMethod
+    public IRubyObject dup(ThreadContext context) {
+        return newBinding(context.runtime, binding.dup(context));
+    }
+
     @JRubyMethod(name = "initialize_copy", visibility = Visibility.PRIVATE)
     @Override
     public IRubyObject initialize_copy(IRubyObject other) {

core/src/main/java/org/jruby/parser/StaticScope.java

@@ -76,7 +76,7 @@
  * will point to the previous scope of the enclosing module/class (cref).
  * 
  */
-public class StaticScope implements Serializable {
+public class StaticScope implements Serializable, Cloneable {
     public static final int MAX_SPECIALIZED_SIZE = 50;
     private static final long serialVersionUID = 3423852552352498148L;
 

core/src/main/java/org/jruby/runtime/Binding.java

@@ -316,4 +316,16 @@ public BacktraceElement getBacktrace() {
                     // Can't use Frame.DUMMY because of circular static init seeing it before it's assigned
                     new Frame(),
                     Visibility.PUBLIC);
+
+    /**
+     * Duplicate this binding and setup the proper cloned instance of the eval scope so that any previously
+     * captured variables still exist but are not shared with the original binding.
+     * @param context the current thread context
+     * @return the duplicated binding
+     */
+    public Binding dup(ThreadContext context) {
+        Binding newBinding = new Binding(this);
+        newBinding.evalScope = getEvalScope(context.runtime).dupEvalScope();
+        return newBinding;
+    }
 }

core/src/main/java/org/jruby/runtime/DynamicScope.java

@@ -32,6 +32,7 @@
 import org.jruby.ir.JIT;
 import org.jruby.parser.StaticScope;
 import org.jruby.runtime.builtin.IRubyObject;
+import org.jruby.runtime.scope.ManyVarsDynamicScope;
 
 public abstract class DynamicScope implements Cloneable {
     // Static scoping information for this scope
@@ -598,4 +599,14 @@ public DynamicScope cloneScope() {
             throw new RuntimeException("BUG: failed to clone scope type " + getClass().getName());
         }
     }
+
+    /**
+     * Binding needs to clone its scope with all the current values.
+     * @return a duplicate of this scope with all the current values
+     */
+    public DynamicScope dupEvalScope() {
+        ManyVarsDynamicScope newScope = new ManyVarsDynamicScope(staticScope.duplicate(), parent);
+        newScope.setVariableValues(getValues());
+        return newScope;
+    }
 }

core/src/main/java/org/jruby/runtime/scope/ManyVarsDynamicScope.java

@@ -50,6 +50,13 @@ private void allocate() {
         if(variableValues == null) variableValues = IRubyObject.array(staticScope.getNumberOfVariables());
     }
 
+    // WARNING:  This is used when dup'ing an eval scope.  We know the size and that it will 100% always be
+    // a ManyVarsDynamicScope.  This should not be used by anything else.  If there ever ends up being another
+    // use then it should be documented in this warning.
+    public void setVariableValues(IRubyObject[] variableValues) {
+        this.variableValues = variableValues;
+    }
+
     public IRubyObject[] getValues() {
         return variableValues;
     }

spec/ruby/core/binding/dup_spec.rb

@@ -10,4 +10,21 @@
     bind.frozen?.should == true
     bind.dup.frozen?.should == false
   end
+
+  it "retains original binding variables but the list is distinct" do
+    bind1 = binding
+    eval "a = 1", bind1
+
+    bind2 = bind1.dup
+    eval("a = 2", bind2)
+    eval("a", bind1).should == 2
+    eval("a", bind2).should == 2
+
+    eval("b = 2", bind2)
+    -> { eval("b", bind1) }.should raise_error(NameError)
+    eval("b", bind2).should == 2
+
+    bind1.local_variables.sort.should == [:a, :bind1, :bind2]
+    bind2.local_variables.sort.should == [:a, :b, :bind1, :bind2]
+  end
 end

spec/tags/ruby/core/kernel/eval_tags.txt

@@ -1,3 +1,3 @@
 fails:Kernel#eval raises a LocalJumpError if there is no lambda-style closure in the chain
 fails:Kernel#eval with a magic encoding comment ignores the magic encoding comment if it is after a frozen_string_literal magic comment
-fails:Kernel#eval makes flip-flop operator work correctly
+