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

Associated type regression? unable to infer enough type information about _ #28871

Closed
arcnmx opened this issue Oct 6, 2015 · 14 comments
Closed
Labels
A-associated-items Area: Associated items (types, constants & functions) regression-from-stable-to-beta Performance or correctness regression from stable to beta.

Comments

@arcnmx
Copy link
Contributor

arcnmx commented Oct 6, 2015

rustc seems to accept this under stable, but not beta or nightly.

playpen repro

trait T {
    type T;
}

struct S;
impl T for S {
    type T = S;
}

trait T2 {
    type T: Iterator<Item=<S as T>::T>;
}
error: unable to infer enough type information about `_`; type annotations or generic parameter binding required
@arielb1
Copy link
Contributor

arielb1 commented Oct 6, 2015

This is the ParamCandidate vs. ProjectionCandidate issue in project rather than in select. Should we do the "just pick the where-clause" thing here too? cc @nikomatsakis

Maybe just apply them all (and enjoy the type-errors on inconsistency) if there are no inference variables - as select will happily deal with these?

@huonw huonw added regression-from-stable-to-beta Performance or correctness regression from stable to beta. A-associated-items Area: Associated items (types, constants & functions) labels Oct 7, 2015
@nikomatsakis
Copy link
Contributor

@arielb1

This is the ParamCandidate vs. ProjectionCandidate issue in project rather than in select. Should we do the "just pick the where-clause" thing here too? cc @nikomatsakis

I suppose we ought to, yes, for consistency. I imagine the same forces that favor where clauses (helping along inference) will play out here too.

@brson
Copy link
Contributor

brson commented Oct 20, 2015

This will hit stable in a little over a week.

@brson
Copy link
Contributor

brson commented Oct 20, 2015

cc @rust-lang/compiler @rust-lang/lang

@nikomatsakis
Copy link
Contributor

It's not as simple as preferring where-clauses over impls. There are in fact two relevant where-clauses included in the environment, one with <S as Trait>::T (unnormalizd) and one with just S. I'm not sure where the duplicate comes from, investigating.

@nikomatsakis
Copy link
Contributor

I think the problem derives from the fact that assemble_candidates_from_trait_def does not normalize.

@nikomatsakis
Copy link
Contributor

(Also, this is caused by the RFC 1214 changes, which are checking for WF'd-ness, which is where the error occurs.)

@nikomatsakis
Copy link
Contributor

So, while this example is a regression, the underlying problem is pre-existing. For example, this test case shows the same conflict (note that the compilation error goes away if you change <S as T>::T in the trait definition to S, even though those ought to be the same). I'm not sure of the best fix overall, honestly, though lazy normalization as I've been envisioning it could help here (but you'd have to add in a policy of "apply all applicable where clauses", which is perhaps fairly sane, modulo inference). I can imagine some shorter-term hacky fixes, which I will investigate, but it may also make sense to accept this regression as a pre-existing shortcoming of normalization (albeit one exacerbated by other bugfixes).

@arcnmx I am curious whether you have some larger context for this minified example.

@arielb1
Copy link
Contributor

arielb1 commented Oct 21, 2015

@nikomatsakis

I was thinking of just picking all where-clauses and projections when there are no inference variables involved. "guessing" in projections is stupid anyway.

@arielb1
Copy link
Contributor

arielb1 commented Oct 21, 2015

@nikomatsakis

projection just uses ParamEnv for its equivalent of ProjectionCandidate. This still won't help if the candidates are mostly the same. The code handles projections pretty idiotically, however.

I would prefer that we make projection not generate unneeded candidates - a projection predicate should always be coupled with some proof of the relevant selection predicate holding.

@nikomatsakis
Copy link
Contributor

@arielb1 certainly just converting to use ProjectionCandidate -- and then give ParamEnv priority -- would be a relatively simple solution to the immediate problem. I'm not sure I understood this comment:

I would prefer that we make projection not generate unneeded candidates - a projection predicate should always be coupled with some proof of the relevant selection predicate holding.

Do you mean that we should fully evaluate all nested obligations? This isn't always possible if there are inference variables, of course.

nikomatsakis added a commit to nikomatsakis/rust that referenced this issue Oct 22, 2015
trait definitions, and give prefence to the former. This is consistent
with what we do for selection. It also works around a limitation
that was leading to rust-lang#28871.
@arcnmx
Copy link
Contributor Author

arcnmx commented Oct 23, 2015

@nikomatsakis there's not much greater context, really. I was using a trait as a sort of "static" compile-time enforced API, so type aliases to <imp::Defs as Defs>::Error and such were used. It's silly, but when these types were used in iterators, they'd fail!

bors added a commit that referenced this issue Oct 23, 2015
Give preference to projections from where-clauses over those from trait definitions. This makes #28871 work again, though I think there's more to fix in this general area.

r? @arielb1 
cc @brson (fixes regression)
brson pushed a commit to brson/rust that referenced this issue Oct 24, 2015
trait definitions, and give prefence to the former. This is consistent
with what we do for selection. It also works around a limitation
that was leading to rust-lang#28871.
@nikomatsakis
Copy link
Contributor

Fixed by #29241.

@nikomatsakis
Copy link
Contributor

@arcnmx thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-associated-items Area: Associated items (types, constants & functions) regression-from-stable-to-beta Performance or correctness regression from stable to beta.
Projects
None yet
Development

No branches or pull requests

5 participants