diff --git a/compiler/src/dotty/tools/dotc/Compiler.scala b/compiler/src/dotty/tools/dotc/Compiler.scala index 8f4ab656881b..47c622bd6686 100644 --- a/compiler/src/dotty/tools/dotc/Compiler.scala +++ b/compiler/src/dotty/tools/dotc/Compiler.scala @@ -109,9 +109,11 @@ class Compiler { new GetClass) :: // Rewrites getClass calls on primitive types. List(new LinkScala2Impls, // Redirect calls to trait methods defined by Scala 2.x, so that they now go to new LambdaLift, // Lifts out nested functions to class scope, storing free variables in environments - // Note: in this mini-phase block scopes are incorrect. No phases that rely on scopes should be here - new ElimStaticThis) :: // Replace `this` references to static objects by global identifiers - List(new Flatten, // Lift all inner classes to package scope + // Note: in this mini-phase block scopes are incorrect. No phases that rely on scopes should be here + new ElimStaticThis, // Replace `this` references to static objects by global identifiers + new CountOuterAccesses) :: // Identify outer accessors that can be dropped + List(new DropOuterAccessors, // Drop unused outer accessors + new Flatten, // Lift all inner classes to package scope new RenameLifted, // Renames lifted classes to local numbering scheme new TransformWildcards, // Replace wildcards with default values new MoveStatics, // Move static methods from companion to the class itself diff --git a/compiler/src/dotty/tools/dotc/core/Phases.scala b/compiler/src/dotty/tools/dotc/core/Phases.scala index ef8866711fef..2743ad56d6c4 100644 --- a/compiler/src/dotty/tools/dotc/core/Phases.scala +++ b/compiler/src/dotty/tools/dotc/core/Phases.scala @@ -230,6 +230,7 @@ object Phases { private var myErasurePhase: Phase = _ private var myElimErasedValueTypePhase: Phase = _ private var myLambdaLiftPhase: Phase = _ + private var myCountOuterAccessesPhase: Phase = _ private var myFlattenPhase: Phase = _ private var myGenBCodePhase: Phase = _ @@ -248,6 +249,7 @@ object Phases { final def erasurePhase: Phase = myErasurePhase final def elimErasedValueTypePhase: Phase = myElimErasedValueTypePhase final def lambdaLiftPhase: Phase = myLambdaLiftPhase + final def countOuterAccessesPhase = myCountOuterAccessesPhase final def flattenPhase: Phase = myFlattenPhase final def genBCodePhase: Phase = myGenBCodePhase @@ -267,6 +269,7 @@ object Phases { myElimErasedValueTypePhase = phaseOfClass(classOf[ElimErasedValueType]) myPatmatPhase = phaseOfClass(classOf[PatternMatcher]) myLambdaLiftPhase = phaseOfClass(classOf[LambdaLift]) + myCountOuterAccessesPhase = phaseOfClass(classOf[CountOuterAccesses]) myFlattenPhase = phaseOfClass(classOf[Flatten]) myExplicitOuterPhase = phaseOfClass(classOf[ExplicitOuter]) myGettersPhase = phaseOfClass(classOf[Getters]) diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 6081bd5a9686..fd30224091e2 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -1079,9 +1079,15 @@ object SymDenotations { final def lexicallyEnclosingClass(implicit ctx: Context): Symbol = if (!exists || isClass) symbol else owner.lexicallyEnclosingClass + /** A class is extensible if it is not final, nor a module class, + * nor an anonymous class. + */ + final def isExtensibleClass(using Context): Boolean = + isClass && !isOneOf(FinalOrModuleClass) && !isAnonymousClass + /** A symbol is effectively final if it cannot be overridden in a subclass */ final def isEffectivelyFinal(implicit ctx: Context): Boolean = - isOneOf(EffectivelyFinalFlags) || !owner.isClass || owner.isOneOf(FinalOrModuleClass) || owner.isAnonymousClass + isOneOf(EffectivelyFinalFlags) || !owner.isExtensibleClass /** A class is effectively sealed if has the `final` or `sealed` modifier, or it * is defined in Scala 3 and is neither abstract nor open. diff --git a/compiler/src/dotty/tools/dotc/transform/Constructors.scala b/compiler/src/dotty/tools/dotc/transform/Constructors.scala index 4e0f29d999c3..37343dfb658a 100644 --- a/compiler/src/dotty/tools/dotc/transform/Constructors.scala +++ b/compiler/src/dotty/tools/dotc/transform/Constructors.scala @@ -177,6 +177,8 @@ class Constructors extends MiniPhase with IdentityDenotTransformer { thisPhase = ) && fn.symbol.info.resultType.classSymbol == outerParam.info.classSymbol => ref(outerParam) + case tree: RefTree if tree.symbol.is(ParamAccessor) && tree.symbol.name == nme.OUTER => + ref(outerParam) case _ => super.transform(tree) } diff --git a/compiler/src/dotty/tools/dotc/transform/CountOuterAccesses.scala b/compiler/src/dotty/tools/dotc/transform/CountOuterAccesses.scala new file mode 100644 index 000000000000..13089a69adba --- /dev/null +++ b/compiler/src/dotty/tools/dotc/transform/CountOuterAccesses.scala @@ -0,0 +1,58 @@ +package dotty.tools.dotc +package transform + +import core._ +import MegaPhase.MiniPhase +import dotty.tools.dotc.core.Contexts.Context +import ast._ +import Trees._ +import Flags._ +import Symbols._ +import Decorators._ +import ExplicitOuter.isOuterParamAccessor + +import collection.mutable + +object CountOuterAccesses: + val name: String = "countOuterAccesses" + + /** Characterizes outer accessors and outer fields that can be dropped + * if there are no references to them from within the toplevel class + * where they are defined. + */ + def mightBeDropped(sym: Symbol)(using Context) = + def isLocal(cls: Symbol) = + cls.isAnonymousClass + || cls.owner.isTerm + || cls.accessBoundary(defn.RootClass).isContainedIn(cls.topLevelClass) + (sym.is(OuterAccessor) || sym.isOuterParamAccessor) && isLocal(sym.owner) + +/** Counts number of accesses to outer accessors and outer fields of + * classes that are visible only within one source file. The info + * is collected in `outerAccessCount` and used in the subsequent + * DropOuterAccessors phase + */ +class CountOuterAccesses extends MiniPhase: + thisPhase => + import tpd._ + + override def phaseName: String = CountOuterAccesses.name + + override def runsAfter: Set[String] = Set(LambdaLift.name) + // LambdaLift can create outer paths. These need to be known in this phase. + + /** The number of times an outer accessor that might be dropped is accessed */ + val outerAccessCount = new mutable.HashMap[Symbol, Int] { + override def default(s: Symbol): Int = 0 + } + + private def markAccessed(tree: RefTree)(implicit ctx: Context): Tree = + val sym = tree.symbol + if CountOuterAccesses.mightBeDropped(sym) then outerAccessCount(sym) += 1 + tree + + override def transformIdent(tree: Ident)(using Context): Tree = + markAccessed(tree) + + override def transformSelect(tree: Select)(using Context): Tree = + markAccessed(tree) diff --git a/compiler/src/dotty/tools/dotc/transform/DropOuterAccessors.scala b/compiler/src/dotty/tools/dotc/transform/DropOuterAccessors.scala new file mode 100644 index 000000000000..0318af1583fc --- /dev/null +++ b/compiler/src/dotty/tools/dotc/transform/DropOuterAccessors.scala @@ -0,0 +1,86 @@ +package dotty.tools.dotc +package transform + +import core._ +import MegaPhase.MiniPhase +import dotty.tools.dotc.core.Contexts.Context +import ast._ +import Trees._ +import Flags._ +import Symbols._ +import Contexts._ +import Decorators._ +import DenotTransformers._ +import ExplicitOuter.isOuterParamAccessor +import CountOuterAccesses.mightBeDropped +import collection.mutable +import annotation.threadUnsafe + +object DropOuterAccessors: + val name: String = "dropOuterAccessors" + +/** Drops unused outer accessors of inner classes that are visible only in one + * toplevel class. For other classes, we can't tell whether an outer accessor + * is used or not. It could for instance be used in a type test in some other source. + */ +class DropOuterAccessors extends MiniPhase with IdentityDenotTransformer: + thisPhase => + import tpd._ + + override def phaseName: String = DropOuterAccessors.name + + override def runsAfterGroupsOf: Set[String] = Set(CountOuterAccesses.name) + + override def changesMembers: Boolean = true // the phase drops outer accessors + + override def transformTemplate(impl: Template)(using ctx: Context): Tree = + val outerAccessCount = ctx.base.countOuterAccessesPhase + .asInstanceOf[CountOuterAccesses] + .outerAccessCount + + def dropOuterAccessor(stat: Tree): Boolean = stat match + case stat: DefDef + if stat.symbol.is(OuterAccessor) + && mightBeDropped(stat.symbol) + && outerAccessCount(stat.symbol) == 0 => + assert(stat.rhs.isInstanceOf[RefTree], stat) + assert(outerAccessCount(stat.rhs.symbol) > 0) + outerAccessCount(stat.rhs.symbol) -= 1 + stat.symbol.dropAfter(thisPhase) + true + case _ => + false + + val droppedParamAccessors = mutable.Set[Symbol]() + + def dropOuterParamAccessor(stat: Tree): Boolean = stat match + case stat: ValDef + if stat.symbol.isOuterParamAccessor + && mightBeDropped(stat.symbol) + && outerAccessCount(stat.symbol) == 1 => + droppedParamAccessors += stat.symbol + stat.symbol.dropAfter(thisPhase) + true + case _ => + false + + def dropOuterInit(stat: Tree): Boolean = stat match + case Assign(lhs, rhs) => droppedParamAccessors.remove(lhs.symbol) + case _ => false + + val body1 = impl.body + .filterNot(dropOuterAccessor) + .filterNot(dropOuterParamAccessor) + val constr1 = + if droppedParamAccessors.isEmpty then impl.constr + else cpy.DefDef(impl.constr)( + rhs = impl.constr.rhs match { + case rhs @ Block(inits, expr) => + cpy.Block(rhs)(inits.filterNot(dropOuterInit), expr) + }) + assert(droppedParamAccessors.isEmpty, + i"""Failed to eliminate: $droppedParamAccessors + when dropping outer accessors for ${ctx.owner} with + $impl""") + cpy.Template(impl)(constr = constr1, body = body1) + end transformTemplate \ No newline at end of file diff --git a/compiler/src/dotty/tools/dotc/transform/ExplicitOuter.scala b/compiler/src/dotty/tools/dotc/transform/ExplicitOuter.scala index 9acc3ef34319..de483dcb333e 100644 --- a/compiler/src/dotty/tools/dotc/transform/ExplicitOuter.scala +++ b/compiler/src/dotty/tools/dotc/transform/ExplicitOuter.scala @@ -1,4 +1,5 @@ -package dotty.tools.dotc +package dotty.tools +package dotc package transform import MegaPhase._ @@ -178,7 +179,7 @@ object ExplicitOuter { /** A new param accessor for the outer field in class `cls` */ private def newOuterParamAccessor(cls: ClassSymbol)(implicit ctx: Context) = - newOuterSym(cls, cls, nme.OUTER, Private | ParamAccessor) + newOuterSym(cls, cls, nme.OUTER, Private | Local | ParamAccessor) /** A new outer accessor for class `cls` which is a member of `owner` */ private def newOuterAccessor(owner: ClassSymbol, cls: ClassSymbol)(implicit ctx: Context) = { @@ -322,6 +323,9 @@ object ExplicitOuter { tpe } + def (sym: Symbol).isOuterParamAccessor(using Context): Boolean = + sym.is(ParamAccessor) && sym.name == nme.OUTER + def outer(implicit ctx: Context): OuterOps = new OuterOps(ctx) /** The operations in this class @@ -386,26 +390,28 @@ object ExplicitOuter { */ def path(start: Tree = This(ctx.owner.lexicallyEnclosingClass.asClass), toCls: Symbol = NoSymbol, - count: Int = -1): Tree = try { - @tailrec def loop(tree: Tree, count: Int): Tree = { - val treeCls = tree.tpe.widen.classSymbol - val outerAccessorCtx = ctx.withPhaseNoLater(ctx.lambdaLiftPhase) // lambdalift mangles local class names, which means we cannot reliably find outer acessors anymore - ctx.log(i"outer to $toCls of $tree: ${tree.tpe}, looking for ${outerAccName(treeCls.asClass)(outerAccessorCtx)} in $treeCls") - if (count == 0 || count < 0 && treeCls == toCls) tree - else { - val acc = outerAccessor(treeCls.asClass)(outerAccessorCtx) - assert(acc.exists, - i"failure to construct path from ${ctx.owner.ownersIterator.toList}%/% to `this` of ${toCls.showLocated};\n${treeCls.showLocated} does not have an outer accessor") - loop(tree.select(acc).ensureApplied, count - 1) - } - } - ctx.log(i"computing outerpath to $toCls from ${ctx.outersIterator.map(_.owner).toList}") - loop(start, count) - } - catch { - case ex: ClassCastException => + count: Int = -1): Tree = + try + @tailrec def loop(tree: Tree, count: Int): Tree = + val treeCls = tree.tpe.widen.classSymbol + val outerAccessorCtx = ctx.withPhaseNoLater(ctx.lambdaLiftPhase) // lambdalift mangles local class names, which means we cannot reliably find outer acessors anymore + ctx.log(i"outer to $toCls of $tree: ${tree.tpe}, looking for ${outerAccName(treeCls.asClass)(outerAccessorCtx)} in $treeCls") + if (count == 0 || count < 0 && treeCls == toCls) tree + else + val enclClass = ctx.owner.lexicallyEnclosingClass.asClass + val outerAcc = tree match + case tree: This if tree.symbol == enclClass && !enclClass.is(Trait) => + outerParamAccessor(enclClass)(using outerAccessorCtx) + case _ => + outerAccessor(treeCls.asClass)(using outerAccessorCtx) + assert(outerAcc.exists, + i"failure to construct path from ${ctx.owner.ownersIterator.toList}%/% to `this` of ${toCls.showLocated};\n${treeCls.showLocated} does not have an outer accessor") + loop(tree.select(outerAcc).ensureApplied, count - 1) + + ctx.log(i"computing outerpath to $toCls from ${ctx.outersIterator.map(_.owner).toList}") + loop(start, count) + catch case ex: ClassCastException => throw new ClassCastException(i"no path exists from ${ctx.owner.enclosingClass} to $toCls") - } /** The outer parameter definition of a constructor if it needs one */ def paramDefs(constr: Symbol): List[ValDef] = diff --git a/compiler/src/dotty/tools/dotc/transform/LambdaLift.scala b/compiler/src/dotty/tools/dotc/transform/LambdaLift.scala index 0f791a217e39..46ffb195faa5 100644 --- a/compiler/src/dotty/tools/dotc/transform/LambdaLift.scala +++ b/compiler/src/dotty/tools/dotc/transform/LambdaLift.scala @@ -23,6 +23,8 @@ object LambdaLift { import ast.tpd._ private class NoPath extends Exception + val name: String = "lambdaLift" + /** The core lambda lift functionality. */ class Lifter(thisPhase: MiniPhase with DenotTransformer)(implicit ctx: Context) { @@ -500,7 +502,7 @@ class LambdaLift extends MiniPhase with IdentityDenotTransformer { thisPhase => import ast.tpd._ /** the following two members override abstract members in Transform */ - val phaseName: String = "lambdaLift" + val phaseName: String = LambdaLift.name override def relaxedTypingInGroup: Boolean = true // Because it adds free vars as additional proxy parameters diff --git a/tests/run/i8033.scala b/tests/run/i8033.scala new file mode 100644 index 000000000000..b047ac552439 --- /dev/null +++ b/tests/run/i8033.scala @@ -0,0 +1,36 @@ +trait Okay extends Serializable { + def okay: Okay +} + +class Foo { + def okay1: Okay = new Okay() { + val okay: Okay = this + override def toString = "okay1" + } + def okay2: Okay = new Okay { + val okay: Okay = okay1 + override def toString = "okay2" + } +} + +object Test { + def main(args: Array[String]): Unit = { + val foo = new Foo + assert(roundTrip(foo.okay1).toString == "okay1") + assert(roundTrip(foo.okay2).toString == "okay2") + } + + def roundTrip[A](a: A): A = { + import java.io._ + + val aos = new ByteArrayOutputStream() + val oos = new ObjectOutputStream(aos) + oos.writeObject(a) + oos.close() + val ais = new ByteArrayInputStream(aos.toByteArray()) + val ois: ObjectInputStream = new ObjectInputStream(ais) + val newA = ois.readObject() + ois.close() + newA.asInstanceOf[A] + } +} \ No newline at end of file diff --git a/tests/run/outer-accessors.scala b/tests/run/outer-accessors.scala new file mode 100644 index 000000000000..47d7a5b1849d --- /dev/null +++ b/tests/run/outer-accessors.scala @@ -0,0 +1,18 @@ +class A: + val a = 2 + + class B: + val b = 3 + + trait T: + def t = a + b + + val bb = B() + + class C extends bb.T: + def result = a + t + +@main def Test = + val a = A() + val c = a.C() + assert(c.result == 7)