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

[BETA] Universe leak check #58641

Closed

Conversation

nikomatsakis
Copy link
Contributor

Backport of #58592 and #58056 to beta.

In our type inference system, when we "generalize" a type T to become
a suitable value for a type variable V, we sometimes wind up creating
new inference variables. So, for example, if we are making V be some
subtype of `&'X u32`, then we might instantiate V with `&'Y u32`.
This generalized type is then related `&'Y u32 <: &'X u32`, resulting
in a region constriant `'Y: 'X`. Previously, however, we were making
these fresh variables like `'Y` in the "current universe", but they
should be created in the universe of V. Moreover, we sometimes cheat
in an invariant context and avoid creating fresh variables if we know
the result must be equal -- we can only do that when the universes
work out.
This set of diffs was produced by combing through
b68fad6 and seeing where the
`leak_check` used to be invoked and how.
One surprise: old-lub-glb-object.rs, may indicate a bug
This preserves the error you currently get on stable for the
old-lub-glb-object.rs test.
@rust-highfive

This comment has been minimized.

@rust-highfive
Copy link
Collaborator

⚠️ Warning ⚠️

  • Pull requests are usually filed against the master branch for this repo, but this one is against beta. Please double check that you specified the right target!

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 22, 2019
@Centril Centril changed the title Universe leak check beta [BETA] Universe leak check Feb 22, 2019
@Centril
Copy link
Contributor

Centril commented Feb 22, 2019

r? @Mark-Simulacrum

@pietroalbini
Copy link
Member

pietroalbini commented Feb 22, 2019

Can you rollup (cherry-picking) all the other beta-accepted PRs with this one? r=me when you do that

@Mark-Simulacrum
Copy link
Member

We'll probably roll this up with one or two other backports, so not yet approving this PR.

@nikomatsakis
Copy link
Contributor Author

@bors try

@bors
Copy link
Contributor

bors commented Feb 22, 2019

⌛ Trying commit c820008 with merge 799097d...

bors added a commit that referenced this pull request Feb 22, 2019
[BETA] Universe leak check

Backport of #58592 and #58056 to beta.
@nikomatsakis
Copy link
Contributor Author

Not sure if try is a good idea or not, but I know we had talked about crater runs, so might as well get started just in case?

@Mark-Simulacrum
Copy link
Member

I don't think we want/need a crater run here - the one on master should be sufficient.

@bors
Copy link
Contributor

bors commented Feb 22, 2019

☀️ Test successful - checks-travis
State: approved= try=True

@nikomatsakis
Copy link
Contributor Author

@Mark-Simulacrum ok, then this is ready to go, and I leave it to you to decide how/when to merge

@Centril
Copy link
Contributor

Centril commented Feb 22, 2019

Let's merge in #58649 and #58227 first and then backport everything in one PR.

@Mark-Simulacrum Mark-Simulacrum mentioned this pull request Feb 22, 2019
bors added a commit that referenced this pull request Feb 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants