Open Seltyk opened 12 months ago
We had this in development with rust-num/num-bigint#136, but I ran into type inferences in issue rust-num/num-bigint#150, and reverted in rust-num/num-bigint#151.
The inference issue is pretty nasty, because it even breaks code that would otherwise have nothing to do with bigints, just by the compiler seeing the additional trait implementations present at all. And I fully expect it would have the same result if we extended others like Ratio
too.
If we cannot have these as PartialOrd
, can we maybe get regular member functions like cmp_i64
[...] (or a generalized cmp_foreign
) or something? Implementing these by hand is pretty tedious and error prone,
and eating a memory allocation (and deallocation) every time you compare against a bog standard i64 (mybigint.cmp(&BigInt::from(myi64))
) is pretty bad.
can we maybe get regular member functions like
cmp_i64
[...] (or a generalizedcmp_foreign
) or something?
Yes, I think we could reasonably have something comparing a T: PrimInt
, using its NumCast
or ToPrimitive
supertraits in the implementation. Those are fallible though, so it still might not handle a case like an external u256
with a value that can't fit in an actual primitive integer.
Those are fallible though
That doesn't sound right to me. A comparison between two integers should never fail, such an API seems really frustrating for users. What are you supposed to do if it fails?
Why not e.g. have a trait like PrimIntCmp<T: PrimInt>
that essentially uses the existing implementation to provide a prim_int_cmp
method?
If we wanted we could even have a FloatCmp<T: Float>
, although that would would actually have to return an Option
because of NaN
s.
We could make a new trait that's more capable, but I was thinking about what is possible with existing traits. So with T: PrimInt
(and PrimInt: NumCast
), we could convert the unknown T
to i128
/u128
and do the comparison -- but that NumCast
conversion returns an Option
. Thus a u256::MAX
couldn't be compared that way.
I see what you mean. I guess that's clever and keeps the amount of code & traits down, although the usability would be a bit worse.
Right now, the only comparison operation between BigInt types is
PartialOrd<Rhs = Self>
. There are times, however, that other comparisons are helpful, especially comparisons against primitive integer types. Most of these comparisons could beOrd
in the case ofBigInt
,BigUint
, andBigRational
, though comparisons involving complex numbers could not beOrd
. Even comparisons across differently-signed bigint types are trivial, simply adding an O(1) sign check before the vector-based comparing.Admittedly, the primitive type comparison can be done already (e.g.
x >= BigUint::from(8u64)
orx < 16u64.into()
), but this is inefficient in terms of time and memory.Naturally, the reverse (primitive cmp bigint) would also be helpful, though thankfully implementing that will be trivial when the above implementation is made; it's the same source code with some text swapped around. I don't know much about macros, but this sounds like a job for macros.