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

Search for implemented kinds recursively on Trait types. Fixes #15155 and #13155. #19011

Merged
merged 1 commit into from
Nov 26, 2014

Conversation

ricky26
Copy link
Contributor

@ricky26 ricky26 commented Nov 16, 2014

It looks like currently kinds required by traits are not propagated when they are wrapped in a TyTrait. Additionally, in SelectionContext::builtin_bound, no attempt is made to check whether the target trait or its supertraits require the kind specified.

This PR alters SelectionContext::builtin_bound to examine all supertraits in the target trait's bounds recursively for required kinds.

Alternatively, the kinds could be added to the TyTrait upon creation (by just setting its builtin_bounds to the union of the bounds requested in this instance and the bounds required by the trait), this option may have less overhead during compilation but information is lost about which kinds were explicitly requested for this instance (vs those specified by traits/supertraits) would be lost.

@alexcrichton
Copy link
Member

Oh wow, I've always wanted this bug fixed, thanks @ricky26! r? @nikomatsakis

@nikomatsakis
Copy link
Contributor

Hi! Thanks for submitting this PR. As I wrote here, I think this is the right general idea, but you can just re-use the supertraits iterator rather than walking the data structures yourself. Let me know when you've got an updated version and I'll review it again, thanks a lot!

@emberian
Copy link
Member

Ping @ricky26

@ricky26
Copy link
Contributor Author

ricky26 commented Nov 23, 2014

@cmr I ended up being busy this week - I was going to fix this up today.

@emberian
Copy link
Member

No problem! Just checking if this PR is stalled :)

@ricky26
Copy link
Contributor Author

ricky26 commented Nov 23, 2014

Okay @nikomatsakis, I've rebased and swapped the supertraits iteration code with a call to the utils::supertraits() method - I've had to create a temporary TraitRef though, since otherwise supertraits() panics.

bors added a commit that referenced this pull request Nov 25, 2014
It looks like currently kinds required by traits are not propagated when they are wrapped in a TyTrait. Additionally, in SelectionContext::builtin_bound, no attempt is made to check whether the target trait or its supertraits require the kind specified.

This PR alters SelectionContext::builtin_bound to examine all supertraits in the target trait's bounds recursively for required kinds.

Alternatively, the kinds could be added to the TyTrait upon creation (by just setting its builtin_bounds to the union of the bounds requested in this instance and the bounds required by the trait), this option may have less overhead during compilation but information is lost about which kinds were explicitly requested for this instance (vs those specified by traits/supertraits) would be lost.
@bors bors closed this Nov 26, 2014
@bors bors merged commit 729bf44 into rust-lang:master Nov 26, 2014
@ricky26 ricky26 deleted the trait_supertraits branch November 26, 2014 20:24
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.

5 participants