Skip to content

Fix inference for projections with associated traits with multiple bounds; fixes #34792#34912

Closed
rozbb wants to merge 1 commit into
rust-lang:masterfrom
rozbb:fix-34792
Closed

Fix inference for projections with associated traits with multiple bounds; fixes #34792#34912
rozbb wants to merge 1 commit into
rust-lang:masterfrom
rozbb:fix-34792

Conversation

@rozbb

@rozbb rozbb commented Jul 19, 2016

Copy link
Copy Markdown

The problem was due to the fact that no particular trait bound was specified when a projection was matched with a trait bound, even when multiple bounds existed. The compiler consistently picked the first bound on the associated type that it found that was able to match the type's obligation (which, in the regression test, is <F as Foo> or <Self as Foo>).

The fix involved modifying the SelectionCandidate::ProjectionCandidate variant to carry a trait ref to refer to its matching bound. And instead of stopping on the first match we find, we add all matching bounds to the candidates vector. Also in the winnowing step, a ProjectionCandidate cannot trump another ProjectionCandidate unless they are identical.

@rust-highfive

Copy link
Copy Markdown
Contributor

r? @pnkfelix

(rust_highfive has picked a reviewer for you, use r? to override)

@rozbb

rozbb commented Jul 19, 2016

Copy link
Copy Markdown
Author

r? @eddyb

@rust-highfive rust-highfive assigned eddyb and unassigned pnkfelix Jul 19, 2016
@rozbb

rozbb commented Jul 19, 2016

Copy link
Copy Markdown
Author

I recall someone saying that this might require a crater test. Is that still the case?

@eddyb

eddyb commented Jul 19, 2016

Copy link
Copy Markdown
Contributor

@doomrobo Indeed, although you first need to fix the two compile-fail tests Travis reports - looks like the error messages changed a bit there.

Comment thread src/librustc/traits/select.rs Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't need to be a probe, I believe in_snapshot will do.

@rozbb

rozbb commented Jul 20, 2016

Copy link
Copy Markdown
Author

Hmm. It appears that the error messages for test/compile-fail/associated-types-project-from-hrtb-in-fn-body.rs and test/compile-fail/associated-types-subtyping-1.rs have gotten significantly worse.

Here is the gist for the first one. The second differs in the same way. I have no intuitions so far, but I'm looking into it.

@eddyb

eddyb commented Jul 20, 2016

Copy link
Copy Markdown
Contributor

cc @nikomatsakis @jonathandturner What could cause those errors? It looks like the Sized bound necessary on local variables, but what is it mismatching on?

@bors

bors commented Aug 12, 2016

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #35091) made this pull request unmergeable. Please resolve the merge conflicts.

@alexcrichton

Copy link
Copy Markdown
Member

Closing due to inactivity, but feel free to resubmit with a rebase!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants