Add support for Add, Sub, Mul & Div ops for Integer

[Maaarcocr]

why?

Currently dealing with Integers in magnus is fairly involved and possibly incorrect in most cases, as you really can't do any integer operation on them in a complete and efficient way.

The 2 options are:

• cast the Integer to i64 and be efficient but not complete, as Integers can represent any arbitrary integer, this would result in major difficulties when porting ruby code to rust, as we do not really have a way to deal with the Bignum case here.
• If you want to be correct, you could do i.coerce_bin("+", other) which would be okay, just possibly a bit slow, as we are calling back into the vm

could we do better?

I think we could do better. We can make using Integers a bit more rust-ic and implement some of the standard ops like Add, Sub, etc. in a way that takes into account both Fixnum and Bignum.

how?

This is a first stab at doing what I just proposed. I implement 4 traits:

• Add: when the 2 numbers are both Fixnum we can add them together as they are and remove 1. If any of this overflows, convert the first to Bignum and use the rb_big_add to calculate the result. Whenever any of the operators is a Bignum convert accordingly and use rb_big_add.
• Sub: when the 2 numbers are both Fixnum we can sub them together as they are and add 1. If any of this overflows, convert the first to Bignum and use the rb_big_sub to calculate the result. Whenever any of the operators is a Bignum convert accordingly and use rb_big_add.
• Mul: when the 2 numbers are both Fixnum convert them to i64, multiply them together and convert the result back to Integer. If the multiplication overflows, fallback to Bignum. Whenever any of the operators is a Bignum convert accordingly and use rb_big_mul.
• Div: when the 2 numbers are both Fixnum convert them to i64, divide them together and convert the result back to Integer. This cannot overflow as explained in the comment. Whenever any of the operators is a Bignum convert accordingly and use rb_big_mul.

In a similar fashion I've implemented PartialOrd and PartialEq

I do plan on possibly implement more traits (like Rem, Shr, Shl etc.) once this is merged and we think that this is a good idea.

I've also added a bunch of tests (which I had to run from just 1 test functions because if not I would get some errors about ruby already existing) to test all the edge cases I could think of.

[matsadler]

There's a repeated pattern here of

match (self.integer_type(), other.integer_type()) {
    (IntegerType::Fixnum(a), IntegerType::Fixnum(b)) => {
        do_in_rust(a, b)
    }
    (IntegerType::Fixnum(a), IntegerType::Bignum(b)) => {
        let a = unsafe { rb_sys::rb_int2big(a.to_isize()) };
        call_ruby(a, b)
    }
    (IntegerType::Bignum(a), _) => {
        call_ruby(a, other)
    }
}

I wonder if that can be extracted out to a helper? It'd be nice if the call_ruby didn't need to be repeated and the rb_int2big call wasn't duplicated across all the trait implementations.

---

I've run into the same problem the tests are encountering before. I'm not sure why those constants aren't available on all platforms. I just defined them myself in value so you should be able to use those.

[matsadler]

@Maaarcocr I know getting stuff working on Windows is the exact opposite of fun, so if you're not feeling it, let me know and I can try and get it working. Of if you're just busy with other stuff, that's cool, no rush.

[Maaarcocr]

@Maaarcocr I know getting stuff working on Windows is the exact opposite of fun, so if you're not feeling it, let me know and I can try and get it working. Of if you're just busy with other stuff, that's cool, no rush.

I have a window machine which should still be usable on which I can try to fix this. Right now work is quite busy and I do not have much time to spend on this. Hopefully after next week I should be able to do so!

[matsadler]

I've rebased this on main and fixed the Windows tests to get this over the line.

Thanks for this, it's great work and I'm excited for this feature.

[Maaarcocr]

I've rebased this on main and fixed the Windows tests to get this over the line.

Thanks for this, it's great work and I'm excited for this feature.

thank you ❤️