Cleanup LabelDefs #3365
Cleanup LabelDefs #3365
Conversation
| @@ -117,12 +114,12 @@ class LabelDefs extends MiniPhaseTransform { | |||
| } | |||
| } | |||
|
|
|||
| object collectLabelDefs extends TreeMap() { | |||
| private object collectLabelDefs extends TreeMap() { | |||
There was a problem hiding this comment.
Is it for performance reason that this is an object and we call clear() before using it, instead of making it a class, and creating a new instance when we use it?
There was a problem hiding this comment.
Not yet, I am exploring what can be simplified step by step. My intention is to fix a stack overflow. My previous attempt (#3362) modified the whole code in one go an something made the bootstrapped compiler buggy. I this PR I intend to change small details at a time and run the full test suite on each change. This is just step 0 of my changes.
nicolasstucki
force-pushed
the
cleanup-LabelDefs
branch
2 times, most recently
from
1499814 to
508d65f
Compare
| override def transform(tree: tpd.Tree)(implicit ctx: Context): tpd.Tree = { | ||
| tree match { | ||
| case t: Template => t |
There was a problem hiding this comment.
I don't think this case will ever be hit since we're after LambdaLift, same for the case _: Template below.
|
test performance please |
|
performance test scheduled: 1 job(s) in queue, 0 running. |
|
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/3365/ to see the changes. Benchmarks is based on merging with master (e1365d9) |
|
I think you can also remove all the |
Replaces one of the TreeMap by a TreeTraverser and implements transformation in the remaining TreeMap.
nicolasstucki
force-pushed
the
cleanup-LabelDefs
branch
from
1411fa8 to
7d850ad
Compare
|
Rebased |
| collectLabelDefs.clear | ||
| val newRhs = collectLabelDefs.transform(tree.rhs) | ||
| var labelDefs = collectLabelDefs.labelDefs | ||
| val labelDefs = collectLabelDefs(tree.rhs) | ||
|
|
||
| def putLabelDefsNearCallees = new TreeMap() { |
There was a problem hiding this comment.
I guess this can be a val
There was a problem hiding this comment.
Yes, but I added a small optimization to not create the transformer if labelDefs is empty. Therefore it will be kept as a def.
No description provided.