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

Replace unsafety in middle::ty with lifetimes. #18483

Merged
merged 9 commits into from Nov 19, 2014

Conversation

Projects
None yet
6 participants
@eddyb
Copy link
Member

eddyb commented Oct 31, 2014

After more than a month of sitting on this patch, rebasing and tracking down some nasty bugs (there's might be still one out there, but it only manifested in middle::trans::reflect which is now gone), I'd like to merge it as it is.

This changeset makes middle::ty safe, linking the lifetime of a type to the type context it was created in.
It's a prerequisite for introducing function-local type contexts to localize types with inference variables, in order to (potentially) free hundreds of MBs from rustc's memory usage peak.

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Oct 31, 2014

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!

@eddyb eddyb force-pushed the eddyb:safe-ty branch from 21aa821 to 0ee4d8b Oct 31, 2014

@eddyb eddyb assigned eddyb and nikomatsakis and unassigned eddyb Oct 31, 2014

@eddyb eddyb force-pushed the eddyb:safe-ty branch 3 times, most recently from 71a8e82 to 315aa8d Oct 31, 2014

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Nov 3, 2014

This is cool stuff

@eddyb eddyb force-pushed the eddyb:safe-ty branch 2 times, most recently from 0448038 to 3a00e42 Nov 4, 2014

@eddyb

This comment has been minimized.

Copy link
Member

eddyb commented Nov 5, 2014

@eddyb eddyb force-pushed the eddyb:safe-ty branch 2 times, most recently from f58aad6 to d12acd8 Nov 6, 2014

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Nov 6, 2014

r+

@eddyb from what I can tell, this PR is a work of art. Thanks for doing such a good job separating out the commits like you did, that really makes this easier to read.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Nov 6, 2014

@eddyb btw I put the r+ on the PR, not the commit, so you could address the question regarding the comparison.

@eddyb eddyb force-pushed the eddyb:safe-ty branch from d12acd8 to d29c23e Nov 7, 2014

bors added a commit that referenced this pull request Nov 7, 2014

auto merge of #18483 : eddyb/rust/safe-ty, r=nikomatsakis
After more than a month of sitting on this patch, rebasing and tracking down some nasty bugs (there's might be still one out there, but it only manifested in `middle::trans::reflect` which is now gone), I'd like to merge it as it is.

This changeset makes middle::ty safe, linking the lifetime of a type to the type context it was created in.
It's a prerequisite for introducing function-local type contexts to localize types with inference variables, in order to (potentially) free hundreds of MBs from rustc's memory usage peak.

@eddyb eddyb force-pushed the eddyb:safe-ty branch from d29c23e to bef61b0 Nov 9, 2014

bors added a commit that referenced this pull request Nov 9, 2014

auto merge of #18483 : eddyb/rust/safe-ty, r=nikomatsakis
After more than a month of sitting on this patch, rebasing and tracking down some nasty bugs (there's might be still one out there, but it only manifested in `middle::trans::reflect` which is now gone), I'd like to merge it as it is.

This changeset makes middle::ty safe, linking the lifetime of a type to the type context it was created in.
It's a prerequisite for introducing function-local type contexts to localize types with inference variables, in order to (potentially) free hundreds of MBs from rustc's memory usage peak.
@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Nov 12, 2014

@eddyb #18784 I guess? Frustrating.

@eddyb eddyb force-pushed the eddyb:safe-ty branch 4 times, most recently from a0d0b39 to 789797b Nov 14, 2014

bors added a commit that referenced this pull request Nov 19, 2014

auto merge of #18483 : eddyb/rust/safe-ty, r=nikomatsakis
After more than a month of sitting on this patch, rebasing and tracking down some nasty bugs (there's might be still one out there, but it only manifested in `middle::trans::reflect` which is now gone), I'd like to merge it as it is.

This changeset makes middle::ty safe, linking the lifetime of a type to the type context it was created in.
It's a prerequisite for introducing function-local type contexts to localize types with inference variables, in order to (potentially) free hundreds of MBs from rustc's memory usage peak.

@eddyb eddyb force-pushed the eddyb:safe-ty branch from 789797b to bf0766a Nov 19, 2014

@eddyb

This comment has been minimized.

Copy link
Owner

eddyb commented on bf0766a Nov 19, 2014

r=nikomatsakis p=5

This comment has been minimized.

Copy link

alexcrichton replied Nov 19, 2014

@bors: retry

@bors

This comment has been minimized.

Copy link
Contributor

bors commented on bf0766a Nov 19, 2014

saw approval from nikomatsakis
at eddyb@bf0766a

This comment has been minimized.

Copy link
Contributor

bors replied Nov 19, 2014

merging eddyb/rust/safe-ty = bf0766a into auto

This comment has been minimized.

Copy link
Contributor

bors replied Nov 19, 2014

eddyb/rust/safe-ty = bf0766a merged ok, testing candidate = f6278e2

This comment has been minimized.

Copy link
Contributor

bors replied Nov 19, 2014

This comment has been minimized.

Copy link
Contributor

bors replied Nov 19, 2014

saw approval from nikomatsakis
at eddyb@bf0766a

This comment has been minimized.

Copy link
Contributor

bors replied Nov 19, 2014

merging eddyb/rust/safe-ty = bf0766a into auto

This comment has been minimized.

Copy link
Contributor

bors replied Nov 19, 2014

eddyb/rust/safe-ty = bf0766a merged ok, testing candidate = cf7df1e

This comment has been minimized.

Copy link
Contributor

bors replied Nov 19, 2014

fast-forwarding master to auto = cf7df1e

bors added a commit that referenced this pull request Nov 19, 2014

auto merge of #18483 : eddyb/rust/safe-ty, r=nikomatsakis
After more than a month of sitting on this patch, rebasing and tracking down some nasty bugs (there's might be still one out there, but it only manifested in `middle::trans::reflect` which is now gone), I'd like to merge it as it is.

This changeset makes middle::ty safe, linking the lifetime of a type to the type context it was created in.
It's a prerequisite for introducing function-local type contexts to localize types with inference variables, in order to (potentially) free hundreds of MBs from rustc's memory usage peak.

bors added a commit that referenced this pull request Nov 19, 2014

auto merge of #18483 : eddyb/rust/safe-ty, r=nikomatsakis
After more than a month of sitting on this patch, rebasing and tracking down some nasty bugs (there's might be still one out there, but it only manifested in `middle::trans::reflect` which is now gone), I'd like to merge it as it is.

This changeset makes middle::ty safe, linking the lifetime of a type to the type context it was created in.
It's a prerequisite for introducing function-local type contexts to localize types with inference variables, in order to (potentially) free hundreds of MBs from rustc's memory usage peak.

@bors bors closed this Nov 19, 2014

@bors bors merged commit bf0766a into rust-lang:master Nov 19, 2014

2 checks passed

continuous-integration/travis-ci The Travis CI build passed
Details
default all tests passed
@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Nov 19, 2014

Wow, congratulations!

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Nov 19, 2014

/me goes to rebase everything

@eddyb eddyb deleted the eddyb:safe-ty branch Nov 19, 2014

@ghost

This comment has been minimized.

Copy link

ghost commented Nov 19, 2014

Amazing. Congratulations, @eddyb. :)

@Gankro

This comment has been minimized.

Copy link
Contributor

Gankro commented Nov 19, 2014

🎉 🎈 🎂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment