Skip to content

on-demand-ify custom_coerce_unsized_kind and inherent-impls #40683

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 3 commits into from
Mar 29, 2017
Merged

on-demand-ify custom_coerce_unsized_kind and inherent-impls #40683

merged 3 commits into from
Mar 29, 2017

Conversation

nikomatsakis
Copy link
Contributor

This "on-demand" task both checks for errors and computes the custom unsized kind, if any. This task is only defined on impls of CoerceUnsized; invoking it on any other kind of impl results in a bug. This is just to avoid having an Option, could easily be changed.

r? @eddyb

@nikomatsakis nikomatsakis mentioned this pull request Mar 20, 2017
Closed
6 tasks
@eddyb
Copy link
Member

eddyb commented Mar 20, 2017

@bors r+

@bors
Copy link
Collaborator

bors commented Mar 20, 2017

📌 Commit 8effa01 has been approved by eddyb

@bors
Copy link
Collaborator

bors commented Mar 20, 2017

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

@nikomatsakis nikomatsakis force-pushed the incr-comp-coerce-unsized-info branch from 8effa01 to 245d491 Compare March 20, 2017 22:57
@nikomatsakis nikomatsakis changed the title convert custom_coerce_unsized_kind into a coerce_unsized_info on-demand-ify custom_coerce_unsized_kind and inherent-impls Mar 20, 2017
@nikomatsakis
Copy link
Contributor Author

@eddyb -- I added another commit, converting inherent-impl things.

r? on those commits.

eddyb
eddyb reviewed Mar 20, 2017
View reviewed changes
src/librustc_typeck/coherence/inherent.rs Outdated
let mut collect = InherentCollect { tcx };
krate.visit_all_item_likes(&mut collect);

herent
Copy link
Member

Choose a reason for hiding this comment

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

What's this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, what is that? weird.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, that file never got removed...it was supposed to be.

eddyb
eddyb reviewed Mar 20, 2017
View reviewed changes
src/librustc_typeck/coherence/inherent_impls.rs Outdated
match self_ty.sty {
ty::TyAdt(def, _) => {
self.check_def_id(item, def.did);
}
Copy link
Member

Choose a reason for hiding this comment

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

This isn't moved from anywhere else?!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is, not sure why git didn't figure it out. I split up inherent.rs into two files, since it was doing two things.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess I could keep it together. Might be cleaner diff.

Copy link
Member

Choose a reason for hiding this comment

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

What I mean is, you don't have a negative diff on inherent.rs matching the positive diff.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, it's because inherent.rs is all dead now. just pushed a revised version which deleted it.

@nikomatsakis nikomatsakis force-pushed the incr-comp-coerce-unsized-info branch from 245d491 to ba85dc7 Compare March 21, 2017 00:56
@nikomatsakis
Copy link
Contributor Author

@eddyb r? on the new changes

@eddyb
Copy link
Member

eddyb commented Mar 22, 2017

@bors r+

@bors
Copy link
Collaborator

bors commented Mar 22, 2017

📌 Commit ba85dc7 has been approved by eddyb

@bors
Copy link
Collaborator

bors commented Mar 23, 2017

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

@nikomatsakis
convert custom_coerce_unsized_kind into a coerce_unsized_info
1a87fc2 
This "on-demand" task both checks for errors and computes the custom
unsized kind, if any. This task is only defined on impls of
`CoerceUnsized`; invoking it on any other kind of impl results in a bug.
This is just to avoid having an `Option`, could easily be changed.
@nikomatsakis
move check to the top of the file, where I would expect to find it
8e6b10a 
Top-down, top-down!
@nikomatsakis
convert inherent-impl-related things to on-demand queries
a29ae30 
There are now 3 queries:

- inherent_impls(def-id): for a given type, get a `Rc<Vec<DefId>>` with
  all its inherent impls. This internally uses `crate_inherent_impls`,
  doing some hacks to keep the current deps (which, btw, are not clearly
  correct).
- crate_inherent_impls(crate): gathers up a map from types
  to `Rc<Vec<DefId>>`, touching the entire krate, possibly generating
  errors.
- crate_inherent_impls_overlap_check(crate): performs overlap checks
  between the inherent impls for a given type, generating errors.
@nikomatsakis nikomatsakis force-pushed the incr-comp-coerce-unsized-info branch from ba85dc7 to a29ae30 Compare March 23, 2017 17:27
@nikomatsakis
Copy link
Contributor Author

@bors r=eddyb

@bors
Copy link
Collaborator

bors commented Mar 23, 2017

📌 Commit a29ae30 has been approved by eddyb

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Mar 25, 2017
@alexcrichton
Rollup merge of rust-lang#40683 - nikomatsakis:incr-comp-coerce-unsiz…
bbeb0c0 
…ed-info, r=eddyb

on-demand-ify `custom_coerce_unsized_kind` and `inherent-impls`

This "on-demand" task both checks for errors and computes the custom unsized kind, if any. This task is only defined on impls of `CoerceUnsized`; invoking it on any other kind of impl results in a bug. This is just to avoid having an `Option`, could easily be changed.

r? @eddyb
@alexcrichton alexcrichton mentioned this pull request Mar 25, 2017
Closed
bors added a commit that referenced this pull request Mar 25, 2017
@bors
Auto merge of #40810 - alexcrichton:rollup, r=alexcrichton
40ae49a 
Rollup of 11 pull requests

- Successful merges: #40347, #40501, #40516, #40524, #40540, #40642, #40683, #40764, #40778, #40807, #40809
- Failed merges: #40771
bors added a commit that referenced this pull request Mar 25, 2017
@bors
Auto merge of #40810 - alexcrichton:rollup, r=alexcrichton
ed9108d 
Rollup of 11 pull requests

- Successful merges: #40347, #40501, #40516, #40524, #40540, #40642, #40683, #40764, #40778, #40807, #40809
- Failed merges: #40771
@bors
Copy link
Collaborator

bors commented Mar 25, 2017

⌛ Testing commit a29ae30 with merge 774298f...

@bors
Copy link
Collaborator

bors commented Mar 25, 2017

💔 Test failed - status-travis

@arielb1
Copy link
Contributor

arielb1 commented Mar 25, 2017

@bors
Copy link
Collaborator

bors commented Mar 25, 2017

⌛ Testing commit a29ae30 with merge a7890df...

@bors
Copy link
Collaborator

bors commented Mar 25, 2017

💔 Test failed - status-travis

@arielb1
Copy link
Contributor

arielb1 commented Mar 25, 2017

@bors
Copy link
Collaborator

bors commented Mar 25, 2017

⌛ Testing commit a29ae30 with merge 63c826f...

@bors
Copy link
Collaborator

bors commented Mar 25, 2017

💔 Test failed - status-travis

@arielb1
Copy link
Contributor

arielb1 commented Mar 25, 2017

@bors
Copy link
Collaborator

bors commented Mar 26, 2017

⌛ Testing commit a29ae30 with merge 68c387c...

@bors
Copy link
Collaborator

bors commented Mar 26, 2017

💔 Test failed - status-appveyor

@nikomatsakis
Copy link
Contributor Author

Error in gdb. Seems unlikely to be related to this PR, but not impossible.

@nikomatsakis
Copy link
Contributor Author

@bors retry

@alexcrichton alexcrichton mentioned this pull request Mar 27, 2017
Merged
alexcrichton added a commit to alexcrichton/rust that referenced this pull request Mar 27, 2017
@alexcrichton
Rollup merge of rust-lang#40683 - nikomatsakis:incr-comp-coerce-unsiz…
12f6c50 
…ed-info, r=eddyb

on-demand-ify `custom_coerce_unsized_kind` and `inherent-impls`

This "on-demand" task both checks for errors and computes the custom unsized kind, if any. This task is only defined on impls of `CoerceUnsized`; invoking it on any other kind of impl results in a bug. This is just to avoid having an `Option`, could easily be changed.

r? @eddyb
bors added a commit that referenced this pull request Mar 27, 2017
@bors
Auto merge of #40867 - alexcrichton:rollup, r=alexcrichton
32a2d73 
Rollup of 19 pull requests

- Successful merges: #40317, #40516, #40524, #40606, #40683, #40751, #40778, #40813, #40818, #40819, #40824, #40828, #40832, #40833, #40837, #40849, #40852, #40853, #40865
- Failed merges:
bors added a commit that referenced this pull request Mar 28, 2017
@bors
Auto merge of #40867 - alexcrichton:rollup, r=alexcrichton
07a3429 
Rollup of 19 pull requests

- Successful merges: #40317, #40516, #40524, #40606, #40683, #40751, #40778, #40813, #40818, #40819, #40824, #40828, #40832, #40833, #40837, #40849, #40852, #40853, #40865
- Failed merges:
@bors bors merged commit a29ae30 into rust-lang:master Mar 29, 2017
@topecongiro topecongiro mentioned this pull request Apr 4, 2017
Merged
@nikomatsakis nikomatsakis deleted the incr-comp-coerce-unsized-info branch April 14, 2017 10:12
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@eddyb eddyb eddyb left review comments

Assignees

@eddyb eddyb

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@nikomatsakis @eddyb @bors @arielb1