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

Improve inference for conditional dispatch #17901

Closed
nikomatsakis opened this issue Oct 10, 2014 · 3 comments
Closed

Improve inference for conditional dispatch #17901

nikomatsakis opened this issue Oct 10, 2014 · 3 comments
Labels
A-traits Area: Trait system

Comments

@nikomatsakis
Copy link
Contributor

In the comments on PR #17669, @japaric raised this example:

use std::vec::MoveItems;

trait IntoIter<I> {
    fn into_iter_(self) -> I;
}

impl<T, I: Iterator<T>> IntoIter<I> for I {
    fn into_iter_(self) -> I {
        self
    }
}

impl<T> IntoIter<MoveItems<T>> for Vec<T> {
    fn into_iter_(self) -> MoveItems<T> {
        self.into_iter()
    }
}

fn into_iter<I, S: IntoIter<I>>(iterable: S) -> I {
    iterable.into_iter_()
}

fn main() {
    let v = vec![0u8, 1, 2];

    // WORKS OK
    let i: MoveItems<u8> = into_iter(v); 

    // FAILS TO INFER
    let i = into_iter(v);
}

This comment describes why inference fails: #17669 (comment)

It's plausible we could infer a result here. More concretely, it would mean changing the logic in evaluate_stack to not be so eager to declare a trait-reference ambiguous it it contains an unbound variable. However, we have to be careful:

  • This rule affects coherence. After all, an impl like impl IntoIter<Vec<u8>> for Vec<u8> would be a coherence violation.
  • This rule also affects an infinite loop problem we had where a rule like impl<T:Eq> Eq for Vec<T>, when matching against an inference variable $0, would unify and yield a subconstraint $1 : Eq for some fresh variable $1, which would then unify with Vec<$2> and repeat into a horrible cycle. This rule was what evades that cycle and made the whole patch work.

So I'm not 100% sure this can be fixed, but I suspect it could if we made the rule more subtle.

@nikomatsakis nikomatsakis added the A-traits Area: Trait system label Oct 15, 2014
nikomatsakis added a commit to nikomatsakis/rust that referenced this issue Nov 6, 2014
…type

variables in the intracrate case. This requires a deeper distinction
between inter- and intra-crate so as to keep coherence working.

I suspect the best fix is to generalize the recursion check that
exists today, but this requires a bit more refactoring to achieve.

(In other words, where today it says OK for an exact match, we'd want
to not detect exact matches but rather skolemize each trait-reference
fresh and return AMBIG -- but that requires us to make builtin bounds
work shallowly like everything else and move the cycle detection into
the fulfillment context.)
@steveklabnik
Copy link
Member

MoveItems is now gone, so I'm not sure what code sample constructs this.

@nikomatsakis
Copy link
Contributor Author

I think I'm going to close this. There are certainly improvements to be made but I think the pressing need for this particular example is past.

@bombless
Copy link
Contributor

cc me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-traits Area: Trait system
Projects
None yet
Development

No branches or pull requests

3 participants