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

regression around trait bounds with associated types #33586

Closed
nikomatsakis opened this issue May 12, 2016 · 10 comments · Fixed by #33596
Closed

regression around trait bounds with associated types #33586

nikomatsakis opened this issue May 12, 2016 · 10 comments · Fixed by #33596
Labels
regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@nikomatsakis
Copy link
Contributor

The following code works on stable, but not on the latest master. I believe it is valid.

trait ParallelIterator {
    type Item;
    fn drive_unindexed<C>(self, consumer: C) -> C::Result
        where C: Consumer<Self::Item>;
}

pub trait Consumer<ITEM> {
    type Result;
}

pub struct Enumerate<M> {
    base: M,
}

impl<M> ParallelIterator for Enumerate<M>
    where M: ParallelIterator,
{
    type Item = (usize, M::Item);

    fn drive_unindexed<C>(self, consumer: C) -> C::Result
        where C: Consumer<Self::Item>
    {
        panic!()
    }
}

fn main() { }

On master I get:

lunch-box. rustc ~/tmp/reduce.rs
/home/nmatsakis/tmp/reduce.rs:3:5: 4:39 error: the trait bound `C: Consumer<Self>` is not satisfied [E0277]
/home/nmatsakis/tmp/reduce.rs:3     fn drive_unindexed<C>(self, consumer: C) -> C::Result
                                    ^
/home/nmatsakis/tmp/reduce.rs:3:5: 4:39 help: run `rustc --explain E0277` to see a detailed explanation
/home/nmatsakis/tmp/reduce.rs:3:5: 4:39 help: consider adding a `where C: Consumer<Self>` bound
/home/nmatsakis/tmp/reduce.rs:20:5: 24:6 error: the trait bound `C: Consumer<Enumerate<M>>` is not satisfied [E0277]
/home/nmatsakis/tmp/reduce.rs:20     fn drive_unindexed<C>(self, consumer: C) -> C::Result
                                     ^
/home/nmatsakis/tmp/reduce.rs:20:5: 24:6 help: run `rustc --explain E0277` to see a detailed explanation
/home/nmatsakis/tmp/reduce.rs:20:5: 24:6 help: consider adding a `where C: Consumer<Enumerate<M>>` bound
error: aborting due to 2 previous errors
@nikomatsakis nikomatsakis added regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 12, 2016
@nikomatsakis
Copy link
Contributor Author

(This affects the latest Rayon.)

@nikomatsakis
Copy link
Contributor Author

Note that the error messages are talking about Consumer<Self>.

@nikomatsakis
Copy link
Contributor Author

Really this is enough to show the problem:

trait ParallelIterator {
    type Item;
    fn drive_unindexed<C>(self, consumer: C) -> C::Result
        where C: Consumer<Self::Item>;
}

pub trait Consumer<ITEM> {
    type Result;
}

fn main() { }

@nikomatsakis
Copy link
Contributor Author

This DOES compile, which gives a pretty good clue:

trait ParallelIterator {
    type Item;
    fn drive_unindexed<C>(self, consumer: C) -> <C as Consumer<Self::Item>>::Result
        where C: Consumer<Self::Item>;
}

pub trait Consumer<ITEM> {
    type Result;
}

fn main() { }

@nikomatsakis
Copy link
Contributor Author

cc @eddyb any PRs that you've seen whih might affect how C::Result is resolved?

@metajack
Copy link
Contributor

This is blocking a Servo upgrade of rustc currently. We seem to be stuck around 2016-05-07 until this is fixed.

@nikomatsakis
Copy link
Contributor Author

Seems likely to be a problem in the resolve refactoring. Basically the Self::Item in Consumer<Self::Item> gets translated to Self.

cc @jseyfried the problem seems to lie in resolve

@nikomatsakis
Copy link
Contributor Author

never mind, I think the problem is in astconv

@eddyb
Copy link
Member

eddyb commented May 12, 2016

Got to love nondescript cleanup commits.
This broke saving the final Def, which was completely hidden by the ast_ty_to_ty_cache, until I removed that recently.
It wasn't a problem with expression paths because those don't use the resulting Def for anything and instead insert their own resolved method or associated constant Def.

@alexcrichton
Copy link
Member

I've also noticed that this crate fails on the current nightly for presumably similar reasons

eddyb added a commit to eddyb/rust that referenced this issue May 13, 2016
…-type-path, r=eddyb

re-introduce a cache for ast-ty-to-ty

It turns out that `ast_ty_to_ty` is supposed to be updating the `def`
after it finishes, but at some point in the past it stopped doing
so. This was never noticed because of the `ast_ty_to_ty_cache`, but that
cache was recently removed. This PR fixes the code to update the def
properly, but apparently that is not quite enough to make the operation
idempotent, so for now we reintroduce the cache too.

Fixes rust-lang#33586.

r? @eddyb
bors added a commit that referenced this issue May 13, 2016
… r=eddyb

re-introduce a cache for ast-ty-to-ty

It turns out that `ast_ty_to_ty` is supposed to be updating the `def`
after it finishes, but at some point in the past it stopped doing
so. This was never noticed because of the `ast_ty_to_ty_cache`, but that
cache was recently removed. This PR fixes the code to update the def
properly, but apparently that is not quite enough to make the operation
idempotent, so for now we reintroduce the cache too.

Fixes #33586.

r? @eddyb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants