search

mhogrefe / malachite

An arbitrary-precision arithmetic library for Rust.
GNU Lesser General Public License v3.0
448 stars 17 forks source link

From implementations panic - use TryFrom instead #4

Closed Andlon closed 1 year ago

Andlon commented 2 years ago

The current From impls for From<f64> for Rational panics if it encounters a NaN or infinity. It is documented to do so in the impl, however it is a violation of the From contract as stipulated by the From trait:

Note: This trait must not fail. The From trait is intended for perfect conversions. If the conversion can fail or is not perfect, use TryFrom.

So it seems that these impls should rather be TryFrom instead.

mhogrefe commented 2 years ago

That's a good point.

A larger issue is that Malachite doesn't use TryFrom at all. TryFrom didn't exist when I started writing Malachite, so I created my own trait, CheckedFrom. The main thing stopping me from migrating to TryFrom is that certain TryFrom impls are missing, e.g. for converting an f64 to a u32. Malachite does provide that conversion via CheckedFrom.

In the short term I'll move the Rational-to-f64 conversion to CheckedFrom and think a bit more about the TryFrom migration.

mhogrefe commented 1 year ago

Fixed in Malachite 0.3.0. I also replaced CheckedFrom by TryFrom everywhere.