diff --git a/compiler/src/dotty/tools/dotc/Compiler.scala b/compiler/src/dotty/tools/dotc/Compiler.scala index 4566aaad486a..381fb3b7869d 100644 --- a/compiler/src/dotty/tools/dotc/Compiler.scala +++ b/compiler/src/dotty/tools/dotc/Compiler.scala @@ -87,8 +87,8 @@ class Compiler { new LazyVals, // Expand lazy vals new Memoize, // Add private fields to getters and setters new NonLocalReturns, // Expand non-local returns - new CapturedVars, // Represent vars captured by closures as heap objects - new Constructors, // Collect initialization code in primary constructors + new CapturedVars), // Represent vars captured by closures as heap objects + List(new Constructors, // Collect initialization code in primary constructors // Note: constructors changes decls in transformTemplate, no InfoTransformers should be added after it new FunctionalInterfaces, // Rewrites closures to implement @specialized types of Functions. new GetClass, // Rewrites getClass calls on primitive types. diff --git a/compiler/src/dotty/tools/dotc/transform/Constructors.scala b/compiler/src/dotty/tools/dotc/transform/Constructors.scala index f4c800e342e7..4698930691e2 100644 --- a/compiler/src/dotty/tools/dotc/transform/Constructors.scala +++ b/compiler/src/dotty/tools/dotc/transform/Constructors.scala @@ -30,7 +30,20 @@ class Constructors extends MiniPhaseTransform with IdentityDenotTransformer { th import tpd._ override def phaseName: String = "constructors" - override def runsAfter: Set[Class[_ <: Phase]] = Set(classOf[Memoize], classOf[HoistSuperArgs]) + override def runsAfter: Set[Class[_ <: Phase]] = Set(classOf[HoistSuperArgs]) + override def runsAfterGroupsOf: Set[Class[_ <: Phase]] = Set(classOf[Memoize]) + // Memoized needs to be finished because we depend on the ownerchain after Memoize + // when checking whether an ident is an access in a constructor or outside it. + // This test is done in the right-hand side of a value definition. If Memoize + // was in the same group as Constructors, the test on the rhs ident would be + // performed before the rhs undergoes the owner change. This would lead + // to more symbols being retained as parameters. Test case in run/capturing.scala. + + /** The private vals that are known to be retained as class fields */ + private val retainedPrivateVals = mutable.Set[Symbol]() + + /** The private vals whose definition comes before the current focus */ + private val seenPrivateVals = mutable.Set[Symbol]() // Collect all private parameter accessors and value definitions that need // to be retained. There are several reasons why a parameter accessor or @@ -40,14 +53,10 @@ class Constructors extends MiniPhaseTransform with IdentityDenotTransformer { th // 3. It is accessed on an object other than `this` // 4. It is a mutable parameter accessor // 5. It is has a wildcard initializer `_` - private val retainedPrivateVals = mutable.Set[Symbol]() - private val seenPrivateVals = mutable.Set[Symbol]() - private def markUsedPrivateSymbols(tree: RefTree)(implicit ctx: Context): Unit = { val sym = tree.symbol - def retain() = - retainedPrivateVals.add(sym) + def retain() = retainedPrivateVals.add(sym) if (sym.exists && sym.owner.isClass && mightBeDropped(sym)) { val owner = sym.owner.asClass @@ -58,7 +67,8 @@ class Constructors extends MiniPhaseTransform with IdentityDenotTransformer { th val method = ctx.owner.enclosingMethod method.isPrimaryConstructor && ctx.owner.enclosingClass == owner } - if (inConstructor && (sym.is(ParamAccessor) || seenPrivateVals.contains(sym))) { + if (inConstructor && + (sym.is(ParamAccessor) || seenPrivateVals.contains(sym))) { // used inside constructor, accessed on this, // could use constructor argument instead, no need to retain field } @@ -79,8 +89,7 @@ class Constructors extends MiniPhaseTransform with IdentityDenotTransformer { th } override def transformValDef(tree: tpd.ValDef)(implicit ctx: Context, info: TransformerInfo): tpd.Tree = { - if (mightBeDropped(tree.symbol)) - (if (isWildcardStarArg(tree.rhs)) retainedPrivateVals else seenPrivateVals) += tree.symbol + if (mightBeDropped(tree.symbol)) seenPrivateVals += tree.symbol tree } diff --git a/compiler/src/dotty/tools/dotc/transform/LambdaLift.scala b/compiler/src/dotty/tools/dotc/transform/LambdaLift.scala index 17772df9b91c..0cb4f3e208f2 100644 --- a/compiler/src/dotty/tools/dotc/transform/LambdaLift.scala +++ b/compiler/src/dotty/tools/dotc/transform/LambdaLift.scala @@ -66,7 +66,7 @@ class LambdaLift extends MiniPhase with IdentityDenotTransformer { thisTransform override def relaxedTyping = true - override def runsAfter: Set[Class[_ <: Phase]] = Set(classOf[Constructors], classOf[HoistSuperArgs]) + override def runsAfterGroupsOf: Set[Class[_ <: Phase]] = Set(classOf[Constructors], classOf[HoistSuperArgs]) // Constructors has to happen before LambdaLift because the lambda lift logic // becomes simpler if it can assume that parameter accessors have already been // converted to parameters in super calls. Without this it is very hard to get diff --git a/tests/run/capturing.check b/tests/run/capturing.check new file mode 100644 index 000000000000..66aa5dce5a1d --- /dev/null +++ b/tests/run/capturing.check @@ -0,0 +1 @@ +private final java.lang.String MT.s diff --git a/tests/run/capturing.scala b/tests/run/capturing.scala new file mode 100644 index 000000000000..7f00a8322129 --- /dev/null +++ b/tests/run/capturing.scala @@ -0,0 +1,9 @@ +class MT(sf: MT => String) { + // `s` is retained as a field, but `sf` should not be. + val s = sf(this) +} +object Test extends App { + def printFields(obj: Any) = + println(obj.getClass.getDeclaredFields.map(_.toString).sorted.deep.mkString("\n")) + printFields(new MT(_ => "")) +} diff --git a/tests/run/variable-pattern-access.check b/tests/run/variable-pattern-access.check index 8ab43375fc97..cfc1a6110516 100644 --- a/tests/run/variable-pattern-access.check +++ b/tests/run/variable-pattern-access.check @@ -1,7 +1,6 @@ # Fields of A: private final int A.a private final int A.b -private final scala.Tuple2 A.$1$ # Methods of A: public int A.a() public int A.b()