The binding behaviour with respect to local variables is inconsistent with MRI #2940

Open
kyrylo opened this Issue Feb 12, 2014 · 6 comments

Projects

None yet

2 participants

@kyrylo

The code.

bindings = [binding, binding, binding, binding]
bindings.each_with_index { |bind, idx| bind.eval("x = #{ idx }") }
puts 'Output:'
p bindings.map { |bind| bind.eval('x') }

Let's run that code.

Rubinius 2.2.5

rubinius 2.2.5 (2.1.0 e543ba32 2014-02-08 JI) [x86_64-linux-gnu]
Output:
[3, 3, 3, 3]

MRI 1.8.7

ruby 1.8.7 (2013-06-27 patchlevel 374) [x86_64-linux]
Output:
[3, 3, 3, 3]

MRI 1.9.3 and greater

Output:
[0, 1, 2, 3]

Personally, I find the current Rubinius' behaviour understandable. Each different binding object references to the same scope it was created in. However, I don't have much knowledge or MRI's and Rubinius' internals, so I have no idea what led them to do this. I consider this a bug, though.

@kyrylo kyrylo referenced this issue in pry/pry-stack_explorer Feb 12, 2014
Open

Fix the test that fails on Rubinius #19

@YorickPeterse
Rubinius member

This is probably caused by Rbx creating new Binding instances for every call to Kernel.binding: https://github.com/rubinius/rubinius/blob/master/kernel/common/eval.rb#L137-L145

@kyrylo

I don't think the problem is creation of new objects. MRI also creates a new Binding object each time.

@ghost

this happens because in your example each binding references the same instance of Rubinius::VariableScope and in turn references the same lookup table for eval locals, so every assignment in the iteration overwrites the last.

the easy solution was to duplicate binding.variables in Kernel#eval.and it seems that is already done for the constant scope. it worked and passes the test suite but a tagged failure on rubinius has now become a segfault, so i don't think its worth it.

the tagged failure that is now a segfault is:

 outer_binding = binding
 eval("if false; a = 1; end", outer_binding)
eval("a", outer_binding).should be_nil
@ghost

the segfault doesn't happen in a standalone script but when you run bin/mspec:

bin/mspec spec/ruby/core/kernel/eval_spec.rb

my diff is:

diff --git a/kernel/common/eval.rb b/kernel/common/eval.rb
index 1ef4e7d..d1648c6 100644
--- a/kernel/common/eval.rb
+++ b/kernel/common/eval.rb
@@ -169,6 +169,8 @@ module Kernel

     existing_scope = binding.constant_scope
     binding.constant_scope = existing_scope.dup
+    new_variables = binding.variables.dup
+    binding.variables = new_variables

some output:
cat b.rb

bindings = [binding, binding, binding, binding]
bindings.each_with_index { |b, index| b.eval("x = #{index}") }
p bindings.map { |b| b.eval("x") }

outer_binding = binding
eval("if false; a = 1; end", outer_binding)
eval("a", outer_binding).should be_nil

result

bin/rbx b.rb
[0, 1, 2, 3]
An exception occurred running b.rb:

    undefined method `a' on an instance of Object. (NoMethodError)
@ghost

i also found this interesting, inside an Array.new(…) {} block 'VariableScope' is a new object on each iteration:

Array.new(4) { p Rubinius::VariableScope.current.object_id }

so, on rubinius this works fine:

bindings = Array.new(4) { binding }
bindings.each_with_index { |b,index| b.eval("x = #{index}") }
p bindings.map { |b| b.eval("x") }
@ghost

i'm not working on this anymore, or even trying to. please feel free to pick it up if you're interested :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment