dnsl48 / fraction

[Rust] Lossless fractions and decimals; drop-in float replacement
Apache License 2.0
83 stars 25 forks source link

Deprecate fraction::convert #16

Open lovesegfault opened 5 years ago

lovesegfault commented 5 years ago

Now that TryFrom and TryInto are stable, the internal replacement should be deprecated in their favour, as hinted at by the documentation.

dnsl48 commented 5 years ago

Agree. Scheduling that for 0.7.0.

lovesegfault commented 5 years ago

FWIW I tried my hand at this but it was a more thorough change than I had expected, since std::convert::TryInto/From yield Result while the built-in trait yields Option

dnsl48 commented 5 years ago

I think there's no much we would gain from Result::Err, but to conform with Rust APIs we'll go for Result<T, ()>.

dnsl48 commented 5 years ago

Actually, now I'm thinking, it could become Result<T, Self>

lovesegfault commented 5 years ago

@dnsl48 That's better, I think!

dnsl48 commented 5 years ago

I've had a go at it and unfortunately, that's not going to be feasible until we get specializations because of the core blanket trait implementations for TryFrom and TryInto.

error[E0119]: conflicting implementations of trait `std::convert::TryFrom<fraction::GenericFraction<_>>` for type `fraction::GenericFraction<_>`:
    --> src/fraction/mod.rs:4400:9
     |
4400 | /         impl<T, F> TryFrom<GenericFraction<F>> for GenericFraction<T>
4401 | |         where
4402 | |             T: Copy + Integer + TryFrom<F>,
4403 | |             F: Copy + Integer,
...    |
4409 | |             }
4410 | |         }
     | |_________^
     |
     = note: conflicting implementation in crate `core`:
             - impl<T, U> std::convert::TryFrom<U> for T
               where U: std::convert::Into<T>;

error: aborting due to previous error

For more information about this error, try `rustc --explain E0119`.

I guess an option for now is to replace the current convert custom internal implementation with the core try_from for the built-in types. However, we still cannot get rid of TryToConvertFrom trait completely without specializations.

lovesegfault commented 5 years ago

@dnsl48 That's a shame, better than nothing still!

Someday we'll get specialization... someday