-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Remove ReScope #72362
Remove ReScope #72362
Conversation
46702eb
to
c79a9cc
Compare
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
bf9afb2
to
eadba03
Compare
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.
Looks great! Left some questions and nits. Wow, it's nice to see the old code get deleted. I guess we kept enough of the scope tree to still handle the generators code that uses it.
@bors r+ |
📌 Commit c8033e7146a43b7ebe1515f643dec726aa45ea2b has been approved by |
☔ The latest upstream changes (presumably #72433) made this pull request unmergeable. Please resolve the merge conflicts. |
c8033e7
to
d5219dc
Compare
d5219dc
to
9754b3f
Compare
@bors r=nikomatsakis |
📌 Commit 9754b3f has been approved by |
⌛ Testing commit 9754b3f with merge 1dccabac15f9026a3afec072feea9959cea48fa1... |
@bors retry yield |
Type checking still has to infer the values of regions. Before this pr this included ensuring that regions inferred in function bodies outlived various scopes. This pr is removing the logic for generating those predicates, because they can't be expressed any more. |
…omatsakis Remove ReScope `ReScope` is unnecessary now that AST borrowck is gone and we're erasing the results of region inference in function bodies. This removes about as much of the old regionck code as possible without having to enable NLL fully. cc rust-lang#68261 r? @nikomatsakis
⌛ Testing commit 9754b3f with merge e09c65e40789dc68465dfd855d4be570aa0e2163... |
@bors retry yield |
☀️ Test successful - checks-azure |
@matthewjasper This PR led to substantial perf wins on check builds for real-world crates. Nice work! |
Good news! This was a significant perf win. Instruction counts dropped by up to 7.7%. |
Add some regression tests Closes rust-lang#68532 Closes rust-lang#70121 Closes rust-lang#71042 CC rust-lang#56445 r? @matthewjasper since they (except for rust-lang#71042) are related to rust-lang#72362.
Add some regression tests Closes rust-lang#68532 Closes rust-lang#70121 Closes rust-lang#71042 CC rust-lang#56445 r? @matthewjasper since they (except for rust-lang#71042) are related to rust-lang#72362.
@@ -5,7 +5,6 @@ | |||
|
|||
use rustc_data_structures::transitive_relation::TransitiveRelation; | |||
use rustc_hir::def_id::DefId; | |||
use rustc_middle::middle::region; | |||
use rustc_middle::ty::{self, Lift, Region, TyCtxt}; | |||
|
|||
/// Combines a `region::ScopeTree` (which governs relationships between |
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.
Missed reference to region::ScopeTree
.
(don't mind me, I came across this PR from perf results, enjoying the diff)
ReScope
is unnecessary now that AST borrowck is gone and we're erasing the results of region inference in function bodies. This removes about as much of the old regionck code as possible without having to enable NLL fully.cc #68261
r? @nikomatsakis