Skip to content

[BETA] Universe leak check#58641

Closed
nikomatsakis wants to merge 8 commits into
rust-lang:betafrom
nikomatsakis:universe-leak-check-beta
Closed

[BETA] Universe leak check#58641
nikomatsakis wants to merge 8 commits into
rust-lang:betafrom
nikomatsakis:universe-leak-check-beta

Conversation

@nikomatsakis

Copy link
Copy Markdown
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
Copy Markdown
Contributor

⚠️ 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

Centril commented Feb 22, 2019

Copy link
Copy Markdown
Contributor

r? @Mark-Simulacrum

@emilyalbini

emilyalbini commented Feb 22, 2019

Copy link
Copy Markdown
Member

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

@Mark-Simulacrum

Copy link
Copy Markdown
Member

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

@nikomatsakis

Copy link
Copy Markdown
Contributor Author

@bors try

@bors

bors commented Feb 22, 2019

Copy link
Copy Markdown
Collaborator

⌛ 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
Copy Markdown
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
Copy Markdown
Member

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

@bors

bors commented Feb 22, 2019

Copy link
Copy Markdown
Collaborator

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

@nikomatsakis

Copy link
Copy Markdown
Contributor Author

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

@Centril

Centril commented Feb 22, 2019

Copy link
Copy Markdown
Contributor

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