-
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
Refactorings in preparation for the removal of the leak check #50397
Refactorings in preparation for the removal of the leak check #50397
Conversation
Always using root environment for now.
This gives each type inference variable a notion of universe but doesn't do anything with it. We can always get the "current universe" from infer_ctxt. This relies on the property of type variables that they can never interact with siblings.
This is sort of confusing "side step". All it does is to change the representation of a skolemized region. but the source of that universe index is not the inference context, which is what we eventually want, but rather an internal counter in the region inference context. We'll patch that up later. But doing this now ought to help with confusing diffs later.
We'll need this in order to start tracking skolemizatoins here, and it's easier to update all the field accesses now rather than later.
src/librustc/infer/type_variable.rs
Outdated
(&TypeVariableValue::Unknown, &TypeVariableValue::Unknown) => Ok(*value1), | ||
(&TypeVariableValue::Unknown { universe: universe1 }, | ||
&TypeVariableValue::Unknown { universe: universe2 }) => { | ||
let universe = cmp::min(universe1, universe2); |
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.
Might be worth adding a comment here. Something like this:
If we have two unbound variables, ?T
and ?U
, that get unified, then whatever value they wind up taking (which must be the same value) must be nameable by both. Therefore, the resulting universe is the minimum of the two universes, because that is the one which contains the fewest names in scope.
@bors r+ Nice |
📌 Commit 68a1fdf has been approved by |
…matsakis Refactorings in preparation for the removal of the leak check This contains all of the commits from #48407 that I was able to pull out on their own. This has most of the refactoring/ground work to unblock other work, but without the behavior changes that still need a crater run and NLL changes. r? @nikomatsakis
☀️ Test successful - status-appveyor, status-travis |
This contains all of the commits from #48407 that I was able to pull out on their own. This has most of the refactoring/ground work to unblock other work, but without the behavior changes that still need a crater run and NLL changes.
r? @nikomatsakis