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

Use TryFrom in interpreter API instead of TryInto #1258

Closed
akiross opened this issue May 12, 2022 · 2 comments
Closed

Use TryFrom in interpreter API instead of TryInto #1258

akiross opened this issue May 12, 2022 · 2 comments

Comments

@akiross
Copy link
Contributor

akiross commented May 12, 2022

Using the interpreter API, I noticed that Value implements TryInto instead of TryFrom: this has a few drawbacks and the doc suggests to avoid direct implementation. For example while it's possible to do

let v = Value::from(123i32);
let can_do: i32 = v.try_into().expect("Not a i32");

but it is not possible to do

let v = Value::from(123i32);
let cant_do: i32 = i32::try_from(v).expect("Not a i32");

Implementing TryFrom<Value> for $ty will provide a blanket implementation for TryInto<$ty> for Value, while the inverse is not true. This would also allow to implement functions with generic types that can be obtained from a value, like where T: TryFrom<Value> - while AFAICT this is not possible with the current implementation, without using workarounds.

I forked slint with the intent to open a PR, but I stopped before actually opening it because I'm not sure if this choice was intentional or not (I see no reason why to avoid implementing TryFrom instead of TryInto, but you might have some).

Would a PR be welcome to fix this?

@ogoffart
Copy link
Member

Yes, please open a PR. I don't think this was intentional

@ogoffart
Copy link
Member

ogoffart commented May 12, 2022

And, in fact, since the conversion cannot fail, i believe even implementing From would be working

Edit: ignore that. From is already implemented. And it's for the conversion to the inner type, and yes, then TryFrom can be used.

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

No branches or pull requests

2 participants