-
Notifications
You must be signed in to change notification settings - Fork 13.4k
On-demandify the typechecking of item bodies #40540
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking good, one minor quibble
src/librustc_typeck/check/mod.rs
Outdated
} | ||
} | ||
|
||
fn typeck_item_bodies<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, _: CrateNum) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably a good idea to have assert that the CrateNum
is indeed LOCAL_CRATE
6b59aeb
to
ee584ac
Compare
@bors r+ |
📌 Commit ee584ac has been approved by |
☔ The latest upstream changes (presumably #39628) made this pull request unmergeable. Please resolve the merge conflicts. |
src/librustc_typeck/check/mod.rs
Outdated
tcx.visit_all_bodies_in_krate(|body_owner_def_id, _body_id| { | ||
tcx.item_tables(body_owner_def_id); | ||
}); | ||
ty::queries::typeck_item_bodies::get(tcx, DUMMY_SP, LOCAL_CRATE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the separate DepNode::TypeckBodiesKrate
task even need to exist?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the moment, yes, because otherwise it causes and ICE with "read/write of Some(TypeckBodiesKrate)
but no current task".
Should I try and fix this here somehow or just add a FIXME
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a FIXME is a good idea.
ee584ac
to
eacd2da
Compare
eacd2da
to
8c92044
Compare
@nikomatsakis Added the |
@bors r+ |
📌 Commit 8c92044 has been approved by |
…ikomatsakis On-demandify the typechecking of item bodies r? @nikomatsakis
🔒 Merge conflict |
☔ The latest upstream changes (presumably #40826) made this pull request unmergeable. Please resolve the merge conflicts. |
8c92044
to
61cd190
Compare
@bors r+ |
📌 Commit 61cd190 has been approved by |
61cd190
to
8a27a51
Compare
@bors r+ |
📌 Commit 8a27a51 has been approved by |
☔ The latest upstream changes (presumably #40867) made this pull request unmergeable. Please resolve the merge conflicts. |
8a27a51
to
3b10b4e
Compare
@bors r+ |
📌 Commit 3b10b4e has been approved by |
On-demandify the typechecking of item bodies r? @nikomatsakis
☀️ Test successful - status-appveyor, status-travis |
r? @nikomatsakis