Skip to content

Transform/mixin #217

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

Merged
merged 34 commits into from
Nov 10, 2014
Merged

Transform/mixin #217

merged 34 commits into from
Nov 10, 2014

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Nov 9, 2014

Mixin now integrated, all tests pass.

Needs adaptations in getters/setters before it can be tested.
Trait getters are no longer needed.
When inheriting from Scala2 traits we sometimes encounter names with a space at the end,
denoting a local variable. Drop the space because our translation scheme has no room for it.
Now also handles all supercalls. Seems to do the right thing on pos/traits.scala.
But does not pass most tests because the sym transformer forces too many things.
Will print owners of symbols when displaying trees. Requires -Yprint-syms to be set also.
Motivation: Avoid needless forcing of symbols in scope. This is a problem when
cloneScope is called in TreeTransforms.
The problem manifests itself when changing the owner of a class field. Being a ValDef,
this is classified as a weak owner. But it's parent (the class) should not be owner-changed.
No more trait_setters are called.
Trait initializers do not have outer params; need to avoid passing
outer args to them.

This problem did not manifest itself before because no trait constructor calls
were generated before erasure.
After Mixin, super calls can appear in statemenr sequences. They need to be
typechecked and transformed using a special context.
Array types are no longer used after erasure, so the test was redundant.
Previous version could fail where the thisType widened to an explicit self type.
Since we demand that after erasure all TermRefs are SymDenotations we need to
assure this when computing the denotations of term refs in new phases.
Otherwise we get binding races, e.g. for pos/t2133.scala after mixin
because we create same-named symbols in subclasses, and type refs now
can refer to the subclass symbol where previously they refereed to the
superclass symbol. To avoid a data race the new term ref will have a
fixed symbol.
Need to have Method flag set
This is reasonable because getters are only generated late in the
compilation pipeline. No need to hide a getter as a val in error messages.
1. Erase unit results in getters to BoxedUnit.
2. Erase => Unit to ()Unit; was ()BoxedUnit
3. Make sure ValDefs have same type in tpt as in symbol, analogous to DefDefs.
Separate test instead of inline.
Now survives the case where a field is written x_=, dissumulating
a setter.
Split into two phases, ResolveSuper before Erasure and Mixin after. Likewise GettersSetters
is split into Getters and Memoize.

All tests pass, except two tests fail when compiled twice. Will investigate next why.
Needs a weakening of an assertion in assertErased
Need to be treated like TermRefs.
Ig an implicit method has a result type which does not match the expected
type, the new case fires. We avoid to diagnose missing arguments, because
that's really confusing for an implicit method.
The fix to Types in 9bb3551 (Retract of optimization in computeDenot)
allowed the two tests to compile "twice" again.
Changes owners after a phase without copying the tree. This should be more
suitable for the changeOwner operations used in the tree transforms so far,
which are linear, i.e. no tree duplication is needed.
@odersky
Copy link
Contributor Author

odersky commented Nov 9, 2014

Rebased on latest master.

Previously, there was a switch in SymDenotation#termRef to create a WithFixedSym ref
when after erasure. This was done to fix a broken test, but it's clearly incomplete.
The scheme is now replaced by one which always creates WithFixedSym Termrefs after
erasure.
DarkDimius added a commit that referenced this pull request Nov 10, 2014
@DarkDimius DarkDimius merged commit 42e51d5 into scala:master Nov 10, 2014
@odersky odersky deleted the transform/mixin branch November 13, 2014 10:07
WojciechMazur pushed a commit to WojciechMazur/dotty that referenced this pull request May 8, 2025
Backport "Backport from Metals" to 3.3 LTS
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.

2 participants