fraction icon indicating copy to clipboard operation
fraction copied to clipboard

Deprecate fraction::convert

Open lovesegfault opened this issue 5 years ago • 7 comments

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

lovesegfault avatar Aug 23 '19 04:08 lovesegfault

Agree. Scheduling that for 0.7.0.

dnsl48 avatar Aug 25 '19 19:08 dnsl48

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

lovesegfault avatar Aug 25 '19 22:08 lovesegfault

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 avatar Aug 25 '19 23:08 dnsl48

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

dnsl48 avatar Aug 26 '19 02:08 dnsl48

@dnsl48 That's better, I think!

lovesegfault avatar Aug 26 '19 06:08 lovesegfault

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.

dnsl48 avatar Oct 05 '19 20:10 dnsl48

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

Someday we'll get specialization... someday

lovesegfault avatar Oct 05 '19 21:10 lovesegfault