diff options
| -rw-r--r-- | src/dotty/tools/dotc/transform/Constructors.scala | 179 | ||||
| -rw-r--r-- | tests/pos/Fileish.scala | 53 | ||||
| -rw-r--r-- | tests/pos/constrs.scala | 33 |
3 files changed, 193 insertions, 72 deletions
diff --git a/src/dotty/tools/dotc/transform/Constructors.scala b/src/dotty/tools/dotc/transform/Constructors.scala index 3d2f5a0d7..9cb348dff 100644 --- a/src/dotty/tools/dotc/transform/Constructors.scala +++ b/src/dotty/tools/dotc/transform/Constructors.scala @@ -21,6 +21,8 @@ import collection.mutable /** This transform * - moves initializers from body to constructor. * - makes all supercalls explicit + * - also moves private fields that are accessed only from constructor + * into the constructor if possible. */ class Constructors extends MiniPhaseTransform with SymTransformer { thisTransform => import tpd._ @@ -35,7 +37,7 @@ class Constructors extends MiniPhaseTransform with SymTransformer { thisTransfor */ override def transformSym(sym: SymDenotation)(implicit ctx: Context): SymDenotation = { def ownerBecomesConstructor(owner: Symbol): Boolean = - (owner.isLocalDummy || owner.isTerm && !owner.is(Method)) && + (owner.isLocalDummy || owner.isTerm && !owner.is(Method | Lazy)) && owner.owner.isClass if (ownerBecomesConstructor(sym.owner)) sym.copySymDenotation(owner = sym.owner.enclosingClass.primaryConstructor) @@ -48,38 +50,14 @@ class Constructors extends MiniPhaseTransform with SymTransformer { thisTransfor private def noDirectRefsFrom(tree: Tree)(implicit ctx: Context) = tree.isDef && tree.symbol.isClass && !tree.symbol.is(InSuperCall) - /** Adjustments performed when moving code into the constructor: - * (1) Replace references to param accessors by constructor parameters - * except possibly references to mutable variables, if `excluded = Mutable`. - * (Mutable parameters should be replaced only during the super call) - * (2) If the parameter accessor reference was to an alias getter, - * drop the () when replacing by the parameter. + /** Class members that can be eliminated if referenced only from their own + * constructor. */ - class IntoConstrMap(accessors: List[Symbol], params: List[Symbol]) extends TreeMap { - private var excluded: FlagSet = _ - override def transform(tree: Tree)(implicit ctx: Context): Tree = tree match { - case Ident(_) | Select(This(_), _) => - val sym = tree.symbol - if (sym is (ParamAccessor, butNot = excluded)) { - val param = sym.subst(accessors, params) - if (param ne sym) ref(param).withPos(tree.pos) - else tree - } - else tree - case Apply(fn, Nil) => - val fn1 = transform(fn) - if ((fn1 ne fn) && fn1.symbol.is(Param) && fn1.symbol.owner.isPrimaryConstructor) - fn1 // in this case, fn1.symbol was an alias for a parameter in a superclass - else cpy.Apply(tree)(fn1, Nil) - case _ => - if (noDirectRefsFrom(tree)) tree else super.transform(tree) - } + private def mightBeDropped(sym: Symbol)(implicit ctx: Context) = + sym.is(Private, butNot = KeeperFlags) && !sym.is(MutableParamAccessor) - def apply(tree: Tree, excluded: FlagSet)(implicit ctx: Context): Tree = { - this.excluded = excluded - transform(tree) - } - } + private final val KeeperFlags = Method | Lazy | NotJavaPrivate + private final val MutableParamAccessor = allOf(Mutable, ParamAccessor) override def transformTemplate(tree: Template)(implicit ctx: Context, info: TransformerInfo): Tree = { val cls = ctx.owner.asClass @@ -102,7 +80,34 @@ class Constructors extends MiniPhaseTransform with SymTransformer { thisTransfor } val paramSyms = vparamsWithOuter map (_.symbol) - val intoConstr = new IntoConstrMap(accessors, paramSyms) + // Adjustments performed when moving code into the constructor: + // (1) Replace references to param accessors by constructor parameters + // except possibly references to mutable variables, if `excluded = Mutable`. + // (Mutable parameters should be replaced only during the super call) + // (2) If the parameter accessor reference was to an alias getter, + // drop the () when replacing by the parameter. + object intoConstr extends TreeMap { + private var excluded: FlagSet = _ + override def transform(tree: Tree)(implicit ctx: Context): Tree = tree match { + case Ident(_) | Select(This(_), _) => + var sym = tree.symbol + if (sym is (ParamAccessor, butNot = excluded)) sym = sym.subst(accessors, paramSyms) + if (sym.owner.isConstructor) ref(sym).withPos(tree.pos) else tree + case Apply(fn, Nil) => + val fn1 = transform(fn) + if ((fn1 ne fn) && fn1.symbol.is(Param) && fn1.symbol.owner.isPrimaryConstructor) + fn1 // in this case, fn1.symbol was an alias for a parameter in a superclass + else cpy.Apply(tree)(fn1, Nil) + case _ => + if (noDirectRefsFrom(tree)) tree else super.transform(tree) + } + + def apply(tree: Tree, inSuperCall: Boolean = false)(implicit ctx: Context): Tree = { + this.excluded = if (inSuperCall) EmptyFlags else Mutable + transform(tree) + } + } + val superCalls = new mutable.ListBuffer[Tree] // If parent is a constructor call, pull out the call into a separate @@ -115,12 +120,12 @@ class Constructors extends MiniPhaseTransform with SymTransformer { thisTransfor nme.CONSTRUCTOR), superArgs) => val toClass = !superType.symbol.is(Trait) - val mappedArgs = superArgs.map( - intoConstr(_, excluded = if (toClass) Mutable else EmptyFlags)) + val mappedArgs = superArgs.map(intoConstr(_, inSuperCall = toClass)) val receiver = if (toClass) Super(This(cls), tpnme.EMPTY, inConstrCall = true) else This(cls) - superCalls += cpy.Apply(superApp)( + superCalls += + cpy.Apply(superApp)( receiver.withPos(superNew.pos) .select(superSel.symbol).withPos(superSel.pos), mappedArgs) @@ -129,67 +134,97 @@ class Constructors extends MiniPhaseTransform with SymTransformer { thisTransfor } val parentTypeTrees = tree.parents.map(normalizeParent) + // Collect all private parameter accessors and value definitions that need + // to be retained. There are several reasons why a parameter accessor or + // definition might need to be retained: + // 1. It is accessed after the constructor has finished + // 2. It is accessed before it is defined + // 3. It is accessed on an object other than `this` + // 4. It is a mutable parameter accessor + // 5. It is has a wildcard initializer `_` + object usage extends TreeTraverser { + private var inConstr: Boolean = true + private val seen = mutable.Set[Symbol](accessors: _*) + val retained = mutable.Set[Symbol]() + def dropped: collection.Set[Symbol] = seen -- retained + override def traverse(tree: Tree) = { + val sym = tree.symbol + tree match { + case Ident(_) | Select(This(_), _) if inConstr && seen(tree.symbol) => + // could refer to definition in constructors, so no retention necessary + case tree: RefTree => + if (mightBeDropped(sym)) retained += sym + case _ => + } + if (!noDirectRefsFrom(tree)) traverseChildren(tree) + } + def collect(stats: List[Tree]): Unit = stats foreach { + case stat: ValDef if !stat.symbol.is(Lazy) => + traverse(stat) + if (mightBeDropped(stat.symbol)) + (if (isWildcardStarArg(stat.rhs)) retained else seen) += stat.symbol + case stat: DefTree => + inConstr = false + traverse(stat) + inConstr = true + case stat => + traverse(stat) + } + } + usage.collect(superCalls.toList ++ tree.body) + + def isRetained(acc: Symbol) = !mightBeDropped(acc) || usage.retained(acc) + + val constrStats, clsStats = new mutable.ListBuffer[Tree] + // Split class body into statements that go into constructor and // definitions that are kept as members of the class. - def splitStats(stats: List[Tree]): (List[Tree], List[Tree]) = stats match { + def splitStats(stats: List[Tree]): Unit = stats match { case stat :: stats1 => - val (constrStats, clsStats) = splitStats(stats1) stat match { - case stat @ ValDef(mods, name, tpt, rhs) => - val inits = - if (rhs.isEmpty || isWildcardArg(rhs)) Nil - else Assign(ref(stat.symbol), intoConstr(rhs, excluded = Mutable)) - .withPos(stat.pos) :: Nil - (inits ::: constrStats, cpy.ValDef(stat)(rhs = EmptyTree) :: clsStats) + case stat @ ValDef(mods, name, tpt, rhs) if !stat.symbol.is(Lazy) => + val sym = stat.symbol + if (isRetained(sym)) { + if (!rhs.isEmpty && !isWildcardArg(rhs)) + constrStats += Assign(ref(sym), intoConstr(rhs)).withPos(stat.pos) + clsStats += cpy.ValDef(stat)(rhs = EmptyTree) + } + else if (!rhs.isEmpty) { + sym.copySymDenotation( + initFlags = sym.flags &~ Private, + owner = constr.symbol).installAfter(thisTransform) + constrStats += intoConstr(stat) + } case _: DefTree => - (constrStats, stat :: clsStats) + clsStats += stat case _ => - (intoConstr(stat, excluded = Mutable) :: constrStats, clsStats) + constrStats += intoConstr(stat) } + splitStats(stats1) case Nil => (Nil, Nil) } - val (constrStats, clsStats) = splitStats(tree.body) - - // Collect all private parameter accessors that need to be retained - // because they are accessed after the constructor has finished. - val collectRetained = new TreeAccumulator[Set[Symbol]] { - override def apply(retained: Set[Symbol], tree: Tree) = tree match { - case tree: RefTree => - val sym = tree.symbol - foldOver( - if (sym.is(PrivateParamAccessor) && sym.owner == cls) retained + sym else retained, - tree) - case _ => - if (noDirectRefsFrom(tree)) retained else foldOver(retained, tree) - } - } - val retainedPrivate = collectRetained(collectRetained(Set[Symbol](), constrStats), clsStats) - def isRetained(acc: Symbol) = - (!acc.is(Private) || acc.is(NotJavaPrivate) || retainedPrivate(acc)) + splitStats(tree.body) val accessorFields = accessors.filterNot(_ is Method) - val (retainedAccessors, droppedAccessors) = accessorFields.partition(isRetained) // The initializers for the retained accessors */ - val copyParams = retainedAccessors.map(acc => + val copyParams = accessorFields.filter(isRetained).map(acc => Assign(ref(acc), ref(acc.subst(accessors, paramSyms))).withPos(tree.pos)) // Drop accessors that are not retained from class scope - if (droppedAccessors.nonEmpty) { + val dropped = usage.dropped + if (dropped.nonEmpty) { val clsInfo = cls.classInfo // TODO investigate: expand clsInfo to cls.info => dotty type error cls.copy( info = clsInfo.derivedClassInfo( - decls = clsInfo.decls.filteredScope(!droppedAccessors.contains(_)))) + decls = clsInfo.decls.filteredScope(!dropped.contains(_)))) } cpy.Template(tree)( constr = cpy.DefDef(constr)( - rhs = Block(superCalls.toList ::: copyParams ::: constrStats, unitLiteral)), + rhs = Block(superCalls.toList ::: copyParams ::: constrStats.toList, unitLiteral)), parents = parentTypeTrees, - body = clsStats filter { - case vdef: ValDef => !droppedAccessors.contains(vdef.symbol) - case _ => true - }) + body = clsStats.toList) } }
\ No newline at end of file diff --git a/tests/pos/Fileish.scala b/tests/pos/Fileish.scala new file mode 100644 index 000000000..d226bb0bd --- /dev/null +++ b/tests/pos/Fileish.scala @@ -0,0 +1,53 @@ +// Inspired by the original Fileish, +// testing combinations of lazy and non-lazy vals for their treatment in constructors +package dotty.tools +package io + +import java.io.{ InputStream } +import java.util.jar.JarEntry +import language.postfixOps + +/** A common interface for File-based things and Stream-based things. + * (In particular, io.File and JarEntry.) + */ +class Fileish(val path: Path, val input: () => InputStream) extends Streamable.Chars { + def inputStream() = input() + + def parent = path.parent + def name = path.name + def isSourceFile = path.hasExtension("java", "scala") + + private lazy val pkgLines = lines() collect { case x if x startsWith "package " => x stripPrefix "package" trim } + lazy val pkgFromPath = parent.path.replaceAll("""[/\\]""", ".") + lazy val pkgFromSource = pkgLines map (_ stripSuffix ";") mkString "." + + override def toString = path.path +} +class Fileish2(val path: Path, val input: () => InputStream) extends Streamable.Chars { + def inputStream() = input() + + def parent = path.parent + def name = path.name + def isSourceFile = path.hasExtension("java", "scala") + + private val pkgLines = lines() collect { case x if x startsWith "package " => x stripPrefix "package" trim } + lazy val pkgFromPath = parent.path.replaceAll("""[/\\]""", ".") + lazy val pkgFromSource = pkgLines map (_ stripSuffix ";") mkString "." + + override def toString = path.path +} + +class Fileish3(val path: Path, val input: () => InputStream) extends Streamable.Chars { + def inputStream() = input() + + def parent = path.parent + def name = path.name + def isSourceFile = path.hasExtension("java", "scala") + + private val pkgLines = lines() collect { case x if x startsWith "package " => x stripPrefix "package" trim } + private val pkgFromPath = parent.path.replaceAll("""[/\\]""", ".") + private val pkgFromSource = pkgLines map (_ stripSuffix ";") mkString "." + + override def toString = path.path +} + diff --git a/tests/pos/constrs.scala b/tests/pos/constrs.scala new file mode 100644 index 000000000..dc0e1a369 --- /dev/null +++ b/tests/pos/constrs.scala @@ -0,0 +1,33 @@ +class Foo(x: Int, var y: Int) { + + val z: Int = 0 + + var u: Int = _ + + def f = x + +} + +class Baz(val base: Int) { + +} + + +class Bar(base: Int, byName: => String, local: Int) extends Baz(base + local) { + + def f() = println(base.toString + byName) + +} + +class Rational(n: Int, d: Int) { + def gcd(x: Int, y: Int): Int = ??? + private val x = gcd(n, d) + def numer = n / x + def denom = d / x +} +class Rational2(n: Int, d: Int) { + def gcd(x: Int, y: Int): Int = ??? + private val x = gcd(n, d) + val numer = n / x + val denom = d / x +} |