Transform/getters setters #186
Conversation
…ories. We better make this configurable. Because sometimes we want to compile only the files in the immediate directory.
Use ref and appliedTo instead.
The idea to traverse with currently enclosing methid is also used in LambdaLift and could be used elsewhere.
…lled. Instead of requiring to be called a given phase, change the context if that is not the case.
Lifting an application `f(arg1).f(args)` requires lifting of the whole prefix `f(arg1)`, because `f` might have a side effect.
Advantage: Can rename typed as well as untyped trees.
Previously, we determined the default getters solely from the method TermRef type. This is wrong if the method is prefix is not a path -- the prefix of the term ref will be a TypeRef and the getter selection will be a SelectFromType tree, which makes no sense.
It changes meaning drastically so should always be visible.
Some transformations encounter applications where new arguments ahve to be supplied. The method type already accounts for these argument but the Application node passed into TreeTransform#transformApply is constructed with a cpy.Apply operation which does a type assignment. That type assignment fails with a parameter mismatch unless relaxedTyping is on.
Environment parameters do not count in th eresult type.
The current formulation of lambda lift is easier to do if that's the case.
Need to drop all non-class type declarations.
Params are already added by Desugar. No special treatment needed here. Besides primaryConstructor.typeParams is always empty, because term symbols do not have type parameters. The fix turns t2660.scala into an error. I believe the error is correct, hence the test was moved with a comment to neg.
Primary constructor was picking last constructor instead of first one. This is now fixed. Also, added paramAccessors utility method.
Needed to fix a problem in CapturedVars to make this work.
Like the corresponding parameters, these also need to be rewritten to function types.
1) Type parameters are now copied to accessors 2) Constructors also work for traits 2) makes it possible do to mixin after constructors.
TypeVars can appear as keys in baseType caches. The problem is that their base types depend on their instantiation, which is not always know yet when the test is performed. So results of baseType on type variables should never be cached. Todo: check whether there are any other caching problems involving typevars.
Typevars can be parts of larger types or underlying types of other types, which renders these other types uncacheable. The new logic takes account of that.
Private fields that are accessed only from the constructor, and are accessed only after they are properly initialized are now moved into the constructor. This avoids creating a redundant objetc field. Good example: gcd in Rationals (see constrs.scala).
TypeVar is a TypeProxy.
Allow to skip labels.
1) TermRefs are now erased to their widened underlying type. The only exception are top-level term refs (e.g. the types of Ident or Select nodes), which are treated as before. Those top-level refs are treated by erasedRef instead of erasure. 2) We make sure that erasure methods are not run after phase erasure. Reason: After erasure underlying types change, which affects itself the result of erasure.
... and add a gettersSettersPhase to Context.
1) Eliminated a case in the comparison that did not work and was apparently not needed anyway. 2) Augmented another case to also work when paths go through getters.
Method isSelfSym decides whether a symbol is the symbol of a self definition in a class. Also, make all self symbols private[this]
The previous test, `symbol != null` crashed for trees that did not have an assigned type.
1) Avoids unneccessary second adapt. 2) Avoids error when used in TreeChecking when we try to compare types in previous tree and new tree - if we used `typed`, the previous tree would not have a type!
Similar to Erasure, we do not check conformance wrt <:< to a FunProto. Nothing is lost, because we typecheck the arguments and result anyway. And this led to failing tests after GettersSetters was added.
Was lost in merge chaos between pattern macther and rest
And bring back two tests for typing of patterns
1) strip TypeVars where necessary before widening 2) allow the combination of widening and dealiasing, in any order.
... because Any is not an erasedType (this is now checked)
Avoids a stale symbol error when compiling all files in tools together.
Previously caused an illegal backwards timetravel in bringForward when compiling all files in tools together twice.
Tests now always include erasure (before quite a few tests failed when erasure was enableed). By contrast lazy vals creates problems with erasure, disabled for now. Some other small polishings on integration of pattern matcher with rest of dotc. Deep recompilation of tools still fails, currently disabled.
|
The branch with patmat was passing all has errasure enabled and I was fixing quite some Ycheck erros errors after erasure. It took me around 4 hours to rebase your Do you still have branch |
|
@odersky it seems that most of problems you have faced during rebasing come from this
I've rebased the getters&setters branch yesterday. It looks like in some point your tools have performed auto-merge(4f4bf41) of your local non-rebased branch with the one I've rebased, breaking the history with duplicated commits. Effectively merging branch with itself with different commit-SHAs would indeed lead to a lot of conflicts. I've followed history without commit 4f4bf41 #187 which lead to a lot cleaner history and only several conflicts, as most of conflicts were already resolved in c0da421 |
|
To see the errors, you need to run the test suite on the pattern matcher commit with the latest "test" setup, i.e. checks after literalize (or gettersSetters) and constructors. Unfortunately, I only have a previous staged version of transform/addSetters which is pretty out of date wrt the internal one. The internal one was the result of the rebasing, but that was dead on arrival. So I think we should just move on. As long as everything works we are good. Let's try to avoid divergences of this scale in the future. |
|
Superseded by #189. |
* Scala 2.13.16 (was .15) * Silence deprecations about AnyRefMap in scalajs-ir. * Upgrade to mtags 1.4.2, which supports Scala 2.13.16. * Silence deprecations about AnyRefMap in a test --------- Co-authored-by: Seth Tisue <seth@tisue.net> Co-authored-by: Sébastien Doeraene <sjrdoeraene@gmail.com>
This commit unifies the very much diverging branches of pattern matcher and other transformations, i.e. capturedVars/constructors/gettersSetters by rebasing the essence of gettersSetters on pattern matcher. It turned out that pattern matcher had quite a few tests turned off before erasure. That has been fixed. The new branch has the same test coverage as the previous transformation branches.
By extending test coverage, two breakages were noticed.
LazyVals causes again problems, which are hard to track down. Since it will change place in the pipeline anyway I disabled it again.
deep tools compile also caused problems when extended beyond erasure. I am investigating those.
In retrospect it was a LOT of work to achieve confluence of the two branches. I spent overall 14 hours on it. Doing the following steps:
Tried to rebase getters setters onto pattern matcher. Lots of errors, impossible to track down. Only later it dawned on me that a good part of these were due to different test coverage in the two branches.
Merged pattern matcher into getters setters. This also did not work but then I could take the merge and commit small bits of it individually that worked until I had a working system. Almost declared victory but when I was done I found that the pattern matcher had already been merged. So I fell back to
Cherry-picked the commits that were not yet integrated by pattern matcher in some way or other on top of pattern matcher. The result was not working, of course. Then, merged the result of 2) into it and committed in little bits until everything worked again.
The commit sequence you are seeing is the result of 3).
In retrospect I think it would have been easier to rebase pattern matcher over getters setters. Lesson for the future: When in doubt pick the version with the higher test coverage as a basis.
I suggest we take this branch as the basis of all future work. It has diverged so much from master that any commits against master would have to be cherry-picked. Rebasing is useless.