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

Clean up hir::lowering #33532

Merged
merged 7 commits into from
May 14, 2016
Merged

Conversation

jseyfried
Copy link
Contributor

@jseyfried jseyfried commented May 10, 2016

Clean up hir::lowering:

  • give lowering functions mutable access to the lowering context
  • refactor the lower_* functions and other functions that take a lowering context into methods
  • simplify the API that hir::lowering exposes to driver
  • other miscellaneous cleanups

r? @nrc

@jseyfried jseyfried force-pushed the mutable_lowering_context branch from 8899264 to 33978b0 Compare May 10, 2016 07:56
@nrc
Copy link
Member

nrc commented May 10, 2016

@bors: r+

@bors
Copy link
Contributor

bors commented May 10, 2016

📌 Commit 33978b0 has been approved by nrc

Manishearth added a commit to Manishearth/rust that referenced this pull request May 12, 2016
… r=nrc

Clean up `hir::lowering`

Clean up `hir::lowering`:
 - give lowering functions mutable access to the lowering context
 - refactor the `lower_*` functions and other functions that take a lowering context into methods
 - simplify the API that `hir::lowering` exposes to `driver`
 - other miscellaneous cleanups

r? @nrc
eddyb added a commit to eddyb/rust that referenced this pull request May 12, 2016
… r=nrc

Clean up `hir::lowering`

Clean up `hir::lowering`:
 - give lowering functions mutable access to the lowering context
 - refactor the `lower_*` functions and other functions that take a lowering context into methods
 - simplify the API that `hir::lowering` exposes to `driver`
 - other miscellaneous cleanups

r? @nrc
bors added a commit that referenced this pull request May 12, 2016
eddyb added a commit to eddyb/rust that referenced this pull request May 13, 2016
… r=nrc

Clean up `hir::lowering`

Clean up `hir::lowering`:
 - give lowering functions mutable access to the lowering context
 - refactor the `lower_*` functions and other functions that take a lowering context into methods
 - simplify the API that `hir::lowering` exposes to `driver`
 - other miscellaneous cleanups

r? @nrc
@bors
Copy link
Contributor

bors commented May 14, 2016

⌛ Testing commit 33978b0 with merge dee865a...

bors added a commit that referenced this pull request May 14, 2016
Clean up `hir::lowering`

Clean up `hir::lowering`:
 - give lowering functions mutable access to the lowering context
 - refactor the `lower_*` functions and other functions that take a lowering context into methods
 - simplify the API that `hir::lowering` exposes to `driver`
 - other miscellaneous cleanups

r? @nrc
@joelself
Copy link

joelself commented May 24, 2016

What gives? I checked the merge request and apparently it has been merged into master. But the current file doesn't show the changes. I even cloned the repo and checked the file myself and it is unchanged. Even the history of the file doesn't show any changes 10 days ago which is when the merge supposedly happened.

I cloned jseyfried's rust fork, checked out the mutable_lowering_context branch then did a pull against rust-lang:master and ended up with the same old file. The git log then shows this pull request's changes several commits back, but it looks like subsequent commits undid them. Was that intentional? Especially given that the change to lowering.rs was so big?

Should I base my work on the refactored lowering.hir or the current lowering.hir in the master branch of the rust-lang repo?

Edit: After looking at it some more it seems it was refactored again and it just happens to make the code I want to change look a lot like the original code from before the merge. So I guess never mind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants