-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Transform/mixin #217
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
dc276eb
to
f2e3489
Compare
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.
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Mixin now integrated, all tests pass.