ruby / spec

The Ruby Spec Suite aka ruby/spec
MIT License
600 stars 389 forks source link

Support to r55024 (Unifying Biignum/Fixnum to Integer) #256

Closed hsbt closed 8 years ago

hsbt commented 8 years ago

Ruby 2.4.0 has been unified bignum/fixnum to Integer at https://github.com/ruby/ruby/commit/f9727c12cc8fbc5f752f5983be1f14bb976e5a13

spec is boken now. We need to solve it. ref https://travis-ci.org/ruby/ruby/builds/130761218

eregon commented 8 years ago

Awesome, although it probably shouldn't segfault, right? :) My understanding is we can keep most of the C-API unchanged and still allow its usage, while at the Ruby level Fixnum = Bignum = Integer.

akr commented 8 years ago

It is because ensure_bignum (optional/capi/bignum_spec.rb) can return fixnum now because Bignum#coerce is moved to Integer#coerce and it returns fixnum as fixnum.

eregon commented 8 years ago

Should rb_big2long and such also handle Fixnums now or not?

eregon commented 8 years ago

We should rewrite some of these specs since they rely on the wrong assumption that Bignum.coerce(Fixnum) gets a Bignum. bignum_value, fixnum_min and fixnum_max should be used instead.

eregon commented 8 years ago

I made an intermediate fix erroring if Bignum#coerce(Fixnum) does not return a Bignum and indeed it stops to segfault.

akr commented 8 years ago

It is possible to modify rb_big2long and other bignum functions to support fixnum too.

But I don't consider it currently because:

eregon commented 8 years ago

I see, indeed this seems a bit specific to ruby/spec for me as well. People are unlikely to call rb_big* on non-Bignums in C code. I'll fix the specs, my first attempt was a miss, there is no way to know whether a value is in Fixnum range anymore now :smiley:

eregon commented 8 years ago

Specs should pass on trunk now.

eregon commented 8 years ago

http://rubyci.org/ is happy so I'll close this. Thanks for raising the issue so I could help to fix this in a timely manner 😃

hsbt commented 8 years ago

👍 Thank you @eregon and @akr