Skip to content
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

Jump to bottom

Add local var methods to Binding #3372

Closed
wants to merge 2 commits into from
Closed

Add local var methods to Binding #3372

wants to merge 2 commits into from

Conversation

mlarraz
Copy link

@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.

@mlarraz
Add local var methods to Binding
b9de770 
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
Copy link
Contributor

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

rubinius/kernel/common/eval.rb

Lines 109 to 112 in 7362475

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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Member

brixen commented May 21, 2015

Are there specs for this?

@yorickpeterse
Copy link
Contributor

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

@yorickpeterse yorickpeterse self-assigned this Feb 5, 2016
@yorickpeterse
Copy link
Contributor

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

@mlarraz mlarraz deleted the binding_locals branch February 5, 2016 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@yorickpeterse yorickpeterse

Labels
Needs Feedback
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@mlarraz @yorickpeterse @brixen