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

impl Fn with generic associated type conflicts, succeeds with concrete type #40761

Closed
aidanhs opened this issue Mar 23, 2017 · 8 comments
Closed
Labels
A-trait-system Area: Trait system C-bug Category: This is a bug.

Comments

@aidanhs
Copy link
Member

aidanhs commented Mar 23, 2017

Wasn't really sure how to describe this issue, please edit title if there's a better summary.

extern crate serde;
use serde::Serialize;

trait SerializeSend {
    fn x(&self);
}
impl<T: Serialize + 'static> SerializeSend for T { fn x(&self) {} }
// Works
impl<T1> SerializeSend for Fn(T1)                { fn x(&self) {} }
// Fails
//impl<T1, T2> SerializeSend for Fn(T1) -> T2      { fn x(&self) {} }

fn main() {
    5usize.x();
    (Box::new(|_: u8| ()) as Box<Fn(_)>).x();
    //x(Box::new(|_: u8| 5u8) as Box<Fn(_)>).x();
}

http://play.integer32.com/?gist=021df0320a79df0146121b59c61ec287&version=undefined

The above compiles, but if you comment out the works line and uncomment the fails line (so there is only one impl of Fn), the compiler will error with:

   Compiling playground v0.0.1 (file:///playground)
error[E0119]: conflicting implementations of trait `SerializeSend` for type `std::ops::Fn(_) -> _ + 'static`:
  --> src/main.rs:11:1
   |
7  | impl<T: Serialize + 'static> SerializeSend for T { fn x(&self) {} }
   | ------------------------------------------------------------------- first implementation here
...
11 | impl<T1, T2> SerializeSend for Fn(T1) -> T2      { fn x(&self) {} }
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `std::ops::Fn(_) -> _ + 'static`

error: aborting due to previous error

error: Could not compile `playground`.

To learn more, run the command again with --verbose.

I'm not clear why there seems to be an inconsistency here. There are a bunch of somewhat related issues to do with conflicts and associated types (#19032, #23341), but nothing that I can see that's directly relevant to this, where the two things being impl'd on are seemingly unrelated.

@durka
Copy link
Contributor

durka commented Mar 23, 2017

Minimized:

trait Trait {}

trait Associate {
    type Assoc;
}

impl<T> Trait for T {}

impl Trait for Associate<Assoc=()> {}   // ok
impl<T> Trait for Associate<Assoc=T> {} // error

@durka
Copy link
Contributor

durka commented Mar 23, 2017

Note that both are errors if the blanket impl is changed to T: ?Sized.

@daboross
Copy link
Contributor

Going off of what durka wrote, impl<T: Sized> Trait for Associate<Assoc=T> {} succeeds...?

@daboross
Copy link
Contributor

daboross commented Mar 24, 2017

withoutboats just explained a bit of the impl Trait for OtherTrait syntax in IRC, I'll try to repeat what I heard and do some out-loud thinking about this issue:

It seems impl Trait for OtherTrait is actually the syntax for implementing methods on the OtherTrait trait object, so like &OtherTrait and Box<OtherTrait> could be used. Because of this, every type in the implementation has to be sized.

impl<A, T> Trait for T where T: Associate<Assoc=A> {} is the right way to write this without involving trait objects.

impl<T1, T2, T3> SerializeSend for T3 where T3: Fn(T1) -> T2 { fn x(&self) {} } would be the way to write an implementation for the non-trait-object Fn in the OP, but it seems this fails because of T1 not being constrained.

All together this is pretty tricky to implement, especially because some struct XXX could exist with both impl Fn(i32) -> i32 for XXX { } and impl Fn(i64) -> i64 for XXX { }. If this XXX struct were called with your SerializeSend::x method, which implementation of Fn does it choose to use?

impl<T1, T2: Sized> SerializeSend for Fn(T1) -> T2 { fn x(&self) {} } may work correctly, since it forces the ambiguity onto the person constructing the Fn(T1) -> T2 object and requires T2 to be sized correctly for the trait object, but for some reason this implementation then conflicts with impl<T: Serialize + 'static> SerializeSend for T { fn x(&self) {} }. I have no idea why this conflicts, when impl<T1> SerializeSend for Fn(T1) { fn x(&self) {} } doesn't.

@durka
Copy link
Contributor

durka commented Mar 24, 2017

Indeed, this normally isn't what you would do -- you'd use a blanket impl (i.e. impl<T: Foo> Trait for T, not impl Trait for Foo). Implementing on a trait object is occasionally legitimate (see std::any::Any), but rare.

Aside from that, I still can't explain the discrepancy in this issue. It seems like once the associated type is made generic, the compiler doesn't realize that Associated<Assoc=T> is still always !Sized‽‽‽

@Mark-Simulacrum Mark-Simulacrum added the A-trait-system Area: Trait system label Jun 14, 2017
@Mark-Simulacrum
Copy link
Member

I'm having a little trouble following this issue. I believe that the bug here is that the following code isn't allowed due to conflicting implementations. However, that's confusing to me, because it seems that T can be (), so I don't see why this is a bug...

trait Trait {}

trait Associate {
    type Assoc;
}

impl<T> Trait for T {}

//impl Trait for Associate<Assoc=()> {}   // ok
impl<T> Trait for Associate<Assoc=T> {} // error

@Mark-Simulacrum Mark-Simulacrum added the C-bug Category: This is a bug. label Jul 27, 2017
@steveklabnik
Copy link
Member

Triage: this seems to be fixed now? The examples compile.

@Dylan-DPC
Copy link
Member

Closing this as it is fixed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-trait-system Area: Trait system C-bug Category: This is a bug.
Projects
None yet
Development

No branches or pull requests

6 participants