Refactor compare_impl_method to use all bounds #20290
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jroesch
force-pushed
the
generalize-impl-bounds
branch
4 times, most recently
from
4dfaccf to
5a2568b
Compare
| ); | ||
|
|
||
| // Construct trait parameter environment and then shift it into the skolemized viewpoint. | ||
| let mut trait_param_env = |
There was a problem hiding this comment.
I think you probably want impl_env cloned and using hybrid_preds, rather than starting with the trait environment. Not that it really matters. But nonetheless, it seems more correct: we've brought everything into the impl namespace, so those are the type parameters etc that are in scope.
jroesch
force-pushed
the
generalize-impl-bounds
branch
2 times, most recently
from
098990d to
03d7e42
Compare
|
We should definitely get the spans cleaned up before we land this. I can help you with that. |
jroesch
force-pushed
the
generalize-impl-bounds
branch
4 times, most recently
from
45a8708 to
574d824
Compare
|
This is waiting on #20789 and a rebase before it is ready to merged. |
jroesch
force-pushed
the
generalize-impl-bounds
branch
3 times, most recently
from
b175240 to
fe65af0
Compare
Refactor compare_impl_method into its own file. Modify the code to stop comparing individual parameter bounds. Instead we now use the predicates list attached to the trait and implementation generics. This ensures consistency even when bounds are declared in different places (i.e on a parameter vs. in a where clause).
jroesch
force-pushed
the
generalize-impl-bounds
branch
from
fe65af0 to
6a66b32
Compare
bors
added a commit
that referenced
this pull request
Jan 15, 2015
…sakis This should fix both #20020 and #20107. This moves out the code into its own file. I have a couple concerns that can either be addressed in this PR or in a future one. - The error reporting for the fulfillment context should be span aware because currently it is attached to the top of the file which is less then desirable. - There is a failure in the test file: run-pass/issue-2611-3.rs, this seems like it should be a failure to me, but I am not sure. As a nit I'm not enthused about the file name, and am open to better suggestions. r? @nikomatsakis
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This should fix both #20020 and #20107. This moves out the code into its own file.
I have a couple concerns that can either be addressed in this PR or in a future one.
of the file which is less then desirable.
As a nit I'm not enthused about the file name, and am open to better suggestions.
r? @nikomatsakis