-
Notifications
You must be signed in to change notification settings - Fork 24
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
Consider implementing From<T> where T: Into<#inner_type>? #124
Comments
@schneiderfelipe Hi, thanks for bringing this. I guess I had a thought about it, the reason I've decided not to implement it is clarity / explicitness. What's you real life use case, where you think it would make things a bit easier? |
Well, it would make newtypes behave more like the inner type. For instance, something like the following would work, but currently doesn't: #[newtype(derive(From))]
struct MyVec(Vec<isize8>);
// This works
let my_vec: Vec<isize8> = [0, 1, 2].into();
// This does not work, requires a separate From impl
let my_vec: MyVec = [0, 1, 2].into(); |
@schneiderfelipe Sold! :) @danma3x Would you be interested in addressing this one? |
Sure! |
There's a problem with my proposed change: it produces conflicting implementations of the trait error[E0119]: conflicting implementations of trait `From<_>` for type `derives::test_trait_from_string::__nutype_Name__::Name`
--> test_suite/tests/string.rs:357:9
|
357 | #[nutype(derive(From))]
| ^^^^^^^^^^^^^^^^^^^^^^^
| |
| first implementation here
| conflicting implementation for `derives::test_trait_from_string::__nutype_Name__::Name`
|
= note: this error originates in the attribute macro `nutype` (in Nightly builds, run with -Z macro-backtrace for more info) In particular, for the case of integers, things seem much worse: error[E0277]: the trait bound `u32: From<i32>` is not satisfied
--> test_suite/tests/integer.rs:532:22
|
532 | let amount = Amount::from(350);
| ^^^^^^ the trait `From<i32>` is not implemented for `u32`
|
= help: the following other types implement trait `From<T>`:
<u32 as From<bool>>
<u32 as From<char>>
<u32 as From<u8>>
<u32 as From<u16>>
<u32 as From<NonZeroU32>>
<u32 as From<Ipv4Addr>>
= note: required for `i32` to implement `Into<u32>`
note: required for `traits::test_trait_from::__nutype_Amount__::Amount` to implement `From<i32>`
--> test_suite/tests/integer.rs:529:9
|
529 | #[nutype(derive(From))]
| ^^^^^^^^^^^^^^^^^^^^^^^ unsatisfied trait bound introduced here
530 | pub struct Amount(u32);
| ^^^^^^
= note: this error originates in the attribute macro `nutype` (in Nightly builds, run with -Z macro-backtrace for more info) So the compiler seems to figure out that I'm afraid my proposal can't be done 😞 @greyblake what do you think? |
this one indeed looks bad:
And this one is kind of expected:
Because the type for Could point to you changes in a branch? |
The changes I made are in the master branch of my fork schneiderfelipe@cca7047 |
Considering nutypes with validation may be fallible, and |
@asasine I think you're correct. If this ever gets implemented, one would consider extending |
Conversions are fallible when there is validation. When there is no validation set, conversions are infallible. If there is validation, nutype won't allow you to derive |
Ok I gave a try for a generic From implementation, but it's not gonna work the way we want. I you want to reproduce the example, I pushed my changes in generic-from-impl-demo, see also d73ddeb commit. Consider the following example: #[nutype(derive(Into, From))]
pub struct Amount(i32); This won't compile:
The expands into the following:
Note, that But in this case it becomes also a source of a problem. It's not very obvious to see, but Rust finds 2 conflicting implementation for for Other path Rust could take is: This could be addressed by implementing Between these 2 available options, there is no clear winner, a trade off needs to be make. With that I am closing the issue. Thanks for you attention, if you followed me! |
@greyblake nicely explained! I'm afraid this won't ever be possible, except maybe if specialisation gets stabilised (rust-lang/rust#31844). Thank you for taking the time though, really appreciated! |
Currently
derive(From)
producesnutype/nutype_macros/src/common/gen/traits.rs
Lines 125 to 134 in c8e3f72
Is there anything holding back the production of something like the following?
The text was updated successfully, but these errors were encountered: