New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add local var methods to Binding #3372

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@mlarraz

mlarraz commented Apr 7, 2015

These are defined in MRI as of 2.1.

I basically just copied these from the RubyDocs.

My implementation of Binding#local_variable_set assumes obj responds to #to_s, which may cause issues.

Add local var methods to Binding
These are defined in MRI as of 2.1.

I basically just copied these from the [RubyDocs](http://ruby-doc.org/core-2.1.0/Binding.html#method-i-local_variable_defined-3F).

My implementation of Binding#local_variable_set assumes obj responds to #to_s, which may cause issues
@YorickPeterse

This comment has been minimized.

Member

YorickPeterse commented Apr 7, 2015

No, we're not going to rely on eval for this. Instead we should use Rubinius::VariableScope to get/set any local variables in a scope. See

def local_variables
scope = Rubinius::VariableScope.of_sender
scope.local_variables
end
as an example.

Also, if we have any tagged specs for this they'll need to be untagged, otherwise new specs should be added.

@mlarraz

This comment has been minimized.

mlarraz commented Apr 7, 2015

So my only question about using Rubinius::VariableScope is that the getter/setter methods only seem to consider dynamic locals. Is there an API to get/set locals from Rubinius::CompiledCode?

@YorickPeterse

This comment has been minimized.

Member

YorickPeterse commented Apr 7, 2015

VariableScope puts eval'd variables under VariableScope#dynamic_locals. Looking at things we probably need to expose some extra bits to this class. There's VariableScope#set_local but this requires a slot number (= basically the position in the local variables list) instead of just a variable name.

@brixen

This comment has been minimized.

Member

brixen commented May 21, 2015

Are there specs for this?

@YorickPeterse

This comment has been minimized.

Member

YorickPeterse commented May 21, 2015

Don't think so, at least I can't find any in our repo.

@YorickPeterse YorickPeterse self-assigned this Feb 5, 2016

@YorickPeterse

This comment has been minimized.

Member

YorickPeterse commented Feb 5, 2016

Fixed in 40f04c1, thanks for initially looking into this.

@mlarraz mlarraz deleted the mlarraz:binding_locals branch Feb 5, 2016

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