Topic/info transforms #72
Conversation
This makes naming uniform between trees and types, and also avoids the clash with transform.TreeTransformer. The idea is that transformers are parts of phases, and have logic that is phase-specific. In particular, a context is passed around when transforming a tree. Maps are simpler, they only have a T -> T apply method.
To bring in line with TreeTransformer terminology.
Many small and large changes. Added samplePhase to demonstrate functionality.
To test functioning, run the compiler with args
tests/pos/uncurry.scala -Ylog:sample,terminal
| @@ -22,7 +22,7 @@ class CreateCompanionObjectsTest extends DottyTest { | |||
|
|
|||
| import tpd._ | |||
|
|
|||
| @Test | |||
| //@Test | |||
There was a problem hiding this comment.
Why did you disable those tests?
There was a problem hiding this comment.
A mistake. The tests are now re-enabled again.
CreateCompanionObjectTests were re-enabled. They failed in a previous version but succeed again in the latest commit. Also uncurry.scala got renamed to curried.scala.
|
|
||
| def changeOwner(from: Symbol, to: Symbol)(implicit ctx: Context): ThisTree = | ||
| new TreeMapper(ownerMap = (sym => if (sym == from) to else sym)).apply(tree) | ||
| new TreeTypeMap(ownerMap = (sym => if (sym == from) to else sym)).apply(tree) |
There was a problem hiding this comment.
I love the fact that changeOwner is exposed on Tree, rather than on Symbol. We have some terrible bugs in scalac whereby speculative typechecking (e.g names/defaults) changes the owner of a symbol before bailing out, which breaks the immutability of Trees.
There was a problem hiding this comment.
How does this work? Does this create new symbols for deftrees whose owners need to be changed?
There was a problem hiding this comment.
|
Just a note for anybody wondering: the question of "what should be done if multiple TreeTransforms in same phase want to register a DenotTransformer?" remains uncovered here. |
| @@ -515,8 +517,8 @@ object Denotations { | |||
| } else { | |||
| // not found, cur points to highest existing variant | |||
| var startPid = cur.validFor.lastPhaseId + 1 | |||
| val transformers = ctx.transformersFor(cur) | |||
| val transformer = transformers.nextTransformer(startPid) | |||
| val transformer = ctx.phases(startPid - 1).asInstanceOf[DenotTransformer] | |||
There was a problem hiding this comment.
If first phase doesn't implement DenotTransformer this line throws exception
|
Result of discussion regarding relationships between DenotTransformers and TreeTransformers: every TreeTransform(miniphase) is promoted to a phase in terms of periods: it will have its own period and can register a DenotTransformer. |
|
I will work on implementing this, after factoring out xml |
|
Merged as part of #81 |
Backport "Fix provablyDisjoint handling enum constants with mixins" to 3.3 LTS
Reworked support for denotation transformers. See last commit message for how to verify usage.
I apologize for the somewhat chaotic commit contents. I tried to reorganize the commits but could not do it with reasonable effort.