BinopAssign<&uX> is not implemented for BigUint

[lcnr]

BigUint(and probably also BigInt) implements Add<u8> and Add<&u8>, but only AddAssign<u8>. We should probably either implement AddAssign<&u8> as well or remove Add<&u8> as the current state is somewhat surprising/unintuitive.

The same is true for all BinopAssign traits: AddAssign, MulAssign, DivAssign etc.

[cuviper]

We shouldn't remove anything -- references can be a big saver when clones can be avoided. The assign ops should be implemented for references too.

[lcnr]

I ran the test at my criterion.rs benchmark suite to check the influence of passing integers by reference instead of by value.

While I am currently running the tests with all integers instead of only u128, I still believe that passing by reference is actually not recommendable. There seems to be a non negligible performance hit when passing &u128 instead of u128.

tmp_data.txt

[lcnr]

first bench this, by value then this, by ref

don't have a lot of time right now, but by ref seems to mostly hurt the performance.

tmp_data.txt

[cuviper]

Oh, yeah, I was only thinking of & among BigUint parameters. For primitive integers, it is a bit wasteful, but consistent with the std implementions. I think it's a matter of convenience for when you already have a reference. That case ought to be roughly equal for performance whether it's dereferenced before the operator or within, so it might as well be made easy for ergonomics.

[lcnr]

I didn't know Add<&Self> is implemented for number types. :open_mouth:

So we should probably implemented the following binops for all integers to be similar to std:

• Binop<BigInt> for int
• Binop<&BigInt> for int
• Binop<int> for BigInt
• Binop<&int> for BigInt
• Binop<int> for &BigInt
• Binop<&int> for &BigInt
• BinopAssign<int> for BigInt
• BinopAssign<&int> for BigInt
• BinopAssign<Bigint> for int
• BinopAssign<&Bigint> for int

I might implement this if it is desired.

[cuviper]

* BinopAssign<&int> for BigInt

I think this is the only one missing -- the rest should be covered by macro forwarding.