From 910481a0f4fe671f3f4d8965eac61870596970e1 Mon Sep 17 00:00:00 2001 From: Dmitry Petrashko Date: Sat, 20 Feb 2016 15:31:11 +0100 Subject: [PATCH 1/3] Do not create companions that will be dropped later. Fix blocker bug reported in #1114 I dislike this fix as now phase needs to know in advance if it will ever need a companion for the class. On the bright side, this change makes it clear which phases need companions --- src/dotty/tools/dotc/Compiler.scala | 2 +- src/dotty/tools/dotc/core/Phases.scala | 5 +++++ .../tools/dotc/transform/ExtensionMethods.scala | 8 ++++++-- .../tools/dotc/transform/FirstTransform.scala | 14 ++++++++++++-- src/dotty/tools/dotc/transform/LazyVals.scala | 8 +++++++- 5 files changed, 31 insertions(+), 6 deletions(-) diff --git a/src/dotty/tools/dotc/Compiler.scala b/src/dotty/tools/dotc/Compiler.scala index 99b7bd880fff..d526903b8015 100644 --- a/src/dotty/tools/dotc/Compiler.scala +++ b/src/dotty/tools/dotc/Compiler.scala @@ -78,7 +78,7 @@ class Compiler { List(new LambdaLift, // in this mini-phase block scopes are incorrect. No phases that rely on scopes should be here new ElimStaticThis, new Flatten, - new DropEmptyCompanions, + // new DropEmptyCompanions, new RestoreScopes), List(new ExpandPrivate, new CollectEntryPoints, diff --git a/src/dotty/tools/dotc/core/Phases.scala b/src/dotty/tools/dotc/core/Phases.scala index 83ac64d536e9..b60f437d5282 100644 --- a/src/dotty/tools/dotc/core/Phases.scala +++ b/src/dotty/tools/dotc/core/Phases.scala @@ -4,6 +4,7 @@ package core import Periods._ import Contexts._ import dotty.tools.backend.jvm.{LabelDefs, GenBCode} +import dotty.tools.dotc.core.Symbols.ClassSymbol import util.DotClass import DenotTransformers._ import Denotations._ @@ -347,6 +348,10 @@ object Phases { override def toString = phaseName } + trait NeedsCompanions { + def isCompanionNeeded(cls: ClassSymbol)(implicit ctx: Context): Boolean + } + /** Replace all instances of `oldPhaseClass` in `current` phases * by the result of `newPhases` applied to the old phase. */ diff --git a/src/dotty/tools/dotc/transform/ExtensionMethods.scala b/src/dotty/tools/dotc/transform/ExtensionMethods.scala index 90e105ee7f17..c5ab49c9cd6d 100644 --- a/src/dotty/tools/dotc/transform/ExtensionMethods.scala +++ b/src/dotty/tools/dotc/transform/ExtensionMethods.scala @@ -11,7 +11,7 @@ import dotty.tools.dotc.ast.{Trees, tpd} import scala.collection.{ mutable, immutable } import mutable.ListBuffer import core._ -import Phases.Phase +import dotty.tools.dotc.core.Phases.{NeedsCompanions, Phase} import Types._, Contexts._, Constants._, Names._, NameOps._, Flags._, DenotTransformers._ import SymDenotations._, Symbols._, StdNames._, Annotations._, Trees._, Scopes._, Denotations._ import TypeErasure.{ valueErasure, ErasedValueType } @@ -33,7 +33,7 @@ import SymUtils._ * This is different from the implementation of value classes in Scala 2 * (see SIP-15) which uses `asInstanceOf` which does not typecheck. */ -class ExtensionMethods extends MiniPhaseTransform with DenotTransformer with FullParameterization { thisTransformer => +class ExtensionMethods extends MiniPhaseTransform with DenotTransformer with FullParameterization with NeedsCompanions { thisTransformer => import tpd._ import ExtensionMethods._ @@ -45,6 +45,10 @@ class ExtensionMethods extends MiniPhaseTransform with DenotTransformer with Ful override def runsAfterGroupsOf = Set(classOf[FirstTransform]) // need companion objects to exist + def isCompanionNeeded(cls: ClassSymbol)(implicit ctx: Context): Boolean = { + isDerivedValueClass(cls) + } + override def transform(ref: SingleDenotation)(implicit ctx: Context): SingleDenotation = ref match { case moduleClassSym: ClassDenotation if moduleClassSym is ModuleClass => moduleClassSym.linkedClass match { diff --git a/src/dotty/tools/dotc/transform/FirstTransform.scala b/src/dotty/tools/dotc/transform/FirstTransform.scala index 3b629f9b6690..5d42d4c1651b 100644 --- a/src/dotty/tools/dotc/transform/FirstTransform.scala +++ b/src/dotty/tools/dotc/transform/FirstTransform.scala @@ -3,7 +3,9 @@ package transform import core._ import Names._ -import dotty.tools.dotc.transform.TreeTransforms.{AnnotationTransformer, TransformerInfo, MiniPhaseTransform, TreeTransformer} +import dotty.tools.dotc.ast.tpd +import dotty.tools.dotc.core.Phases.NeedsCompanions +import dotty.tools.dotc.transform.TreeTransforms._ import ast.Trees._ import Flags._ import Types._ @@ -32,6 +34,14 @@ class FirstTransform extends MiniPhaseTransform with IdentityDenotTransformer wi override def phaseName = "firstTransform" + private var needsCompanionPredicate: ClassSymbol => Boolean = null + + override def prepareForUnit(tree: tpd.Tree)(implicit ctx: Context): TreeTransform = { + needsCompanionPredicate = ctx.phasePlan.flatMap(_.filter(x => x.isInstanceOf[NeedsCompanions])). + foldLeft((x: ClassSymbol) => false)((pred, phase) => x => pred(x) || phase.asInstanceOf[NeedsCompanions].isCompanionNeeded(x)) + this + } + def transformInfo(tp: Type, sym: Symbol)(implicit ctx: Context): Type = tp override def checkPostCondition(tree: Tree)(implicit ctx: Context): Unit = tree match { @@ -80,7 +90,7 @@ class FirstTransform extends MiniPhaseTransform with IdentityDenotTransformer wi } def addMissingCompanions(stats: List[Tree]): List[Tree] = stats map { - case stat: TypeDef if singleClassDefs contains stat.name => + case stat: TypeDef if (singleClassDefs contains stat.name) && needsCompanionPredicate(stat.symbol.asClass) => val objName = stat.name.toTermName val nameClash = stats.exists { case other: MemberDef => diff --git a/src/dotty/tools/dotc/transform/LazyVals.scala b/src/dotty/tools/dotc/transform/LazyVals.scala index 4a8ff83caa4c..2aece06638fc 100644 --- a/src/dotty/tools/dotc/transform/LazyVals.scala +++ b/src/dotty/tools/dotc/transform/LazyVals.scala @@ -1,6 +1,7 @@ package dotty.tools.dotc package transform +import dotty.tools.dotc.core.Phases.NeedsCompanions import dotty.tools.dotc.typer.Mode import scala.collection.mutable @@ -23,7 +24,7 @@ import dotty.tools.dotc.core.SymDenotations.SymDenotation import dotty.tools.dotc.core.DenotTransformers.{SymTransformer, IdentityDenotTransformer, DenotTransformer} import Erasure.Boxing.adaptToType -class LazyVals extends MiniPhaseTransform with IdentityDenotTransformer { +class LazyVals extends MiniPhaseTransform with IdentityDenotTransformer with NeedsCompanions { import LazyVals._ import tpd._ @@ -46,6 +47,11 @@ class LazyVals extends MiniPhaseTransform with IdentityDenotTransformer { * before this phase starts processing same tree */ override def runsAfter = Set(classOf[Mixin]) + def isCompanionNeeded(cls: ClassSymbol)(implicit ctx: Context): Boolean = { + def hasLazyVal(x: ClassSymbol) = x.classInfo.membersBasedOnFlags(Flags.Lazy, excludedFlags = Flags.EmptyFlags).nonEmpty + hasLazyVal(cls) || cls.mixins.exists(hasLazyVal) + } + override def transformDefDef(tree: tpd.DefDef)(implicit ctx: Context, info: TransformerInfo): tpd.Tree = transformLazyVal(tree) From 8439c7feb204cfacf861322ad6dcfe4056188891 Mon Sep 17 00:00:00 2001 From: Dmitry Petrashko Date: Sat, 20 Feb 2016 17:24:07 +0100 Subject: [PATCH 2/3] FirstTransform: simplify needsCompanion code --- src/dotty/tools/dotc/transform/FirstTransform.scala | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/dotty/tools/dotc/transform/FirstTransform.scala b/src/dotty/tools/dotc/transform/FirstTransform.scala index 5d42d4c1651b..c24c5bbd9ada 100644 --- a/src/dotty/tools/dotc/transform/FirstTransform.scala +++ b/src/dotty/tools/dotc/transform/FirstTransform.scala @@ -34,11 +34,13 @@ class FirstTransform extends MiniPhaseTransform with IdentityDenotTransformer wi override def phaseName = "firstTransform" - private var needsCompanionPredicate: ClassSymbol => Boolean = null + private var addCompanionPhases: List[NeedsCompanions] = _ + + def needsCompanion(cls: ClassSymbol)(implicit ctx: Context) = + addCompanionPhases.exists(_.isCompanionNeeded(cls)) override def prepareForUnit(tree: tpd.Tree)(implicit ctx: Context): TreeTransform = { - needsCompanionPredicate = ctx.phasePlan.flatMap(_.filter(x => x.isInstanceOf[NeedsCompanions])). - foldLeft((x: ClassSymbol) => false)((pred, phase) => x => pred(x) || phase.asInstanceOf[NeedsCompanions].isCompanionNeeded(x)) + addCompanionPhases = ctx.phasePlan.flatMap(_ collect { case p: NeedsCompanions => p }) this } @@ -90,7 +92,7 @@ class FirstTransform extends MiniPhaseTransform with IdentityDenotTransformer wi } def addMissingCompanions(stats: List[Tree]): List[Tree] = stats map { - case stat: TypeDef if (singleClassDefs contains stat.name) && needsCompanionPredicate(stat.symbol.asClass) => + case stat: TypeDef if (singleClassDefs contains stat.name) && needsCompanion(stat.symbol.asClass) => val objName = stat.name.toTermName val nameClash = stats.exists { case other: MemberDef => From 0f4d74d9bc7ef99599082b403cb3aa55d181a071 Mon Sep 17 00:00:00 2001 From: Dmitry Petrashko Date: Sat, 20 Feb 2016 17:36:03 +0100 Subject: [PATCH 3/3] Rename unused phases. --- ...opEmptyCompanions.scala => DropEmptyCompanions.scala.disabled} | 0 .../transform/{Literalize.scala => Literalize.scala.disabled} | 0 .../{PrivateToStatic.scala => PrivateToStatic.scala.disabled} | 0 3 files changed, 0 insertions(+), 0 deletions(-) rename src/dotty/tools/dotc/transform/{DropEmptyCompanions.scala => DropEmptyCompanions.scala.disabled} (100%) rename src/dotty/tools/dotc/transform/{Literalize.scala => Literalize.scala.disabled} (100%) rename src/dotty/tools/dotc/transform/{PrivateToStatic.scala => PrivateToStatic.scala.disabled} (100%) diff --git a/src/dotty/tools/dotc/transform/DropEmptyCompanions.scala b/src/dotty/tools/dotc/transform/DropEmptyCompanions.scala.disabled similarity index 100% rename from src/dotty/tools/dotc/transform/DropEmptyCompanions.scala rename to src/dotty/tools/dotc/transform/DropEmptyCompanions.scala.disabled diff --git a/src/dotty/tools/dotc/transform/Literalize.scala b/src/dotty/tools/dotc/transform/Literalize.scala.disabled similarity index 100% rename from src/dotty/tools/dotc/transform/Literalize.scala rename to src/dotty/tools/dotc/transform/Literalize.scala.disabled diff --git a/src/dotty/tools/dotc/transform/PrivateToStatic.scala b/src/dotty/tools/dotc/transform/PrivateToStatic.scala.disabled similarity index 100% rename from src/dotty/tools/dotc/transform/PrivateToStatic.scala rename to src/dotty/tools/dotc/transform/PrivateToStatic.scala.disabled