ToBigInt and ToBigUint should not return an error

[rust-highfive]

Issue by lifthrasiir
Wednesday Apr 16, 2014 at 10:23 GMT

For earlier discussion, see rust-lang/rust#13555

This issue was labelled with: in the Rust repository

---

It is a side effect of #9250, which added those traits as a special case of the FromPrimitive trait. This is rather strange, since the current ToBigInt implementors do not fail at all ( ToBigUint does fail, but only because they are implemented for signed integers as well).

Given that BigInt and BigUint already implements FromStr (that would correspond to impl<'a> ToBigInt for &'a str and so on), it would be better to make them implemented only for types that would definitely convertible to the big integers without an error. More precisely:

• Make ToBigInt::to_bigint and ToBigUint::to_biguint return BigInt and BigUint respectively.
• Remove implementations of ToBigUint for signed integers and BigInt.
• Add a new into_biguint (since it's more efficient, and does not confuse users of ToBigUint) to BigInt, which returns Option<BigUint>. We may also have into_bigint to BigUint, at expense of symmetry (it becomes asymmetric since it should not return Option<BigInt>).

[gsingh93]

Can a core team member comment on this? It's an easy change to make, and it can have huge benefits. I was thinking of changing all the std::ops traits to take a type that implements ToBigInt, so that you can do useful things like adding regular integer constants to BigInts, which would greatly reduce the verbosity of the current API. However, I can't do this because these conversion methods can fail.

[gsingh93]

@huonw, @alexcrichton, do you have thoughts on this? I'd like to implement it.

[gsingh93]

@hauleth and @cuviper, you are the new maintainers, correct? What do you think about this issue?

[cuviper]

I'd rather use From for infallible conversions (#117) and leave these alone. You already pointed out that ToBigUint could fail on signed values, and even ToBigInt could fail if it were implemented for something like BigRational. I think it's useful to allow this possibility, and we don't need to break the current API when From and Into can do the job you want.

Expanding generic std::ops for From/Into types sounds fine, but do note the issues that came up in #125 doing something like this for Complex. You can be generic on the RHS, but the compiler won't let you implement for T on the LHS.

[cuviper]

FYI, num-bigint has its own repo now: https://github.com/rust-num/num-bigint

But I think my prior comment still stands, and we do have From primitive integers now.