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

Tweaks to ExplicitOuter and TreeTypeMap #1168

Merged
merged 5 commits into from
Mar 14, 2016

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Mar 11, 2016

Fixes #1131.

The test shows interesting behavior where we have to copy local classes using a TreeTypeMap. Since
this happens after erasure (in phase Mixin), we need at the same time be prepared to issue explicit outer paths. There were two problems encountered with this, which are fixed in this PR.

@odersky
Copy link
Contributor Author

odersky commented Mar 11, 2016

The PR is based on top of #1154 because it needed the better explicit outer diagnostic that is part of #1154. Only the last three commits are new.

@DarkDimius
Copy link
Member

Otherwise, LGTM for last 3 commits.

In a New we need to decide based on the prefix of the type of object created.
When recursing in a template body, need to update the
context's owner, so that `ref` can work correctly.
The test fails if either of the previous two commits is missing.
Two changes:

1. Replace changeOwer with changeOwnerAfter for code
that moves into the $initial methods in Mixin. This is needed because
otherwise subsequent transforms gets confused wrt new vs old owners.
`i1131.scala` exhibits the problem.

2. Drop `transformSym` changed the owner of tenplate-local symbols
to be the primary constructor. But that is done anyway with a "changeOwnerAfter"
in `intoConstr`. So it is redundant and actually gets in the way with
a `changeOwnerAfter` in `Mixin`. The faulty scenario is this:

  1. The SymTransformer of Constructor is run on a constructor-local definition.
     The owner of that definition is set to <init> after phase Constructors.

  2. The body of the definition is transformed in Mixin. The owner is set
     to the initializer method, but only for the interval between Mixin
     and Constructors.

Changing to changeOwner in Mixin avoided that problem by duplicating the symbol but
it runs into other problems.

Fortunately, the solution is much simpler than the status quo: Two changeOwnerAfter calls
and no SymTransformer.
@odersky
Copy link
Contributor Author

odersky commented Mar 12, 2016

@DarkDimius I rebased the PR to master to better isolate its commits. Can you have a look at dad9dfc
to complete the review?

@DarkDimius
Copy link
Member

LGTM

DarkDimius added a commit that referenced this pull request Mar 14, 2016
Tweaks to ExplicitOuter and TreeTypeMap
@DarkDimius DarkDimius merged commit 01269e9 into scala:master Mar 14, 2016
@allanrenucci allanrenucci deleted the fix-#1131 branch December 14, 2017 16:59
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.

ExplicitOuter crash with local class defined in val in trait if the local class extends a path-dependent class
3 participants