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

method dispatcher doesn't handle generic implementations properly #5898

Closed
thestinger opened this issue Apr 16, 2013 · 7 comments
Closed

method dispatcher doesn't handle generic implementations properly #5898

thestinger opened this issue Apr 16, 2013 · 7 comments
Labels
A-frontend Area: Compiler frontend (errors, parsing and HIR)

Comments

@thestinger
Copy link
Contributor

With this example:

pub trait Iterator<A> {
    fn next(&mut self) -> Option<A>;
}

pub trait IteratorUtil<A> {
    fn map<'r, B>(self, f: &'r fn(A) -> B) -> MapIterator<'r, A, B, Self>;
}

impl<A, T: Iterator<A>> IteratorUtil<A> for T {
    fn map<'r, B>(self, f: &'r fn(A) -> B) -> MapIterator<'r, A, B, T> {
        MapIterator{iter: self, f: f}
    }
}

pub struct MapIterator<'self, A, B, T> {
    priv iter: T,
    priv f: &'self fn(A) -> B
}

fn main() {
    let x = Some(5);
    x.map(|x| x.to_str());
}

Option doesn't implement Iterator, but the following error happens:

foo.rs:22:4: 22:26 error: failed to find an implementation of trait Iterator<<V4>> for <V5>
foo.rs:22     x.map(|x| x.to_str());
              ^~~~~~~~~~~~~~~~~~~~~~
@gifnksm
Copy link
Contributor

gifnksm commented May 2, 2013

This issue is similar to #3429 in the sense that the compiler cannot select the proper implementation.
The problem of #3429 is that the implementation for generic type T conflicts with the other 's implementation.

@alexcrichton
Copy link
Member

Nominating for backwards-compatible, and failing that, feature-complete.

@graydon
Copy link
Contributor

graydon commented May 9, 2013

declined for milestone, this feels like just-a-bug of some sort, probably #3429

@Blei
Copy link
Contributor

Blei commented Jun 7, 2013

Yes, this is the same bug as #3429. The underlying issue in both of them is that the initial filtering of relevant methods is not precise enough, i.e. that it treats impls such as the following as being applicable to each type, regardless whether it implements Bar or not:

trait Foo {
    fn foo(&self);
}

trait Bar {
    fn bar(&self);
}

impl <T: Bar> Foo for T {
    fn foo(&self) { self.bar(); }
}

(This filtering currently happens in librustc/middle/typeck/check/method.rs in the method is_relevant)

In this issue, this causes a surprising error for a type that is not connected to the relevant traits at all, in issue #3429, this causes a conflict of multiple methods, even though only one of them should be applicable.

@thestinger
Copy link
Contributor Author

@graydon: I think this should have the backwards compatible milestone because it will mean renaming a bunch of methods like transform -> map due to the current workarounds. By itself, it's not a backwards compatibility issue but the standard library makes it one.

bors added a commit that referenced this issue Jun 11, 2013
This was a lot more painful than just changing `x.each` to `x.iter().advance` . I ran into my old friend #5898 and had to add underscores to some method names as a temporary workaround.

The borrow checker also had other ideas because rvalues aren't handled very well yet so temporary variables had to be added. However, storing the temporary in a variable led to dynamic `@mut` failures, so those had to be wrapped in blocks except where the scope ends immediately.

Anyway, the ugliness will be fixed as the compiler issues are fixed and this change will amount to `for x.each |x|` becoming `for x.iter |x|` and making all the iterator adaptors available.

I dropped the run-pass tests for `old_iter` because there's not much point in fixing a module that's on the way out in the next week or so.
bors added a commit that referenced this issue Jun 13, 2013
The annoying method resolve bug with generic implementations is still around (#5898), but the conflicts ended up being resolved by adding underscores as a hack, in previous commits.
huonw added a commit to huonw/rust that referenced this issue Jun 15, 2013
…h StrVector.

This is caused by StrVector having a generic implementation for &[S]
and so rust-lang#5898 means that method resolution of ~[~[1]].concat() sees that
both StrVector and VectorVector have methods that (superficially) match.

They are now connect_vec and concat_vec, which means that they can actually be
called.
bors added a commit that referenced this issue Jun 15, 2013
This is caused by StrVector having a generic implementation for &[S]
and so #5898 means that method resolution of ~[~[1]].concat() sees that
both StrVector and VectorVector have methods that (superficially) match.

They are now connect_vec and concat_vec, which means that they can actually be
called.
@huonw
Copy link
Member

huonw commented Jun 24, 2013

@indutny is currently working on this.

@catamorphism
Copy link
Contributor

@nikomatsakis confirms that, as @Blei said, this is a dup of #3429. Closing

flip1995 pushed a commit to flip1995/rust that referenced this issue Aug 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-frontend Area: Compiler frontend (errors, parsing and HIR)
Projects
None yet
Development

No branches or pull requests

7 participants