search

rust-num / num-traits

Numeric traits for generic mathematics in Rust
Apache License 2.0
731 stars 135 forks source link

Add a blanket impl<T: ToPrimitive> ToPrimitive for &T #191

Closed coolreader18 closed 4 years ago

coolreader18 commented 4 years ago

Not sure if this is a breaking change or not, but I think this makes sense.

cuviper commented 4 years ago

Yes, it is a breaking change, because &T is a fundamental type.

Can you explain why you want to be generic in this way? Maybe it can be done another way, like T: Borrow<U>, U: ToPrimitive to accept either owned or borrowed values.

coolreader18 commented 4 years ago

This is specifically due to a problem I ran into with passing a &BigInt to NumCast::from(), which expects an owned T where T: ToPrimitive. Would it be better to change that to accept a AsRef/Borrow<T>?

cuviper commented 4 years ago

Hmm, I doubt we could change NumCast::from without it being a breaking change. I guess we could add a new method like from_ref, but that would need T: Clone + ToPrimitive so it can be defaulted to use from, while all of our impls can call specific to_foo conversions without cloning.

Personally, I avoid NumCast because I don't like how it overlaps with From::from. Directly using ToPrimitive is also better for &BigInt since the methods only require &self.

I'd also suggest the standard TryFrom if possible, stable as of Rust 1.34, but that's not implemented for floating point.

coolreader18 commented 4 years ago

Ah, yeah, TryFrom/Into might be better; I can't use ToPrimitive directly because I'm using a libc type definition. Very specific use case :upside_down_face:. I'll close this I guess, but maybe it's something to think if num-traits ever gets a 0.3? Though that's probably unlikely anytime soon.