Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate fraction::convert #16

Open
lovesegfault opened this issue Aug 23, 2019 · 7 comments
Open

Deprecate fraction::convert #16

lovesegfault opened this issue Aug 23, 2019 · 7 comments

Comments

@lovesegfault
Copy link

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

@dnsl48
Copy link
Owner

dnsl48 commented Aug 25, 2019

Agree. Scheduling that for 0.7.0.

@lovesegfault
Copy link
Author

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
Copy link
Owner

dnsl48 commented Aug 25, 2019

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
Copy link
Owner

dnsl48 commented Aug 26, 2019

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

@lovesegfault
Copy link
Author

@dnsl48 That's better, I think!

@dnsl48
Copy link
Owner

dnsl48 commented Oct 5, 2019

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
Copy link
Author

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

Someday we'll get specialization... someday

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants