Consider implementing From<T> where T: Into<#inner_type>?
Closed this issue ยท 12 comments
Currently derive(From)
produces
nutype/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?
quote! {
impl<T: Into<#inner_type>> ::core::convert::From<T> for #type_name {
#[inline]
fn from(raw_value: T) -> Self {
Self::new(raw_value.into())
}
}
}
@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.
But you may make me reconsider this.
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!
@schneiderfelipe Sold! :)
@danma3x Would you be interested in addressing this one?
There's a problem with my proposed change: it produces conflicting implementations of the trait From
, e.g.,
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 350
should be a u32
just fine if there is only a single From
implementation, but "falls back" to i32
and relies on a separate From<i32>
implementation otherwise, which should not exist obviously.
I'm afraid my proposal can't be done ๐ @greyblake what do you think?
this one indeed looks bad:
error[E0119]: conflicting implementations of trait `From<_>`
And this one is kind of expected:
error[E0277]: the trait bound `u32: From<i32>` is not satisfied
Because the type for new
is generic now and Rust cannot derive type of 350
and in such situations it assumes usually i32
, but i32
cannot be converted into u32
. That can be gone if type is explicitly specified, for example 350u32
.
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 From
shouldn't support fallible conversions, shouldn't the trait implementation be TryFrom
, not From
?
@asasine I think you're correct. If this ever gets implemented, one would consider extending TryFrom
as well (and only that in fallible cases as you mentioned). @greyblake thoughts?
Considering nutypes with validation may be fallible, and
From
shouldn't support fallible conversions, shouldn't the trait implementation beTryFrom
, notFrom
?
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 From
.
Though you can still derive TryFrom
if there is no validation.
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:
error[E0119]: conflicting implementations of trait `From<Amount>` for type `Amount`
The expands into the following:
impl ::core::convert::From<Amount> for i32 {
#[inline]
fn from(value: Amount) -> Self {
value.into_inner()
}
}
impl<T> ::core::convert::From<T> for Amount
where
i32: ::core::convert::From<T>,
{
#[inline]
fn from(raw_value: T) -> Self {
let inner_value = i32::from(raw_value);
Self::new(inner_value)
}
}
Note, that Into<i32> for Amount
is actually defined as From<Amount> for i32
, to keep the symmetry. Into<i32> for Amount
will got implemented automatically through blanket implementation. This is common practice / idiom of the Rust language.
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 From<Amount> for Amount
.
The std has reasonable implementation of impl<T> From<T> for T
, meaning converting from type T
to T
should just return self.
Other path Rust could take is: Amount -> i32 ->Amount
, because our generic implementation of From
enable this.
This could be addressed by implementing Into
trait as solely Into<i32> from Amount
,
meaning we lose From<Amount> for i32
implementation.
Between these 2 available options, there is no clear winner, a trade off needs to be make.
In this regard, my personal preference would be the current status quo.
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!