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

Assortiment of cleanups and comments around the backend #3826

Merged
merged 7 commits into from
Jul 25, 2014

Conversation

lrytz
Copy link
Member

@lrytz lrytz commented Jun 22, 2014

No description provided.

@lrytz lrytz changed the title Assortment of cleanups and comments around the backend Assortiment of cleanups and comments around the backend Jun 22, 2014
@lrytz lrytz added the tested label Jun 22, 2014
@lrytz
Copy link
Member Author

lrytz commented Jun 23, 2014

Tests went fine, review by @retronym

@lrytz
Copy link
Member Author

lrytz commented Jul 7, 2014

Thanks @retronym for insisting on a before-after comparison: I found a serious issue and have to give up on introducing ownerAtCurrentPhase. The problem is that the current phase in saveOriginalOwner can be anything, not necessarily the phase in which the owner was/is changed.

For example, the tree transformation in the lambdalift phase, which lifts definitions (and modifies their symbol's owner), is executed afterOwnPhase. Also, an owner might be modified in an info transformer, which are executed lazily.

I'll go back to just save the originalOwner for now. It's really hard to touch/improve incrementally around flags and owners that depend change depending on the phase :( I'm open to discuss things, but I think my time budget for that has run out long ago..

@lrytz lrytz removed the tested label Jul 8, 2014
@lrytz
Copy link
Member Author

lrytz commented Jul 8, 2014

-Ybackend:GenBCode build ok

@lrytz
Copy link
Member Author

lrytz commented Jul 8, 2014

removed ownerAtCurrentPhase again. review by @retronym, @gkossakowski

@gkossakowski
Copy link
Contributor

LGTM.

In the future please emphasize the why over what when writing commit messages. A year from now it's going to be a lot easier to work out what has been changed in the diff than why it was done. I'd like to emphasize this point in our policy: https://github.com/scala/scala/wiki/Pull-Request-Policy

@lrytz lrytz added the reviewed label Jul 14, 2014
@lrytz lrytz modified the milestones: 2.11.3, 2.11.2 Jul 15, 2014
gkossakowski added a commit that referenced this pull request Jul 25, 2014
Assortiment of cleanups and comments around the backend
@gkossakowski gkossakowski merged commit e89fe92 into scala:2.11.x Jul 25, 2014
@lrytz lrytz deleted the opt/refactorTracked branch August 12, 2014 13:18
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.

3 participants