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

Can not use derive_more::From with forward and derive_more::Into at the same time #198

Closed
xu-cheng opened this issue Aug 28, 2022 · 5 comments

Comments

@xu-cheng
Copy link

xu-cheng commented Aug 28, 2022

The following code cannot be compiled:

use std::collections::HashMap;

#[derive(
    derive_more::From,
    derive_more::Into,
)]
#[from(forward)]
pub struct Foo(HashMap<i32, i32>);

I got the following error message:

error[E0119]: conflicting implementations of trait `std::convert::From<Foo>` for type `Foo`
 --> src/lib.rs:4:5
  |
4 |     derive_more::From,
  |     ^^^^^^^^^^^^^^^^^
  |
  = note: conflicting implementation in crate `core`:
          - impl<T> From<T> for T;
  = note: this error originates in the derive macro `derive_more::From` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0119`.
error: could not compile `foo` due to previous error

FYI, this is the generated code obtained from cargo expand

#![feature(prelude_import)]
#[prelude_import]
use std::prelude::rust_2021::*;
#[macro_use]
extern crate std;
use std::collections::HashMap;
#[from(forward)]
pub struct Foo(HashMap<i32, i32>);
#[automatically_derived]
impl<__FromT0> ::core::convert::From<(__FromT0)> for Foo
where
    HashMap<i32, i32>: ::core::convert::From<__FromT0>,
{
    #[inline]
    fn from(original: (__FromT0)) -> Foo {
        Foo(<HashMap<i32, i32> as ::core::convert::From<__FromT0>>::from(original))
    }
}
#[automatically_derived]
impl ::core::convert::From<Foo> for (HashMap<i32, i32>) {
    #[inline]
    fn from(original: Foo) -> Self {
        (original.0)
    }
}
@xu-cheng
Copy link
Author

This seems to be an known issue in Rust's trait system. We probably need negative trait bound to fix it.

@tyranron
Copy link
Collaborator

@xu-cheng yes, there is nothing we can do here from our side. Once Rust allows such things through specialization or anything else, these derives will support them too.

@xu-cheng
Copy link
Author

xu-cheng commented Aug 29, 2022

@tyranron Well, we can actually workaround this issue like this:

use std::collections::HashMap;

pub struct Foo(HashMap<i32, i32>);

impl<T> From<T> for Foo
where
    HashMap<i32, i32>: From<T>,
{
    #[inline]
    fn from(original: T) -> Foo {
        Foo(<HashMap<i32, i32> as From<T>>::from(original))
    }
}

impl Into<HashMap<i32, i32>> for Foo {
    #[inline]
    fn into(self) -> HashMap<i32, i32> {
        self.0
    }
}

fn main() {
    let hashmap: HashMap<i32, i32> = HashMap::from([(1, 1)]);
    let foo = Foo::from(hashmap);
    let hashmap2: HashMap<i32, i32> = foo.into();
}

The only downside is that HashMap::from(Foo) is not available. Would it make sense to make derive_more offers an option to implement Into instead of From, like this:

#[derive(
    derive_more::From,
    derive_more::Into,
)]
#[from(forward)]
#[into(into_only)]
pub struct Foo(HashMap<i32, i32>);

@tyranron
Copy link
Collaborator

@xu-cheng I do think it's not worth it. Into and From are intuitively interchangeable. Providing an option to have only an .into() which doesn't work in From trait bounds and in HashMap::from(Foo) form would be rather confusing.

It's not that much of code to write by hand, if you really need it. But I'd argue that would be a good pattern in a code base. I'd rather go with providing a Foo::into_inner(self) method instead.

@ilslv @JelteF any comments on this?

@ilslv
Copy link
Contributor

ilslv commented Aug 30, 2022

Providing an option to have only an .into() which doesn't work in From trait bounds and in HashMap::from(Foo) form would be rather confusing.

I agree with @tyranron

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

3 participants