Navigation Menu

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

Move methods from BasicObject to Object #2520

Merged
merged 3 commits into from Jan 28, 2015

Conversation

sferik
Copy link
Contributor

@sferik sferik commented Jan 26, 2015

In the process of debugging an unrelated issue, I came across an inconsistency between JRuby and CRuby: the #itself and #object_id methods are defined on BasicObject instead of Object.

> ruby -e "puts RUBY_DESCRIPTION; p BasicObject.instance_methods.sort"
ruby 2.2.0p0 (2014-12-25 revision 49005) [x86_64-darwin14]
[:!, :!=, :==, :__id__, :__send__, :equal?, :instance_eval, :instance_exec]
> jruby -e "puts RUBY_DESCRIPTION; p BasicObject.instance_methods.sort"
jruby 9.0.0.0.pre1 (2.2.0p0) 2015-01-20 d537cab Java HotSpot(TM) 64-Bit Server VM 24.51-b03 on 1.7.0_51-b13 +jit [darwin-x86_64]
[:!, :!=, :==, :__id__, :__send__, :equal?, :instance_eval, :instance_exec, :itself, :object_id]

WARNING

i-have-no-idea-what-im-doing-5

I am not an expert ☕ programmer nor am I particularly familiar with the JRuby codebase, however I’ve made an attempt to fix this issue myself, as it appears to be mostly a ✂️ and 🍝 job.

I’ve also attempted to add a test to prevent this behavior from regressing, however I may have put it in the wrong place, as JRuby appears to have multiple test suites.

I’ve also taken the liberty to remove the Object#id method, which was deprecated when Ruby 1.9 was released and removed from CRuby before version 1.9 was released (search for rb_obj_id_obsolete).

@sferik sferik force-pushed the move_methods_to_object branch 18 times, most recently from 8c8bc6b to 66fe1e8 Compare January 28, 2015 16:34
chrisseaton added a commit that referenced this pull request Jan 28, 2015
Move methods from BasicObject to Object
@chrisseaton chrisseaton merged commit a003621 into jruby:master Jan 28, 2015
@chrisseaton
Copy link
Contributor

@enebo should this be cherry-picked onto 17?

@sferik
Copy link
Contributor Author

sferik commented Jan 28, 2015

@chrisseaton @enebo IMHO, the only commit to consider cherry-picking into the 1.7 branch would be 519fd75, as 0ce0215 is not backwards-compatible and c2184b5 Object#itself was only added in Ruby 2.2.0.

@sferik sferik deleted the move_methods_to_object branch January 28, 2015 20:26
@sferik sferik restored the move_methods_to_object branch January 28, 2015 20:26
@sferik sferik deleted the move_methods_to_object branch January 28, 2015 20:26
@enebo
Copy link
Member

enebo commented Jan 28, 2015

So yeah I think 519fd75 should be and I even think 0ce0215 should for --1.9 if it really was removed for 1.9.3 since that is what version -1.9 supports. @sferik 'id' was removed during 1.9.3 or after?

@sferik
Copy link
Contributor Author

sferik commented Jan 28, 2015

@enebo Object#id was removed before 1.9.3.

@headius headius added this to the JRuby 9.0.0.0.pre2 milestone May 4, 2015
chrisroberts added a commit to spox/zoidberg that referenced this pull request Jun 24, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants