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

Fixes Fixnum#bitLength when the value is negative. #3831

Merged
merged 1 commit into from
Apr 27, 2016

Conversation

lucasallan
Copy link
Member

Fixes #3829

@kares
Copy link
Member

kares commented Apr 27, 2016

Hey Lucas! seems a bit wrong isn't it ? ... maybe some test-cases would help :

2.3.0 :002 > 112.bit_length
 => 7 
2.3.0 :003 > -112.bit_length
 => 7 

@lucasallan
Copy link
Member Author

@kares

>> 112.bit_length
=> 7
>> -112.bit_length
=> 0

It seems to be working for me - I'll send a PR with tests to rubyspecs.

@kares
Copy link
Member

kares commented Apr 27, 2016

-112 should not have a bit length of 0 ... if it does at your MRI its platform dependent or a bug.
... if you think of it it would be a pretty weird outcome to expect all < 0 to have a bit length of 0

@lucasallan
Copy link
Member Author

lucasallan commented Apr 27, 2016

You're right, it seems pretty weird - I was assuming MRI was right - my bad. Done @kares

@kares kares merged commit 9b63036 into jruby:master Apr 27, 2016
@kares kares added this to the JRuby 9.1.0.0 milestone Apr 27, 2016
@kares
Copy link
Member

kares commented Apr 27, 2016

Great, thanks Lucas.

@jkr2255
Copy link

jkr2255 commented Apr 27, 2016

Thanks for improvement, but for some values still return different value from MRI.

In MRI implementation uses bit inversion, (-(2**x)).bit_lengthbecomes x in MRI (and x + 1 for this implementation)

@lucasallan
Copy link
Member Author

@jkr2255 It will be fixed by #3833

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

Successfully merging this pull request may close these issues.

None yet

4 participants