From 90070e2e9131ccacbd60f575b5539ebf95154bf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Wed, 1 Apr 2020 15:09:48 +0200 Subject: [PATCH 01/24] First pass of reverting to the trait encoding of Scala 2. --- compiler/src/dotty/tools/dotc/Compiler.scala | 2 +- .../tools/dotc/transform/Constructors.scala | 10 ++ .../dotc/transform/LinkScala2Impls.scala | 128 +++++++++++------- .../dotty/tools/dotc/transform/Mixin.scala | 88 +++++++----- .../tools/dotc/transform/ResolveSuper.scala | 2 +- .../dotty/tools/dotc/transform/SymUtils.scala | 4 + 6 files changed, 146 insertions(+), 88 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/Compiler.scala b/compiler/src/dotty/tools/dotc/Compiler.scala index c55fa34d2ebc..f00d0704262a 100644 --- a/compiler/src/dotty/tools/dotc/Compiler.scala +++ b/compiler/src/dotty/tools/dotc/Compiler.scala @@ -86,7 +86,7 @@ class Compiler { new LiftTry, // Put try expressions that might execute on non-empty stacks into their own methods new CollectNullableFields, // Collect fields that can be nulled out after use in lazy initialization new ElimOuterSelect, // Expand outer selections - new AugmentScala2Traits, // Augments Scala2 traits with additional members needed for mixin composition. + //new AugmentScala2Traits, // Augments Scala2 traits with additional members needed for mixin composition. new ResolveSuper, // Implement super accessors new FunctionXXLForwarders, // Add forwarders for FunctionXXL apply method new ParamForwarding, // Add forwarders for aliases of superclass parameters diff --git a/compiler/src/dotty/tools/dotc/transform/Constructors.scala b/compiler/src/dotty/tools/dotc/transform/Constructors.scala index 14ac216b556a..40859b1c9b34 100644 --- a/compiler/src/dotty/tools/dotc/transform/Constructors.scala +++ b/compiler/src/dotty/tools/dotc/transform/Constructors.scala @@ -205,6 +205,16 @@ class Constructors extends MiniPhase with IdentityDenotTransformer { thisPhase = owner = constr.symbol).installAfter(thisPhase) constrStats += intoConstr(stat, sym) } + case stat @ DefDef(name, _, _, tpt, _) + if stat.symbol.isGetter && stat.symbol.owner.is(Trait) && !stat.symbol.is(Lazy) => + val sym = stat.symbol + assert(isRetained(sym), sym) + if (!stat.rhs.isEmpty && !isWildcardArg(stat.rhs)) + val setter = + if (sym.setter.exists) sym.setter + else sym.accessorNamed(Mixin.traitSetterName(sym.asTerm)) + constrStats += Apply(ref(setter), intoConstr(stat.rhs, sym).withSpan(stat.span) :: Nil) + clsStats += cpy.DefDef(stat)(rhs = EmptyTree) case DefDef(nme.CONSTRUCTOR, _, ((outerParam @ ValDef(nme.OUTER, _, _)) :: _) :: Nil, _, _) => clsStats += mapOuter(outerParam.symbol).transform(stat) case _: DefTree => diff --git a/compiler/src/dotty/tools/dotc/transform/LinkScala2Impls.scala b/compiler/src/dotty/tools/dotc/transform/LinkScala2Impls.scala index 3a30ae41fc6e..e83607abc8b5 100644 --- a/compiler/src/dotty/tools/dotc/transform/LinkScala2Impls.scala +++ b/compiler/src/dotty/tools/dotc/transform/LinkScala2Impls.scala @@ -10,7 +10,9 @@ import Symbols._ import Types._ import Decorators._ import DenotTransformers._ +import Names._ import StdNames._ +import SymDenotations._ import ast.Trees._ import NameKinds.ImplMethName @@ -18,13 +20,15 @@ import NameKinds.ImplMethName * * super[M].f(args) * - * where M is a Scala 2.x trait implemented by the current class to + * where M is a trait implemented by the current class to * * M.f$(this, args) * * where f$ is a static member of M. + * + * Also generates said static members, as forwarders to the normal methods. */ -class LinkScala2Impls extends MiniPhase with IdentityDenotTransformer { thisPhase => +class LinkScala2Impls extends MiniPhase with SymTransformer { thisPhase => import ast.tpd._ override def phaseName: String = "linkScala2Impls" @@ -34,47 +38,77 @@ class LinkScala2Impls extends MiniPhase with IdentityDenotTransformer { thisPhas // Adds as a side effect static members to traits which can confuse Mixin, // that's why it is runsAfterGroupOf - private def addStaticForwarders(mixin: ClassSymbol)(using Context): Unit = { - val ops = new MixinOps(mixin, thisPhase) - import ops._ + override def transformSym(sym: SymDenotation)(using Context): SymDenotation = + if sym.is(Trait) && !ctx.settings.scalajs.value then + val mixin = sym.asClass.classSymbol + val ops = new MixinOps(mixin, thisPhase) + import ops._ - def newImpl(meth: TermSymbol): Symbol = { - def staticInfo(tp: Type) = tp match { - case mt: MethodType => - MethodType(nme.SELF :: mt.paramNames, mixin.typeRef :: mt.paramInfos, mt.resType) - } - val mold = - if (meth.isConstructor) - meth.copySymDenotation( - name = nme.TRAIT_CONSTRUCTOR, - info = MethodType(Nil, defn.UnitType)) - else meth.ensureNotPrivate - meth.copy( - owner = mixin, - name = if (meth.isConstructor) mold.name.asTermName else ImplMethName(mold.name.asTermName), - flags = Method | JavaStatic, - info = staticInfo(mold.info) - ) - } - for (sym <- mixin.info.decls) - if (needsMixinForwarder(sym) || sym.isConstructor || sym.isGetter && sym.is(Lazy) || sym.is(Method, butNot = Deferred)) - newImpl(sym.asTerm).enteredAfter(thisPhase) - // The trait is now fully augmented so the flag isn't needed anymore. - mixin.resetFlag(Scala2xPartiallyAugmented) - } + def newImpl(meth: TermSymbol): Symbol = + def staticInfo(tp: Type) = tp match + case mt: MethodType => + MethodType(nme.SELF :: mt.paramNames, mixin.typeRef :: mt.paramInfos, mt.resType) + val mold = meth.ensureNotPrivate + meth.copy( + owner = mixin, + name = staticForwarderName(mold.name.asTermName), + flags = Method | JavaStatic, + info = staticInfo(mold.info) + ) - override def prepareForTemplate(impl: Template)(using Context): Context = { - val cls = impl.symbol.owner.asClass - for (mixin <- cls.mixins if (mixin.is(Scala2xPartiallyAugmented))) - addStaticForwarders(mixin) - ctx - } + val classInfo = sym.asClass.classInfo + val decls1 = classInfo.decls.cloneScope + var modified: Boolean = false + for (meth <- classInfo.decls) + if needsStaticForwarder(meth) then + decls1.enter(newImpl(meth.asTerm)) + modified = true + if modified then + sym.copySymDenotation( + info = classInfo.derivedClassInfo(decls = decls1)) + else + sym + else + sym + end transformSym + + private def needsStaticForwarder(sym: Symbol)(using Context): Boolean = + sym.is(Method, butNot = Deferred) + + private def staticForwarderName(name: TermName)(using Context): TermName = + if name == nme.TRAIT_CONSTRUCTOR then name + else ImplMethName(name) + + override def transformTemplate(impl: Template)(using Context): Tree = + val sym = impl.symbol.owner.asClass + if sym.is(Trait) && !ctx.settings.scalajs.value then + val newConstr :: newBody = (impl.constr :: impl.body).flatMap { + case stat: DefDef if needsStaticForwarder(stat.symbol) => + val meth = stat.symbol + val staticForwarderDef = DefDef(implMethod(meth).asTerm, { paramss => + val params = paramss.head + /* FIXME This is wrong. We are emitting a virtual call to `meth`, + * but we must instead emit an `invokespecial` so that it is not + * virtual. However, I don't know how to represent an invokevirtual + * call on a non-`this` receiver. My best attempt is the following, + * but that throws an exception when type-assigning the Super node: + */ + //Apply(Super(params.head, sym.name.asTypeName, sym).select(meth), params.tail) + Apply(params.head.select(meth), params.tail) + }) + stat :: transformFollowing(staticForwarderDef) :: Nil + case stat => + stat :: Nil + } + cpy.Template(impl)(constr = newConstr.asInstanceOf[DefDef], body = newBody) + else + impl override def transformApply(app: Apply)(using Context): Tree = { def currentClass = ctx.owner.enclosingClass.asClass app match { case Apply(sel @ Select(Super(_, _), _), args) - if sel.symbol.owner.is(Scala2x) && currentClass.mixins.contains(sel.symbol.owner) && !ctx.settings.scalajs.value => + if sel.symbol.owner.is(Trait) && currentClass.mixins.contains(sel.symbol.owner) && !ctx.settings.scalajs.value => val impl = implMethod(sel.symbol) if (impl.exists) Apply(ref(impl), This(currentClass) :: args).withSpan(app.span) else app // could have been an abstract method in a trait linked to from a super constructor @@ -83,19 +117,13 @@ class LinkScala2Impls extends MiniPhase with IdentityDenotTransformer { thisPhas } } - /** The 2.12 implementation method of a super call or implementation class target */ - private def implMethod(meth: Symbol)(using Context): Symbol = { - val implName = ImplMethName(meth.name.asTermName) + /** The implementation method of a super call or implementation class target */ + private def implMethod(meth: Symbol)(using Context): Symbol = val cls = meth.owner - if (cls.isAllOf(Scala2xTrait)) - if (meth.isConstructor) - cls.info.decl(nme.TRAIT_CONSTRUCTOR).symbol - else - cls.info.decl(implName) - .suchThat(c => FullParameterization.memberSignature(c.info) == meth.signature) - .symbol - else throw new AssertionError(i"no impl method for $meth") - } - - private val Scala2xTrait = Scala2x | Trait + if meth.name == nme.TRAIT_CONSTRUCTOR then + cls.info.decl(nme.TRAIT_CONSTRUCTOR).suchThat(_.is(JavaStatic)).symbol + else + cls.info.decl(staticForwarderName(meth.name.asTermName)) + .suchThat(c => FullParameterization.memberSignature(c.info) == meth.signature) + .symbol } diff --git a/compiler/src/dotty/tools/dotc/transform/Mixin.scala b/compiler/src/dotty/tools/dotc/transform/Mixin.scala index 6cdf54d5d305..7ab957f77676 100644 --- a/compiler/src/dotty/tools/dotc/transform/Mixin.scala +++ b/compiler/src/dotty/tools/dotc/transform/Mixin.scala @@ -13,12 +13,19 @@ import Types._ import Decorators._ import DenotTransformers._ import StdNames._ +import Names._ import NameKinds._ +import NameOps._ import ast.Trees._ import collection.mutable object Mixin { val name: String = "mixin" + + def traitSetterName(getter: TermSymbol)(using Context): TermName = + getter.ensureNotPrivate.name + .expandedName(getter.owner, TraitSetterName) + .asTermName.setterName } /** This phase performs the following transformations: @@ -127,12 +134,27 @@ class Mixin extends MiniPhase with SymTransformer { thisPhase => else sym.copySymDenotation(initFlags = sym.flags &~ ParamAccessor | Deferred) sym1.ensureNotPrivate } - else if (sym.isConstructor && (sym.owner.is(Trait, butNot = Scala2x) || (sym.owner.isAllOf(Trait | Scala2x) && ctx.settings.scalajs.value))) + else if (sym.isConstructor && sym.owner.is(Trait)) sym.copySymDenotation( name = nme.TRAIT_CONSTRUCTOR, info = MethodType(Nil, sym.info.resultType)) + else if sym.is(Trait) then + val classInfo = sym.asClass.classInfo + val decls1 = classInfo.decls.cloneScope + var modified: Boolean = false + for (getter <- classInfo.decls) + if needsTraitSetter(getter) then + val setter = makeTraitSetter(getter.asTerm) + decls1.enter(setter) + modified = true + if modified then + sym.copySymDenotation( + info = classInfo.derivedClassInfo(decls = decls1)) + else + sym else sym + end transformSym private def initializer(sym: Symbol)(using Context): TermSymbol = { if (sym.is(Lazy)) sym @@ -149,32 +171,34 @@ class Mixin extends MiniPhase with SymTransformer { thisPhase => } }.asTerm + private def wasOneOf(sym: Symbol, flags: FlagSet)(using Context): Boolean = + atPhase(thisPhase) { sym.isOneOf(flags) } + + private def needsTraitSetter(sym: Symbol)(using Context): Boolean = + sym.isGetter && !wasOneOf(sym, DeferredOrLazy | ParamAccessor) && !sym.setter.exists + && !sym.info.resultType.isInstanceOf[ConstantType] + + private def makeTraitSetter(getter: TermSymbol)(using Context): Symbol = + getter.copy( + name = Mixin.traitSetterName(getter), + flags = Method | Accessor | Deferred, + info = MethodType(getter.info.resultType :: Nil, defn.UnitType)) + override def transformTemplate(impl: Template)(using Context): Template = { val cls = impl.symbol.owner.asClass val ops = new MixinOps(cls, thisPhase) import ops._ def traitDefs(stats: List[Tree]): List[Tree] = { - val initBuf = new mutable.ListBuffer[Tree] - stats.flatMap({ - case stat: DefDef if stat.symbol.isGetter && !stat.rhs.isEmpty && !stat.symbol.is(Lazy) => - // make initializer that has all effects of previous getter, - // replace getter rhs with empty tree. - val vsym = stat.symbol - val isym = initializer(vsym) - val rhs = Block( - initBuf.toList.map(_.changeOwnerAfter(impl.symbol, isym, thisPhase)), - stat.rhs.changeOwnerAfter(vsym, isym, thisPhase).wildcardToDefault) - initBuf.clear() - cpy.DefDef(stat)(rhs = EmptyTree) :: DefDef(isym, rhs) :: Nil + stats.flatMap { + case stat: DefDef if needsTraitSetter(stat.symbol) => + // add a trait setter for this getter + stat :: DefDef(stat.symbol.traitSetter.asTerm, EmptyTree) :: Nil case stat: DefDef if stat.symbol.isSetter => cpy.DefDef(stat)(rhs = EmptyTree) :: Nil - case stat: DefTree => - stat :: Nil case stat => - initBuf += stat - Nil - }) ++ initBuf + stat :: Nil + } } /** Map constructor call to a triple of a supercall, and if the target @@ -221,9 +245,6 @@ class Mixin extends MiniPhase with SymTransformer { thisPhase => //println(i"synth super call ${baseCls.primaryConstructor}: ${baseCls.primaryConstructor.info}") transformFollowingDeep(superRef(baseCls.primaryConstructor).appliedToNone) :: Nil - def wasOneOf(sym: Symbol, flags: FlagSet) = - atPhase(thisPhase) { sym.isOneOf(flags) } - def traitInits(mixin: ClassSymbol): List[Tree] = { val argsIt = superCallsAndArgs.get(mixin) match case Some((_, _, args)) => args.iterator @@ -241,33 +262,28 @@ class Mixin extends MiniPhase with SymTransformer { thisPhase => EmptyTree for (getter <- mixin.info.decls.toList if getter.isGetter && !wasOneOf(getter, Deferred)) yield { - val isScala2x = mixin.is(Scala2x) - def default = Underscore(getter.info.resultType) - def initial = transformFollowing(superRef(initializer(getter)).appliedToNone) - if (isCurrent(getter) || getter.name.is(ExpandedName)) { val rhs = if (wasOneOf(getter, ParamAccessor)) nextArgument() - else if (isScala2x) - if (getter.is(Lazy, butNot = Module)) - initial - else if (getter.is(Module)) - New(getter.info.resultType, List(This(cls))) - else - Underscore(getter.info.resultType) + else if (getter.is(Lazy, butNot = Module)) + transformFollowing(superRef(getter).appliedToNone) + else if (getter.is(Module)) + New(getter.info.resultType, List(This(cls))) else - initial + Underscore(getter.info.resultType) // transformFollowing call is needed to make memoize & lazy vals run transformFollowing(DefDef(mkForwarderSym(getter.asTerm), rhs)) } - else if (isScala2x || wasOneOf(getter, ParamAccessor | Lazy)) EmptyTree - else initial + else EmptyTree } } def setters(mixin: ClassSymbol): List[Tree] = - for (setter <- mixin.info.decls.filter(setr => setr.isSetter && !wasOneOf(setr, Deferred))) + val mixinSetters = mixin.info.decls.filter { sym => + sym.isSetter && (!wasOneOf(sym, Deferred) || sym.name.is(TraitSetterName)) + } + for (setter <- mixinSetters) yield transformFollowing(DefDef(mkForwarderSym(setter.asTerm), unitLiteral.withSpan(cls.span))) def mixinForwarders(mixin: ClassSymbol): List[Tree] = diff --git a/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala b/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala index 1f3600b471cd..31d59795a180 100644 --- a/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala +++ b/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala @@ -34,7 +34,7 @@ class ResolveSuper extends MiniPhase with IdentityDenotTransformer { thisPhase = override def phaseName: String = ResolveSuper.name override def runsAfter: Set[String] = Set(ElimByName.name, // verified empirically, need to figure out what the reason is. - AugmentScala2Traits.name, + //AugmentScala2Traits.name, PruneErasedDefs.name) // Erased decls make `isCurrent` work incorrectly override def changesMembers: Boolean = true // the phase adds super accessors diff --git a/compiler/src/dotty/tools/dotc/transform/SymUtils.scala b/compiler/src/dotty/tools/dotc/transform/SymUtils.scala index 56bc53b17da6..e04915e8883c 100644 --- a/compiler/src/dotty/tools/dotc/transform/SymUtils.scala +++ b/compiler/src/dotty/tools/dotc/transform/SymUtils.scala @@ -152,6 +152,10 @@ class SymUtils(val self: Symbol) extends AnyVal { if (self.isSetter) self else accessorNamed(self.asTerm.name.setterName) + def traitSetter(using Context): Symbol = + if (self.name.is(TraitSetterName)) self + else accessorNamed(Mixin.traitSetterName(self.asTerm)) + def field(using Context): Symbol = { val thisName = self.name.asTermName val fieldName = From d495ee0c261c17b4e482d0797831125602bcf91c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Fri, 3 Apr 2020 16:14:11 +0200 Subject: [PATCH 02/24] Move the trait super call encoding logic to the back-end. --- .../tools/backend/jvm/BCodeBodyBuilder.scala | 14 +++++-- .../tools/backend/jvm/BCodeHelpers.scala | 12 ++++++ .../tools/backend/jvm/BCodeSkelBuilder.scala | 37 +++++++++++++++++-- compiler/src/dotty/tools/dotc/Compiler.scala | 2 +- 4 files changed, 58 insertions(+), 7 deletions(-) diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala index 2350efca074a..0db9662d6eee 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala @@ -780,6 +780,7 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { val invokeStyle = if (sym.isStaticMember) InvokeStyle.Static else if (sym.is(Private) || sym.isClassConstructor) InvokeStyle.Special + else if (app.hasAttachment(BCodeHelpers.UseInvokeSpecial)) InvokeStyle.Special else InvokeStyle.Virtual if (invokeStyle.hasInstance) genLoadQualifier(fun) @@ -1165,9 +1166,16 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { val isInterface = isEmittedInterface(receiverClass) import InvokeStyle._ if (style == Super) { - // DOTTY: this differ from how super-calls in traits are handled in the scalac backend, - // this is intentional but could change in the future, see https://github.com/lampepfl/dotty/issues/5928 - bc.invokespecial(receiverName, jname, mdescr, isInterface) + if (isInterface && !method.is(JavaDefined)) { + val args = new Array[BType](bmType.argumentTypes.length + 1) + val ownerBType = toTypeKind(method.owner.info) + bmType.argumentTypes.copyToArray(args, 1) + val staticDesc = MethodBType(ownerBType :: bmType.argumentTypes, bmType.returnType).descriptor + val staticName = traitSuperAccessorName(method) + bc.invokestatic(receiverName, staticName, staticDesc, isInterface) + } else { + bc.invokespecial(receiverName, jname, mdescr, isInterface) + } } else { val opc = style match { case Static => Opcodes.INVOKESTATIC diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala b/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala index 5e13a9b4964b..d72cfa7e8ae2 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala @@ -82,6 +82,12 @@ trait BCodeHelpers extends BCodeIdiomatic with BytecodeWriters { } } + final def traitSuperAccessorName(sym: Symbol): String = { + val nameString = sym.javaSimpleName.toString + if (sym.name == nme.TRAIT_CONSTRUCTOR) nameString + else nameString + "$" + } + // ----------------------------------------------------------------------------------------- // finding the least upper bound in agreement with the bytecode verifier (given two internal names handed by ASM) // Background: @@ -950,4 +956,10 @@ object BCodeHelpers { val Super = new InvokeStyle(3) // InvokeSpecial (super calls) } + /** An attachment on Apply nodes indicating that it should be compiled with + * `invokespecial` instead of `invokevirtual`. This is used for static + * forwarders. + */ + val UseInvokeSpecial = new dotc.util.Property.Key[Unit] + } diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala index 8721b5a10828..85c74f52667f 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala @@ -14,9 +14,9 @@ import dotty.tools.dotc.CompilationUnit import dotty.tools.dotc.core.Annotations.Annotation import dotty.tools.dotc.core.Decorators._ import dotty.tools.dotc.core.Flags._ -import dotty.tools.dotc.core.StdNames.str +import dotty.tools.dotc.core.StdNames.{nme, str} import dotty.tools.dotc.core.Symbols._ -import dotty.tools.dotc.core.Types.Type +import dotty.tools.dotc.core.Types.{MethodType, Type} import dotty.tools.dotc.util.Spans._ import dotty.tools.dotc.report @@ -496,7 +496,19 @@ trait BCodeSkelBuilder extends BCodeHelpers { case ValDef(name, tpt, rhs) => () // fields are added in `genPlainClass()`, via `addClassFields()` - case dd: DefDef => genDefDef(dd) + case dd: DefDef => + val sym = dd.symbol + + def needsStaticImplMethod: Boolean = + claszSymbol.isInterface + && !dd.rhs.isEmpty + && !sym.isPrivate + && !sym.isStaticMember + + if needsStaticImplMethod then + genStaticForwarderForDefDef(dd) + + genDefDef(dd) case tree: Template => val body = @@ -537,6 +549,25 @@ trait BCodeSkelBuilder extends BCodeHelpers { } // end of method initJMethod + private def genStaticForwarderForDefDef(dd: DefDef): Unit = + val forwarderDef = makeStaticForwarder(dd, traitSuperAccessorName(dd.symbol)) + genDefDef(forwarderDef) + + private def makeStaticForwarder(dd: DefDef, name: String): DefDef = + val origSym = dd.symbol.asTerm + val info = origSym.info match + case mt: MethodType => + MethodType(nme.SELF :: mt.paramNames, origSym.owner.typeRef :: mt.paramInfos, mt.resType) + val sym = origSym.copy( + name = name.toTermName, + flags = Method | JavaStatic, + info = info + ) + tpd.DefDef(sym.asTerm, { paramss => + val params = paramss.head + tpd.Apply(params.head.select(origSym), params.tail) + .withAttachment(BCodeHelpers.UseInvokeSpecial, ()) + }) def genDefDef(dd: DefDef): Unit = { val rhs = dd.rhs diff --git a/compiler/src/dotty/tools/dotc/Compiler.scala b/compiler/src/dotty/tools/dotc/Compiler.scala index f00d0704262a..138fec14ba22 100644 --- a/compiler/src/dotty/tools/dotc/Compiler.scala +++ b/compiler/src/dotty/tools/dotc/Compiler.scala @@ -110,7 +110,7 @@ class Compiler { // Note: constructors changes decls in transformTemplate, no InfoTransformers should be added after it new FunctionalInterfaces, // Rewrites closures to implement @specialized types of Functions. new Instrumentation) :: // Count closure allocations under -Yinstrument-closures - List(new LinkScala2Impls, // Redirect calls to trait methods defined by Scala 2.x, so that they now go to + 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 From 11de1d366774e1994d686ec6167b588dd1a24dec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Mon, 6 Apr 2020 17:53:59 +0200 Subject: [PATCH 03/24] Do not store to non-existent fields in trait setters. --- compiler/src/dotty/tools/dotc/core/NameOps.scala | 10 +++++++--- compiler/src/dotty/tools/dotc/transform/Memoize.scala | 4 +++- tests/run/final-fields.check | 2 +- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/NameOps.scala b/compiler/src/dotty/tools/dotc/core/NameOps.scala index 07e1d6963b22..e91189fbe02d 100644 --- a/compiler/src/dotty/tools/dotc/core/NameOps.scala +++ b/compiler/src/dotty/tools/dotc/core/NameOps.scala @@ -274,9 +274,13 @@ object NameOps { def setterName: TermName = name.exclude(FieldName) ++ str.SETTER_SUFFIX def getterName: TermName = - name.exclude(FieldName).mapLast(n => - if (n.endsWith(str.SETTER_SUFFIX)) n.take(n.length - str.SETTER_SUFFIX.length).asSimpleName - else n) + if name.is(TraitSetterName) then + val TraitSetterName(_, original) = name + original.getterName + else + name.exclude(FieldName).mapLast(n => + if (n.endsWith(str.SETTER_SUFFIX)) n.take(n.length - str.SETTER_SUFFIX.length).asSimpleName + else n) def fieldName: TermName = if (name.isSetterName) diff --git a/compiler/src/dotty/tools/dotc/transform/Memoize.scala b/compiler/src/dotty/tools/dotc/transform/Memoize.scala index e8a3c564d16a..0f52852efbf6 100644 --- a/compiler/src/dotty/tools/dotc/transform/Memoize.scala +++ b/compiler/src/dotty/tools/dotc/transform/Memoize.scala @@ -12,6 +12,7 @@ import SymUtils._ import Constants._ import ast.Trees._ import MegaPhase._ +import NameKinds.TraitSetterName import NameOps._ import Flags._ import Decorators._ @@ -119,7 +120,8 @@ class Memoize extends MiniPhase with IdentityDenotTransformer { thisPhase => val constantFinalVal = sym.isAllOf(Accessor | Final, butNot = Mutable) && tree.rhs.isInstanceOf[Literal] - if (sym.is(Accessor, butNot = NoFieldNeeded) && !constantFinalVal) { + if (sym.is(Accessor, butNot = NoFieldNeeded) && !constantFinalVal + && (!sym.name.is(TraitSetterName) || sym.getter.is(Accessor, butNot = NoFieldNeeded))) { val field = sym.field.orElse(newField).asTerm def adaptToField(tree: Tree): Tree = diff --git a/tests/run/final-fields.check b/tests/run/final-fields.check index 1b6c826871da..3ebde7d7f735 100644 --- a/tests/run/final-fields.check +++ b/tests/run/final-fields.check @@ -2,6 +2,6 @@ T.f1 T.f2 T.f3 T.f4 -3 2 0 0 +3 2 -3 -4 3 g From de5df367350ce3574f7663587c06b683baf0cd62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Tue, 7 Apr 2020 14:44:27 +0200 Subject: [PATCH 04/24] Fix trait setter to getter lookup. --- compiler/src/dotty/tools/dotc/transform/Memoize.scala | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/Memoize.scala b/compiler/src/dotty/tools/dotc/transform/Memoize.scala index 0f52852efbf6..a9a74a943da2 100644 --- a/compiler/src/dotty/tools/dotc/transform/Memoize.scala +++ b/compiler/src/dotty/tools/dotc/transform/Memoize.scala @@ -118,10 +118,19 @@ class Memoize extends MiniPhase with IdentityDenotTransformer { thisPhase => EmptyTree } + def traitSetterGetter: Symbol = + /* We have to compare SimpleNames here, because the setter name only + * embed the original getter's simple name, not its semantic name. + */ + val getterSimpleName = sym.asTerm.name.getterName + sym.owner.info.decls.find { getter => + getter.is(Accessor) && getter.asTerm.name.toSimpleName == getterSimpleName + } + val constantFinalVal = sym.isAllOf(Accessor | Final, butNot = Mutable) && tree.rhs.isInstanceOf[Literal] if (sym.is(Accessor, butNot = NoFieldNeeded) && !constantFinalVal - && (!sym.name.is(TraitSetterName) || sym.getter.is(Accessor, butNot = NoFieldNeeded))) { + && (!sym.name.is(TraitSetterName) || traitSetterGetter.is(Accessor, butNot = NoFieldNeeded))) { val field = sym.field.orElse(newField).asTerm def adaptToField(tree: Tree): Tree = From ce4f7ba9455f2b2c67bc1e6320bf4ad35128521a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Tue, 7 Apr 2020 16:16:20 +0200 Subject: [PATCH 05/24] Use a fast path when looking up trait setter getters. --- .../src/dotty/tools/dotc/core/NameOps.scala | 10 +++------- .../dotty/tools/dotc/transform/Memoize.scala | 18 +++++++++++++----- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/NameOps.scala b/compiler/src/dotty/tools/dotc/core/NameOps.scala index e91189fbe02d..07e1d6963b22 100644 --- a/compiler/src/dotty/tools/dotc/core/NameOps.scala +++ b/compiler/src/dotty/tools/dotc/core/NameOps.scala @@ -274,13 +274,9 @@ object NameOps { def setterName: TermName = name.exclude(FieldName) ++ str.SETTER_SUFFIX def getterName: TermName = - if name.is(TraitSetterName) then - val TraitSetterName(_, original) = name - original.getterName - else - name.exclude(FieldName).mapLast(n => - if (n.endsWith(str.SETTER_SUFFIX)) n.take(n.length - str.SETTER_SUFFIX.length).asSimpleName - else n) + name.exclude(FieldName).mapLast(n => + if (n.endsWith(str.SETTER_SUFFIX)) n.take(n.length - str.SETTER_SUFFIX.length).asSimpleName + else n) def fieldName: TermName = if (name.isSetterName) diff --git a/compiler/src/dotty/tools/dotc/transform/Memoize.scala b/compiler/src/dotty/tools/dotc/transform/Memoize.scala index a9a74a943da2..e9288bf165f4 100644 --- a/compiler/src/dotty/tools/dotc/transform/Memoize.scala +++ b/compiler/src/dotty/tools/dotc/transform/Memoize.scala @@ -120,12 +120,20 @@ class Memoize extends MiniPhase with IdentityDenotTransformer { thisPhase => def traitSetterGetter: Symbol = /* We have to compare SimpleNames here, because the setter name only - * embed the original getter's simple name, not its semantic name. + * embeds the original getter's simple name, not its semantic name. + * To mitigate the issue, we first try a fast path where we look up the + * simple name itself, which works for public fields. */ - val getterSimpleName = sym.asTerm.name.getterName - sym.owner.info.decls.find { getter => - getter.is(Accessor) && getter.asTerm.name.toSimpleName == getterSimpleName - } + val TraitSetterName(_, original) = sym.name + val getterSimpleName = original.getterName + val ownerInfo = sym.owner.info + val fastPath = ownerInfo.decl(getterSimpleName) + if fastPath.exists then + fastPath.symbol + else + ownerInfo.decls.find { getter => + getter.is(Accessor) && getter.asTerm.name.toSimpleName == getterSimpleName + } val constantFinalVal = sym.isAllOf(Accessor | Final, butNot = Mutable) && tree.rhs.isInstanceOf[Literal] From 3bd5da439ce7c32e5e8e3e71fa713ccd409b8410 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Tue, 23 Jun 2020 16:33:52 +0200 Subject: [PATCH 06/24] Mark dropped private vars without rhs as indeed dropped. Otherwise, their `ValDef`s are removed but they are not removed from their owner class' scope. --- compiler/src/dotty/tools/dotc/transform/Constructors.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/Constructors.scala b/compiler/src/dotty/tools/dotc/transform/Constructors.scala index 40859b1c9b34..cec18bd7b2c9 100644 --- a/compiler/src/dotty/tools/dotc/transform/Constructors.scala +++ b/compiler/src/dotty/tools/dotc/transform/Constructors.scala @@ -204,7 +204,8 @@ class Constructors extends MiniPhase with IdentityDenotTransformer { thisPhase = initFlags = sym.flags &~ Private, owner = constr.symbol).installAfter(thisPhase) constrStats += intoConstr(stat, sym) - } + } else + dropped += sym case stat @ DefDef(name, _, _, tpt, _) if stat.symbol.isGetter && stat.symbol.owner.is(Trait) && !stat.symbol.is(Lazy) => val sym = stat.symbol From e12f1f9e8315ab40416bfe1bfbb677be85c90c60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Wed, 24 Jun 2020 12:14:59 +0200 Subject: [PATCH 07/24] Fix trait encoding for `private var`s in traits. --- .../tools/dotc/transform/Constructors.scala | 17 ++++++++++++++++- .../src/dotty/tools/dotc/transform/Mixin.scala | 3 ++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/Constructors.scala b/compiler/src/dotty/tools/dotc/transform/Constructors.scala index cec18bd7b2c9..36961dd60c4d 100644 --- a/compiler/src/dotty/tools/dotc/transform/Constructors.scala +++ b/compiler/src/dotty/tools/dotc/transform/Constructors.scala @@ -9,6 +9,7 @@ import dotty.tools.dotc.core.StdNames._ import ast._ import Trees._ import Flags._ +import NameOps._ import SymUtils._ import Symbols._ import Decorators._ @@ -211,8 +212,22 @@ class Constructors extends MiniPhase with IdentityDenotTransformer { thisPhase = val sym = stat.symbol assert(isRetained(sym), sym) if (!stat.rhs.isEmpty && !isWildcardArg(stat.rhs)) + /* !!! This should really just be `sym.setter`. However, if we do that, we'll miss + * setters for mixed in `private var`s. Even though the scope clearly contains the + * setter symbol with the correct Name structure (since the `find` finds it), + * `.decl(setterName)` used by `.setter` through `.accessorNamed` will *not* find it. + * Could it be that the hash table of the `Scope` is corrupted? + * We still try `sym.setter` first as an optimization, since it will work for all + * public vars in traits and all (public or private) vars in classes. + */ + val symSetter = + if sym.setter.exists then + sym.setter + else + val setterName = sym.asTerm.name.setterName + sym.owner.info.decls.find(d => d.is(Accessor) && d.name == setterName) val setter = - if (sym.setter.exists) sym.setter + if (symSetter.exists) symSetter else sym.accessorNamed(Mixin.traitSetterName(sym.asTerm)) constrStats += Apply(ref(setter), intoConstr(stat.rhs, sym).withSpan(stat.span) :: Nil) clsStats += cpy.DefDef(stat)(rhs = EmptyTree) diff --git a/compiler/src/dotty/tools/dotc/transform/Mixin.scala b/compiler/src/dotty/tools/dotc/transform/Mixin.scala index 7ab957f77676..d692484dd059 100644 --- a/compiler/src/dotty/tools/dotc/transform/Mixin.scala +++ b/compiler/src/dotty/tools/dotc/transform/Mixin.scala @@ -175,7 +175,8 @@ class Mixin extends MiniPhase with SymTransformer { thisPhase => atPhase(thisPhase) { sym.isOneOf(flags) } private def needsTraitSetter(sym: Symbol)(using Context): Boolean = - sym.isGetter && !wasOneOf(sym, DeferredOrLazy | ParamAccessor) && !sym.setter.exists + sym.isGetter && !wasOneOf(sym, DeferredOrLazy | ParamAccessor) + && atPhase(thisPhase) { !sym.setter.exists } && !sym.info.resultType.isInstanceOf[ConstantType] private def makeTraitSetter(getter: TermSymbol)(using Context): Symbol = From d68245537079f0731ab1381a744a30d00ebd1e31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Wed, 24 Jun 2020 14:53:40 +0200 Subject: [PATCH 08/24] Adapt one check file for new emitted static methods. --- tests/run/t7932.check | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/run/t7932.check b/tests/run/t7932.check index 823f1183f0e0..5d2d50162801 100644 --- a/tests/run/t7932.check +++ b/tests/run/t7932.check @@ -2,5 +2,9 @@ public Category C.category() public Category C.category1() public default Category M1.category() public default Category M1.category1() +public static Category M1.category$(M1) +public static Category M1.category1$(M1) public default Category M2.category() public default Category M2.category1() +public static Category M2.category$(M2) +public static Category M2.category1$(M2) From d7bea54b85bb22d32ed17e2578aa9509a4074336 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Wed, 24 Jun 2020 15:00:26 +0200 Subject: [PATCH 09/24] Modules in traits always get an outer pointer. They will be instantiated in the classes that mix in their owner trait. --- compiler/src/dotty/tools/dotc/transform/ExplicitOuter.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/ExplicitOuter.scala b/compiler/src/dotty/tools/dotc/transform/ExplicitOuter.scala index 5ab591ca1e68..877c964ed65c 100644 --- a/compiler/src/dotty/tools/dotc/transform/ExplicitOuter.scala +++ b/compiler/src/dotty/tools/dotc/transform/ExplicitOuter.scala @@ -215,9 +215,9 @@ object ExplicitOuter { /** Class is always instantiated in the compilation unit where it is defined */ private def hasLocalInstantiation(cls: ClassSymbol)(using Context): Boolean = - // scala2x modules always take an outer pointer(as of 2.11) - // dotty modules are always locally instantiated - cls.owner.isTerm || cls.is(Private) || cls.is(Module, butNot = Scala2x) + // Modules are normally locally instantiated, except if they are declared in a trait, + // in which case they will be instantiated in the classes that mix in the trait. + cls.owner.isTerm || cls.is(Private, butNot = Module) || (cls.is(Module) && !cls.owner.is(Trait)) /** The outer parameter accessor of cass `cls` */ private def outerParamAccessor(cls: ClassSymbol)(using Context): TermSymbol = From 84b230ae3fb69635a3c56849c7112221ff211199 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Wed, 24 Jun 2020 15:28:36 +0200 Subject: [PATCH 10/24] Make module classes in traits non-private in Mixin. They will be instantiated in classes that mix in their owner trait, so they must be visible there. --- compiler/src/dotty/tools/dotc/transform/Mixin.scala | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/compiler/src/dotty/tools/dotc/transform/Mixin.scala b/compiler/src/dotty/tools/dotc/transform/Mixin.scala index d692484dd059..078a6b8ee41b 100644 --- a/compiler/src/dotty/tools/dotc/transform/Mixin.scala +++ b/compiler/src/dotty/tools/dotc/transform/Mixin.scala @@ -134,6 +134,10 @@ class Mixin extends MiniPhase with SymTransformer { thisPhase => else sym.copySymDenotation(initFlags = sym.flags &~ ParamAccessor | Deferred) sym1.ensureNotPrivate } + else if sym.isAllOf(ModuleClass | Private) && sym.owner.is(Trait) then + // modules in trait will be instantiated in the classes mixing in the trait; they must be made non-private + // do not use ensureNotPrivate because the `name` must not be expanded in this case + sym.copySymDenotation(initFlags = sym.flags &~ Private) else if (sym.isConstructor && sym.owner.is(Trait)) sym.copySymDenotation( name = nme.TRAIT_CONSTRUCTOR, From 288fa1d5db478c5e88276d9990a9a543db9d1b9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Wed, 24 Jun 2020 15:49:09 +0200 Subject: [PATCH 11/24] Adapt the expected result of the jUnitForwarders test. --- tests/run/junitForwarders/C_1.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/run/junitForwarders/C_1.scala b/tests/run/junitForwarders/C_1.scala index 82ff9d3600c4..04bd263bf791 100644 --- a/tests/run/junitForwarders/C_1.scala +++ b/tests/run/junitForwarders/C_1.scala @@ -17,7 +17,7 @@ object Test extends App { assert(s == e, s"found: $s\nexpected: $e") } check(classOf[C], "foo - @org.junit.Test()") - // TODO scala-dev#213: should `foo$` really carry the @Test annotation? - check(classOf[T], "$init$ - ;foo - @org.junit.Test()") - check(classOf[U], "bar - @org.junit.Test()") + // scala/scala-dev#213, scala/scala#5570: `foo$` should not have the @Test annotation + check(classOf[T], "$init$ - ;$init$ - ;foo - @org.junit.Test();foo$ - ") + check(classOf[U], "bar - @org.junit.Test();bar$ - ") } From 2eb743c312c83156b5a45758cbb5d14af8cac579 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Wed, 24 Jun 2020 16:08:19 +0200 Subject: [PATCH 12/24] Restore AugmentScala2Traits, but limit it to super accessors. --- compiler/src/dotty/tools/dotc/Compiler.scala | 2 +- .../dotc/transform/AugmentScala2Traits.scala | 25 ++++--------------- .../tools/dotc/transform/ResolveSuper.scala | 2 +- 3 files changed, 7 insertions(+), 22 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/Compiler.scala b/compiler/src/dotty/tools/dotc/Compiler.scala index 138fec14ba22..2b16405cdf32 100644 --- a/compiler/src/dotty/tools/dotc/Compiler.scala +++ b/compiler/src/dotty/tools/dotc/Compiler.scala @@ -86,7 +86,7 @@ class Compiler { new LiftTry, // Put try expressions that might execute on non-empty stacks into their own methods new CollectNullableFields, // Collect fields that can be nulled out after use in lazy initialization new ElimOuterSelect, // Expand outer selections - //new AugmentScala2Traits, // Augments Scala2 traits with additional members needed for mixin composition. + new AugmentScala2Traits, // Augments Scala2 traits so that super accessors are made non-private new ResolveSuper, // Implement super accessors new FunctionXXLForwarders, // Add forwarders for FunctionXXL apply method new ParamForwarding, // Add forwarders for aliases of superclass parameters diff --git a/compiler/src/dotty/tools/dotc/transform/AugmentScala2Traits.scala b/compiler/src/dotty/tools/dotc/transform/AugmentScala2Traits.scala index af74ba9f1de6..6a12aa54d0da 100644 --- a/compiler/src/dotty/tools/dotc/transform/AugmentScala2Traits.scala +++ b/compiler/src/dotty/tools/dotc/transform/AugmentScala2Traits.scala @@ -20,14 +20,12 @@ object AugmentScala2Traits { val name: String = "augmentScala2Traits" } -/** This phase augments Scala2 traits with additional members needed for mixin composition. +/** This phase augments Scala2 traits to fix up super accessors. * - * These symbols would have been added between Unpickling and Mixin in the Scala2 pipeline. + * Strangely, Scala 2 super accessors are pickled as private, but are compiled as public expanded. + * In this phase we expand them and make them non-private, so that `ResolveSuper` does something meaningful. * - * Specifically, we: - * - Add trait setters for vals defined in traits. - * - Expand the names of all private getters and setters as well as super accessors in the trait and make - * not-private. + * TODO Should we merge this into `ResolveSuper` at this point? */ class AugmentScala2Traits extends MiniPhase with IdentityDenotTransformer { thisPhase => import ast.tpd._ @@ -47,21 +45,8 @@ class AugmentScala2Traits extends MiniPhase with IdentityDenotTransformer { this } private def augmentScala2Trait(mixin: ClassSymbol)(using Context): Unit = { - def traitSetter(getter: TermSymbol) = - getter.copy( - name = getter.ensureNotPrivate.name - .expandedName(getter.owner, TraitSetterName) - .asTermName.setterName, - flags = Method | Accessor, - info = MethodType(getter.info.resultType :: Nil, defn.UnitType)) - for (sym <- mixin.info.decls) { - if (sym.isGetter && !sym.isOneOf(DeferredOrLazy) && !sym.setter.exists && - !sym.info.resultType.isInstanceOf[ConstantType]) - traitSetter(sym.asTerm).enteredAfter(thisPhase) - if ((sym.isAllOf(PrivateAccessor) && !sym.name.is(ExpandedName) && - (sym.isGetter || sym.isSetter)) // strangely, Scala 2 fields are also methods that have Accessor set. - || sym.isSuperAccessor) // scala2 superaccessors are pickled as private, but are compiled as public expanded + if (sym.isSuperAccessor) sym.ensureNotPrivate.installAfter(thisPhase) } mixin.setFlagFrom(thisPhase, Scala2xPartiallyAugmented) diff --git a/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala b/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala index 31d59795a180..1f3600b471cd 100644 --- a/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala +++ b/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala @@ -34,7 +34,7 @@ class ResolveSuper extends MiniPhase with IdentityDenotTransformer { thisPhase = override def phaseName: String = ResolveSuper.name override def runsAfter: Set[String] = Set(ElimByName.name, // verified empirically, need to figure out what the reason is. - //AugmentScala2Traits.name, + AugmentScala2Traits.name, PruneErasedDefs.name) // Erased decls make `isCurrent` work incorrectly override def changesMembers: Boolean = true // the phase adds super accessors From 3f999152a3ff26c7ee8562b4d33ce23e26675e55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Wed, 24 Jun 2020 16:43:47 +0200 Subject: [PATCH 13/24] Reset the `inline` flag for accessors in traits at Mixin. This might not be the best way to fix t4753.scala, but this is the last run test and I'll take anything that makes the tests pass at this point. --- compiler/src/dotty/tools/dotc/transform/Mixin.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/Mixin.scala b/compiler/src/dotty/tools/dotc/transform/Mixin.scala index 078a6b8ee41b..5c59e1a0eac8 100644 --- a/compiler/src/dotty/tools/dotc/transform/Mixin.scala +++ b/compiler/src/dotty/tools/dotc/transform/Mixin.scala @@ -131,7 +131,7 @@ class Mixin extends MiniPhase with SymTransformer { thisPhase => if (sym.is(Accessor, butNot = Deferred) && sym.owner.is(Trait)) { val sym1 = if (sym.is(Lazy)) sym - else sym.copySymDenotation(initFlags = sym.flags &~ ParamAccessor | Deferred) + else sym.copySymDenotation(initFlags = sym.flags &~ (ParamAccessor | Inline) | Deferred) sym1.ensureNotPrivate } else if sym.isAllOf(ModuleClass | Private) && sym.owner.is(Trait) then From d990e6421a41f38365764e6fa6ac2334a9cee8b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Wed, 24 Jun 2020 17:02:35 +0200 Subject: [PATCH 14/24] Cleanup, including removing dead code. --- compiler/src/dotty/tools/dotc/Compiler.scala | 3 +- .../src/dotty/tools/dotc/core/Flags.scala | 7 +- .../dotc/transform/AugmentScala2Traits.scala | 4 +- .../dotc/transform/LinkScala2Impls.scala | 129 ------------------ .../dotty/tools/dotc/transform/Mixin.scala | 15 -- docs/docs/internals/overall-structure.md | 5 +- 6 files changed, 8 insertions(+), 155 deletions(-) delete mode 100644 compiler/src/dotty/tools/dotc/transform/LinkScala2Impls.scala diff --git a/compiler/src/dotty/tools/dotc/Compiler.scala b/compiler/src/dotty/tools/dotc/Compiler.scala index 2b16405cdf32..f5a2bb4fec81 100644 --- a/compiler/src/dotty/tools/dotc/Compiler.scala +++ b/compiler/src/dotty/tools/dotc/Compiler.scala @@ -110,8 +110,7 @@ class Compiler { // Note: constructors changes decls in transformTemplate, no InfoTransformers should be added after it new FunctionalInterfaces, // Rewrites closures to implement @specialized types of Functions. new Instrumentation) :: // Count closure allocations under -Yinstrument-closures - 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 + List(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 new CountOuterAccesses) :: // Identify outer accessors that can be dropped diff --git a/compiler/src/dotty/tools/dotc/core/Flags.scala b/compiler/src/dotty/tools/dotc/core/Flags.scala index 2c46f2c95924..8fc9f1920b3c 100644 --- a/compiler/src/dotty/tools/dotc/core/Flags.scala +++ b/compiler/src/dotty/tools/dotc/core/Flags.scala @@ -380,11 +380,10 @@ object Flags { /** A module variable (Scala 2.x only) * / - * A Scala 2.x trait that has been partially augmented. - * This is set in `AugmentScala2Trait` and reset in `LinkScala2Impls` - * when the trait is fully augmented. + * A Scala 2.x trait that has been augmented. + * This is set in `AugmentScala2Trait` when the trait is augmented. */ - val (_, Scala2ModuleVar @ _, Scala2xPartiallyAugmented @ _) = newFlags(57, "", "") + val (_, Scala2ModuleVar @ _, Scala2xAugmented @ _) = newFlags(57, "", "") /** A macro */ val (Macro @ _, _, _) = newFlags(58, "") diff --git a/compiler/src/dotty/tools/dotc/transform/AugmentScala2Traits.scala b/compiler/src/dotty/tools/dotc/transform/AugmentScala2Traits.scala index 6a12aa54d0da..afbd76f0fd43 100644 --- a/compiler/src/dotty/tools/dotc/transform/AugmentScala2Traits.scala +++ b/compiler/src/dotty/tools/dotc/transform/AugmentScala2Traits.scala @@ -38,7 +38,7 @@ class AugmentScala2Traits extends MiniPhase with IdentityDenotTransformer { this val cls = impl.symbol.owner.asClass for (mixin <- cls.mixins) { val erasedMixin = TypeErasure.normalizeClass(mixin) - if (erasedMixin.is(Scala2x) && !erasedMixin.is(Scala2xPartiallyAugmented)) + if (erasedMixin.is(Scala2x) && !erasedMixin.is(Scala2xAugmented)) augmentScala2Trait(erasedMixin) } impl @@ -49,6 +49,6 @@ class AugmentScala2Traits extends MiniPhase with IdentityDenotTransformer { this if (sym.isSuperAccessor) sym.ensureNotPrivate.installAfter(thisPhase) } - mixin.setFlagFrom(thisPhase, Scala2xPartiallyAugmented) + mixin.setFlagFrom(thisPhase, Scala2xAugmented) } } diff --git a/compiler/src/dotty/tools/dotc/transform/LinkScala2Impls.scala b/compiler/src/dotty/tools/dotc/transform/LinkScala2Impls.scala deleted file mode 100644 index e83607abc8b5..000000000000 --- a/compiler/src/dotty/tools/dotc/transform/LinkScala2Impls.scala +++ /dev/null @@ -1,129 +0,0 @@ -package dotty.tools.dotc -package transform - -import core._ -import MegaPhase._ -import Contexts._ -import Flags._ -import SymUtils._ -import Symbols._ -import Types._ -import Decorators._ -import DenotTransformers._ -import Names._ -import StdNames._ -import SymDenotations._ -import ast.Trees._ -import NameKinds.ImplMethName - -/** Rewrite calls - * - * super[M].f(args) - * - * where M is a trait implemented by the current class to - * - * M.f$(this, args) - * - * where f$ is a static member of M. - * - * Also generates said static members, as forwarders to the normal methods. - */ -class LinkScala2Impls extends MiniPhase with SymTransformer { thisPhase => - import ast.tpd._ - - override def phaseName: String = "linkScala2Impls" - override def changesMembers: Boolean = true - - override def runsAfterGroupsOf: Set[String] = Set(Mixin.name) - // Adds as a side effect static members to traits which can confuse Mixin, - // that's why it is runsAfterGroupOf - - override def transformSym(sym: SymDenotation)(using Context): SymDenotation = - if sym.is(Trait) && !ctx.settings.scalajs.value then - val mixin = sym.asClass.classSymbol - val ops = new MixinOps(mixin, thisPhase) - import ops._ - - def newImpl(meth: TermSymbol): Symbol = - def staticInfo(tp: Type) = tp match - case mt: MethodType => - MethodType(nme.SELF :: mt.paramNames, mixin.typeRef :: mt.paramInfos, mt.resType) - val mold = meth.ensureNotPrivate - meth.copy( - owner = mixin, - name = staticForwarderName(mold.name.asTermName), - flags = Method | JavaStatic, - info = staticInfo(mold.info) - ) - - val classInfo = sym.asClass.classInfo - val decls1 = classInfo.decls.cloneScope - var modified: Boolean = false - for (meth <- classInfo.decls) - if needsStaticForwarder(meth) then - decls1.enter(newImpl(meth.asTerm)) - modified = true - if modified then - sym.copySymDenotation( - info = classInfo.derivedClassInfo(decls = decls1)) - else - sym - else - sym - end transformSym - - private def needsStaticForwarder(sym: Symbol)(using Context): Boolean = - sym.is(Method, butNot = Deferred) - - private def staticForwarderName(name: TermName)(using Context): TermName = - if name == nme.TRAIT_CONSTRUCTOR then name - else ImplMethName(name) - - override def transformTemplate(impl: Template)(using Context): Tree = - val sym = impl.symbol.owner.asClass - if sym.is(Trait) && !ctx.settings.scalajs.value then - val newConstr :: newBody = (impl.constr :: impl.body).flatMap { - case stat: DefDef if needsStaticForwarder(stat.symbol) => - val meth = stat.symbol - val staticForwarderDef = DefDef(implMethod(meth).asTerm, { paramss => - val params = paramss.head - /* FIXME This is wrong. We are emitting a virtual call to `meth`, - * but we must instead emit an `invokespecial` so that it is not - * virtual. However, I don't know how to represent an invokevirtual - * call on a non-`this` receiver. My best attempt is the following, - * but that throws an exception when type-assigning the Super node: - */ - //Apply(Super(params.head, sym.name.asTypeName, sym).select(meth), params.tail) - Apply(params.head.select(meth), params.tail) - }) - stat :: transformFollowing(staticForwarderDef) :: Nil - case stat => - stat :: Nil - } - cpy.Template(impl)(constr = newConstr.asInstanceOf[DefDef], body = newBody) - else - impl - - override def transformApply(app: Apply)(using Context): Tree = { - def currentClass = ctx.owner.enclosingClass.asClass - app match { - case Apply(sel @ Select(Super(_, _), _), args) - if sel.symbol.owner.is(Trait) && currentClass.mixins.contains(sel.symbol.owner) && !ctx.settings.scalajs.value => - val impl = implMethod(sel.symbol) - if (impl.exists) Apply(ref(impl), This(currentClass) :: args).withSpan(app.span) - else app // could have been an abstract method in a trait linked to from a super constructor - case _ => - app - } - } - - /** The implementation method of a super call or implementation class target */ - private def implMethod(meth: Symbol)(using Context): Symbol = - val cls = meth.owner - if meth.name == nme.TRAIT_CONSTRUCTOR then - cls.info.decl(nme.TRAIT_CONSTRUCTOR).suchThat(_.is(JavaStatic)).symbol - else - cls.info.decl(staticForwarderName(meth.name.asTermName)) - .suchThat(c => FullParameterization.memberSignature(c.info) == meth.signature) - .symbol -} diff --git a/compiler/src/dotty/tools/dotc/transform/Mixin.scala b/compiler/src/dotty/tools/dotc/transform/Mixin.scala index 5c59e1a0eac8..341ebb954987 100644 --- a/compiler/src/dotty/tools/dotc/transform/Mixin.scala +++ b/compiler/src/dotty/tools/dotc/transform/Mixin.scala @@ -160,21 +160,6 @@ class Mixin extends MiniPhase with SymTransformer { thisPhase => sym end transformSym - private def initializer(sym: Symbol)(using Context): TermSymbol = { - if (sym.is(Lazy)) sym - else { - val initName = InitializerName(sym.name.asTermName) - sym.owner.info.decl(initName).symbol - .orElse( - newSymbol( - sym.owner, - initName, - Protected | Synthetic | Method, - sym.info, - coord = sym.coord).enteredAfter(thisPhase)) - } - }.asTerm - private def wasOneOf(sym: Symbol, flags: FlagSet)(using Context): Boolean = atPhase(thisPhase) { sym.isOneOf(flags) } diff --git a/docs/docs/internals/overall-structure.md b/docs/docs/internals/overall-structure.md index f49942df3aee..6b3765471be1 100644 --- a/docs/docs/internals/overall-structure.md +++ b/docs/docs/internals/overall-structure.md @@ -146,7 +146,7 @@ phases. The current list of phases is specified in class [Compiler] as follows: new ElimByName, // Expand by-name parameter references new CollectNullableFields, // Collect fields that can be nulled out after use in lazy initialization new ElimOuterSelect, // Expand outer selections - new AugmentScala2Traits, // Augments Scala2 traits with additional members needed for mixin composition. + new AugmentScala2Traits, // Augments Scala2 traits so that super accessors are made non-private new ResolveSuper, // Implement super accessors new FunctionXXLForwarders, // Add forwarders for FunctionXXL apply method new TupleOptimizations, // Optimize generic operations on tuples @@ -168,8 +168,7 @@ phases. The current list of phases is specified in class [Compiler] as follows: new Instrumentation, // Count closure allocations under -Yinstrument-closures new GetClass, // Rewrites getClass calls on primitive types. new LiftTry) :: // Put try expressions that might execute on non-empty stacks into their own methods their implementations - 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 + List(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 From 29a561b8995cb86da888d8e607cd1f4252d6a747 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Thu, 25 Jun 2020 10:29:27 +0200 Subject: [PATCH 15/24] Adapt the expect result in ByteCodeTests. --- compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala b/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala index 15d88d129083..ebf6d9f09d5d 100644 --- a/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala +++ b/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala @@ -856,8 +856,7 @@ class TestBCode extends DottyBytecodeTest { checkBCode(List(invocationReceiversTestCode.definitions("Object"))) { dir => val c1 = loadClassNode(dir.lookupName("C1.class", directory = false).input) val c2 = loadClassNode(dir.lookupName("C2.class", directory = false).input) - // Scala 2 uses "invokestatic T.clone$" here, see https://github.com/lampepfl/dotty/issues/5928 - assertSameCode(getMethod(c1, "clone"), List(VarOp(ALOAD, 0), Invoke(INVOKESPECIAL, "T", "clone", "()Ljava/lang/Object;", true), Op(ARETURN))) + assertSameCode(getMethod(c1, "clone"), List(VarOp(ALOAD, 0), Invoke(INVOKESTATIC, "T", "clone$", "(LT;)Ljava/lang/Object;", true), Op(ARETURN))) assertInvoke(getMethod(c1, "f1"), "T", "clone") assertInvoke(getMethod(c1, "f2"), "T", "clone") assertInvoke(getMethod(c1, "f3"), "C1", "clone") From cb84d0443071eaa6b2f621cda07f87816c844b40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Thu, 25 Jun 2020 12:28:55 +0200 Subject: [PATCH 16/24] Break cycles in transformSym. --- compiler/src/dotty/tools/dotc/transform/Mixin.scala | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/Mixin.scala b/compiler/src/dotty/tools/dotc/transform/Mixin.scala index 341ebb954987..eb2729b27a17 100644 --- a/compiler/src/dotty/tools/dotc/transform/Mixin.scala +++ b/compiler/src/dotty/tools/dotc/transform/Mixin.scala @@ -128,21 +128,23 @@ class Mixin extends MiniPhase with SymTransformer { thisPhase => override def changesMembers: Boolean = true // the phase adds implementions of mixin accessors override def transformSym(sym: SymDenotation)(using Context): SymDenotation = - if (sym.is(Accessor, butNot = Deferred) && sym.owner.is(Trait)) { + def ownerIsTrait: Boolean = wasOneOf(sym.owner, Trait) + + if (sym.is(Accessor, butNot = Deferred) && ownerIsTrait) { val sym1 = if (sym.is(Lazy)) sym else sym.copySymDenotation(initFlags = sym.flags &~ (ParamAccessor | Inline) | Deferred) sym1.ensureNotPrivate } - else if sym.isAllOf(ModuleClass | Private) && sym.owner.is(Trait) then + else if sym.isAllOf(ModuleClass | Private) && ownerIsTrait then // modules in trait will be instantiated in the classes mixing in the trait; they must be made non-private // do not use ensureNotPrivate because the `name` must not be expanded in this case sym.copySymDenotation(initFlags = sym.flags &~ Private) - else if (sym.isConstructor && sym.owner.is(Trait)) + else if (sym.isConstructor && ownerIsTrait) sym.copySymDenotation( name = nme.TRAIT_CONSTRUCTOR, info = MethodType(Nil, sym.info.resultType)) - else if sym.is(Trait) then + else if sym.is(Trait, butNot = JavaDefined) then val classInfo = sym.asClass.classInfo val decls1 = classInfo.decls.cloneScope var modified: Boolean = false From 80db02159ba2e78c629fe915a46de0b174624d29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Thu, 25 Jun 2020 16:23:37 +0200 Subject: [PATCH 17/24] Break more cycles in transformSym. --- compiler/src/dotty/tools/dotc/transform/Mixin.scala | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/Mixin.scala b/compiler/src/dotty/tools/dotc/transform/Mixin.scala index eb2729b27a17..7e34bca4e58e 100644 --- a/compiler/src/dotty/tools/dotc/transform/Mixin.scala +++ b/compiler/src/dotty/tools/dotc/transform/Mixin.scala @@ -148,9 +148,10 @@ class Mixin extends MiniPhase with SymTransformer { thisPhase => val classInfo = sym.asClass.classInfo val decls1 = classInfo.decls.cloneScope var modified: Boolean = false - for (getter <- classInfo.decls) - if needsTraitSetter(getter) then - val setter = makeTraitSetter(getter.asTerm) + for (decl <- classInfo.decls) + // !decl.isClass avoids forcing nested traits, preventing cycles + if !decl.isClass && needsTraitSetter(decl) then + val setter = makeTraitSetter(decl.asTerm) decls1.enter(setter) modified = true if modified then From a7715749abf322935e45fce21f64bfaa3fb679ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Mon, 29 Jun 2020 15:27:03 +0200 Subject: [PATCH 18/24] Ignore bridges when looking for the getter of a trait setter. --- .../dotty/tools/dotc/transform/Memoize.scala | 4 ++-- tests/run/traitValBridge.scala | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) create mode 100644 tests/run/traitValBridge.scala diff --git a/compiler/src/dotty/tools/dotc/transform/Memoize.scala b/compiler/src/dotty/tools/dotc/transform/Memoize.scala index e9288bf165f4..155a44904514 100644 --- a/compiler/src/dotty/tools/dotc/transform/Memoize.scala +++ b/compiler/src/dotty/tools/dotc/transform/Memoize.scala @@ -127,12 +127,12 @@ class Memoize extends MiniPhase with IdentityDenotTransformer { thisPhase => val TraitSetterName(_, original) = sym.name val getterSimpleName = original.getterName val ownerInfo = sym.owner.info - val fastPath = ownerInfo.decl(getterSimpleName) + val fastPath = ownerInfo.findDecl(getterSimpleName, excluded = Bridge) if fastPath.exists then fastPath.symbol else ownerInfo.decls.find { getter => - getter.is(Accessor) && getter.asTerm.name.toSimpleName == getterSimpleName + getter.is(Accessor, butNot = Bridge) && getter.asTerm.name.toSimpleName == getterSimpleName } val constantFinalVal = sym.isAllOf(Accessor | Final, butNot = Mutable) && tree.rhs.isInstanceOf[Literal] diff --git a/tests/run/traitValBridge.scala b/tests/run/traitValBridge.scala new file mode 100644 index 000000000000..a1f14b65c7ec --- /dev/null +++ b/tests/run/traitValBridge.scala @@ -0,0 +1,18 @@ +class Foo + +trait GenericTrait[A] { + def bar: A +} + +trait ConcreteTrait extends GenericTrait[Foo] { + val bar: Foo = new Foo +} + +class ConcreteClass extends ConcreteTrait + +object Test { + def main(args: Array[String]): Unit = { + val obj = new ConcreteClass + assert(obj.bar != null) + } +} From 1128a98617e078ec26952fe8243509257c8e3ec5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Thu, 16 Jul 2020 11:42:50 +0200 Subject: [PATCH 19/24] Fix handling of `final val`s after rebase on top of #9261. --- .../dotty/tools/dotc/transform/Memoize.scala | 92 +++++++++---------- 1 file changed, 41 insertions(+), 51 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/Memoize.scala b/compiler/src/dotty/tools/dotc/transform/Memoize.scala index 155a44904514..adee622fbf16 100644 --- a/compiler/src/dotty/tools/dotc/transform/Memoize.scala +++ b/compiler/src/dotty/tools/dotc/transform/Memoize.scala @@ -118,61 +118,51 @@ class Memoize extends MiniPhase with IdentityDenotTransformer { thisPhase => EmptyTree } - def traitSetterGetter: Symbol = - /* We have to compare SimpleNames here, because the setter name only - * embeds the original getter's simple name, not its semantic name. - * To mitigate the issue, we first try a fast path where we look up the - * simple name itself, which works for public fields. - */ - val TraitSetterName(_, original) = sym.name - val getterSimpleName = original.getterName - val ownerInfo = sym.owner.info - val fastPath = ownerInfo.findDecl(getterSimpleName, excluded = Bridge) - if fastPath.exists then - fastPath.symbol - else - ownerInfo.decls.find { getter => - getter.is(Accessor, butNot = Bridge) && getter.asTerm.name.toSimpleName == getterSimpleName - } - - val constantFinalVal = sym.isAllOf(Accessor | Final, butNot = Mutable) && tree.rhs.isInstanceOf[Literal] - - if (sym.is(Accessor, butNot = NoFieldNeeded) && !constantFinalVal - && (!sym.name.is(TraitSetterName) || traitSetterGetter.is(Accessor, butNot = NoFieldNeeded))) { - val field = sym.field.orElse(newField).asTerm - - def adaptToField(tree: Tree): Tree = + if sym.is(Accessor, butNot = NoFieldNeeded) then + def adaptToField(field: Symbol, tree: Tree): Tree = if (tree.isEmpty) tree else tree.ensureConforms(field.info.widen) - def isErasableBottomField(cls: Symbol): Boolean = + def isErasableBottomField(field: Symbol, cls: Symbol): Boolean = !field.isVolatile && ((cls eq defn.NothingClass) || (cls eq defn.NullClass) || (cls eq defn.BoxedUnitClass)) - if (sym.isGetter) { - var rhs = tree.rhs.changeOwnerAfter(sym, field, thisPhase) - if (isWildcardArg(rhs)) rhs = EmptyTree - val fieldDef = transformFollowing(ValDef(field, adaptToField(rhs))) - val rhsClass = tree.tpt.tpe.widenDealias.classSymbol - val getterRhs = - if (isErasableBottomField(rhsClass)) erasedBottomTree(rhsClass) - else transformFollowingDeep(ref(field))(using ctx.withOwner(sym)) - val getterDef = cpy.DefDef(tree)(rhs = getterRhs) - addAnnotations(fieldDef.denot) - removeUnwantedAnnotations(sym) - Thicket(fieldDef, getterDef) - } - else if (sym.isSetter) { + if sym.isGetter then + val constantFinalVal = sym.isAllOf(Accessor | Final, butNot = Mutable) && tree.rhs.isInstanceOf[Literal] + if constantFinalVal then + // constant final vals do not need to be transformed at all, and do not need a field + tree + else + val field = newField.asTerm + var rhs = tree.rhs.changeOwnerAfter(sym, field, thisPhase) + if (isWildcardArg(rhs)) rhs = EmptyTree + val fieldDef = transformFollowing(ValDef(field, adaptToField(field, rhs))) + val rhsClass = tree.tpt.tpe.widenDealias.classSymbol + val getterRhs = + if isErasableBottomField(field, rhsClass) then erasedBottomTree(rhsClass) + else transformFollowingDeep(ref(field))(using ctx.withOwner(sym)) + val getterDef = cpy.DefDef(tree)(rhs = getterRhs) + addAnnotations(fieldDef.denot) + removeUnwantedAnnotations(sym) + Thicket(fieldDef, getterDef) + else if sym.isSetter then if (!sym.is(ParamAccessor)) { val Literal(Constant(())) = tree.rhs } // This is intended as an assertion - field.setFlag(Mutable) // Necessary for vals mixed in from Scala2 traits - val initializer = - if (isErasableBottomField(tree.vparamss.head.head.tpt.tpe.classSymbol)) Literal(Constant(())) - else Assign(ref(field), adaptToField(ref(tree.vparamss.head.head.symbol))) - val setterDef = cpy.DefDef(tree)(rhs = transformFollowingDeep(initializer)(using ctx.withOwner(sym))) - removeUnwantedAnnotations(sym) - setterDef - } - else tree // curiously, some accessors from Scala2 have ' ' suffixes. They count as - // neither getters nor setters - } - else tree + val field = sym.field + if !field.exists then + // When transforming the getter, we determined that no field was needed. + // In that case we can keep the setter as is, with a () rhs. + tree + else + field.setFlag(Mutable) // Necessary for vals mixed in from traits + val initializer = + if (isErasableBottomField(field, tree.vparamss.head.head.tpt.tpe.classSymbol)) Literal(Constant(())) + else Assign(ref(field), adaptToField(field, ref(tree.vparamss.head.head.symbol))) + val setterDef = cpy.DefDef(tree)(rhs = transformFollowingDeep(initializer)(using ctx.withOwner(sym))) + removeUnwantedAnnotations(sym) + setterDef + else + // Curiously, some accessors from Scala2 have ' ' suffixes. + // They count as neither getters nor setters. + tree + else + tree } } From 44de114f501403f0e5008b82da5ce7a897b9fcb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Thu, 16 Jul 2020 17:41:04 +0200 Subject: [PATCH 20/24] Support param accessors that override concrete trait vals. --- .../dotty/tools/dotc/transform/Memoize.scala | 6 ++++++ .../run/traitValOverriddenByParamAccessor.scala | 17 +++++++++++++++++ 2 files changed, 23 insertions(+) create mode 100644 tests/run/traitValOverriddenByParamAccessor.scala diff --git a/compiler/src/dotty/tools/dotc/transform/Memoize.scala b/compiler/src/dotty/tools/dotc/transform/Memoize.scala index adee622fbf16..0113b06c1618 100644 --- a/compiler/src/dotty/tools/dotc/transform/Memoize.scala +++ b/compiler/src/dotty/tools/dotc/transform/Memoize.scala @@ -150,6 +150,12 @@ class Memoize extends MiniPhase with IdentityDenotTransformer { thisPhase => // When transforming the getter, we determined that no field was needed. // In that case we can keep the setter as is, with a () rhs. tree + else if field.getter.is(ParamAccessor, butNot = Mutable) then + // This is a trait setter (because not Mutable) for a param accessor. + // We must keep the () rhs of the trait setter, otherwise the value + // inherited from the trait will overwrite the value of the parameter. + // See tests/run/traitValOverriddenByParamAccessor.scala + tree else field.setFlag(Mutable) // Necessary for vals mixed in from traits val initializer = diff --git a/tests/run/traitValOverriddenByParamAccessor.scala b/tests/run/traitValOverriddenByParamAccessor.scala new file mode 100644 index 000000000000..2dd6b6fbb167 --- /dev/null +++ b/tests/run/traitValOverriddenByParamAccessor.scala @@ -0,0 +1,17 @@ +trait Foo { + val someField: Int = 5 +} + +class SimpleFoo extends Foo + +class Bar(override val someField: Int) extends Foo + +object Test { + def main(args: Array[String]): Unit = { + val simpleFoo = new SimpleFoo + assert(simpleFoo.someField == 5) + + val bar = new Bar(44) + assert(bar.someField == 44) + } +} From e88c5b782b30ea6c74be1c285fd1a7217c0ecc3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Fri, 17 Jul 2020 13:52:14 +0200 Subject: [PATCH 21/24] Avoid round-trip through sourceModule in scalacLinkedClass. The round-trip is useless, and causes issues with some Scala2-emitted modules that do not have module classes (seems to be related to modules inside traits). --- compiler/src/dotty/tools/dotc/core/SymDenotations.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 340082041d55..70525007707a 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -1153,7 +1153,7 @@ object SymDenotations { final def scalacLinkedClass(using Context): Symbol = if (this.is(ModuleClass)) companionNamed(effectiveName.toTypeName) - else if (this.isClass) companionNamed(effectiveName.moduleClassName).sourceModule.moduleClass + else if (this.isClass) companionNamed(effectiveName.moduleClassName) else NoSymbol /** Find companion class symbol with given name, or NoSymbol if none exists. From 64d3b9a0b280d6f32191a758e6946f286f2ebeb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Sun, 19 Jul 2020 13:56:32 +0200 Subject: [PATCH 22/24] Improve comments for the new trait encoding. --- .../tools/backend/jvm/BCodeHelpers.scala | 1 + .../tools/backend/jvm/BCodeSkelBuilder.scala | 34 +++++++++---- .../dotc/transform/AugmentScala2Traits.scala | 2 +- .../tools/dotc/transform/Constructors.scala | 3 +- .../dotty/tools/dotc/transform/Mixin.scala | 50 ++++++++----------- 5 files changed, 51 insertions(+), 39 deletions(-) diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala b/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala index d72cfa7e8ae2..34ec2b2bb747 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala @@ -959,6 +959,7 @@ object BCodeHelpers { /** An attachment on Apply nodes indicating that it should be compiled with * `invokespecial` instead of `invokevirtual`. This is used for static * forwarders. + * See BCodeSkelBuilder.makeStaticForwarder for more details. */ val UseInvokeSpecial = new dotc.util.Property.Key[Unit] diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala index 85c74f52667f..acc80157238a 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala @@ -497,14 +497,18 @@ trait BCodeSkelBuilder extends BCodeHelpers { case ValDef(name, tpt, rhs) => () // fields are added in `genPlainClass()`, via `addClassFields()` case dd: DefDef => + /* First generate a static forwarder if this is a non-private trait + * trait method. This is required for super calls to this method, which + * go through the static forwarder in order to work around limitations + * of the JVM. + * In theory, this would go in a separate MiniPhase, but it would have to + * sit in a MegaPhase of its own between GenSJSIR and GenBCode, so the cost + * is not worth it. We directly do it in this back-end instead, which also + * kind of makes sense because it is JVM-specific. + */ val sym = dd.symbol - - def needsStaticImplMethod: Boolean = - claszSymbol.isInterface - && !dd.rhs.isEmpty - && !sym.isPrivate - && !sym.isStaticMember - + val needsStaticImplMethod = + claszSymbol.isInterface && !dd.rhs.isEmpty && !sym.isPrivate && !sym.isStaticMember if needsStaticImplMethod then genStaticForwarderForDefDef(dd) @@ -550,11 +554,23 @@ trait BCodeSkelBuilder extends BCodeHelpers { } // end of method initJMethod private def genStaticForwarderForDefDef(dd: DefDef): Unit = - val forwarderDef = makeStaticForwarder(dd, traitSuperAccessorName(dd.symbol)) + val forwarderDef = makeStaticForwarder(dd) genDefDef(forwarderDef) - private def makeStaticForwarder(dd: DefDef, name: String): DefDef = + /* Generates a synthetic static forwarder for a trait method. + * For a method such as + * def foo(...args: Ts): R + * in trait X, we generate the following method: + * static def foo$($this: X, ...args: Ts): R = + * invokespecial $this.X::foo(...args) + * We force an invokespecial with the attachment UseInvokeSpecial. It is + * necessary to make sure that the call will not follow overrides of foo() + * in subtraits and subclasses, since the whole point of this forward is to + * encode super calls. + */ + private def makeStaticForwarder(dd: DefDef): DefDef = val origSym = dd.symbol.asTerm + val name = traitSuperAccessorName(origSym) val info = origSym.info match case mt: MethodType => MethodType(nme.SELF :: mt.paramNames, origSym.owner.typeRef :: mt.paramInfos, mt.resType) diff --git a/compiler/src/dotty/tools/dotc/transform/AugmentScala2Traits.scala b/compiler/src/dotty/tools/dotc/transform/AugmentScala2Traits.scala index afbd76f0fd43..8cd35da5361e 100644 --- a/compiler/src/dotty/tools/dotc/transform/AugmentScala2Traits.scala +++ b/compiler/src/dotty/tools/dotc/transform/AugmentScala2Traits.scala @@ -25,7 +25,7 @@ object AugmentScala2Traits { * Strangely, Scala 2 super accessors are pickled as private, but are compiled as public expanded. * In this phase we expand them and make them non-private, so that `ResolveSuper` does something meaningful. * - * TODO Should we merge this into `ResolveSuper` at this point? + * TODO Should we already drop the Private flag when reading from Scala2 pickles in Scala2Unpickler? */ class AugmentScala2Traits extends MiniPhase with IdentityDenotTransformer { thisPhase => import ast.tpd._ diff --git a/compiler/src/dotty/tools/dotc/transform/Constructors.scala b/compiler/src/dotty/tools/dotc/transform/Constructors.scala index 36961dd60c4d..e7f04570e029 100644 --- a/compiler/src/dotty/tools/dotc/transform/Constructors.scala +++ b/compiler/src/dotty/tools/dotc/transform/Constructors.scala @@ -212,7 +212,8 @@ class Constructors extends MiniPhase with IdentityDenotTransformer { thisPhase = val sym = stat.symbol assert(isRetained(sym), sym) if (!stat.rhs.isEmpty && !isWildcardArg(stat.rhs)) - /* !!! This should really just be `sym.setter`. However, if we do that, we'll miss + /* !!! Work around #9390 + * This should really just be `sym.setter`. However, if we do that, we'll miss * setters for mixed in `private var`s. Even though the scope clearly contains the * setter symbol with the correct Name structure (since the `find` finds it), * `.decl(setterName)` used by `.setter` through `.accessorNamed` will *not* find it. diff --git a/compiler/src/dotty/tools/dotc/transform/Mixin.scala b/compiler/src/dotty/tools/dotc/transform/Mixin.scala index 7e34bca4e58e..7e4d3f0c3217 100644 --- a/compiler/src/dotty/tools/dotc/transform/Mixin.scala +++ b/compiler/src/dotty/tools/dotc/transform/Mixin.scala @@ -30,18 +30,13 @@ object Mixin { /** This phase performs the following transformations: * - * 1. (done in `traitDefs` and `transformSym`) Map every concrete trait getter + * 1. (done in `traitDefs` and `transformSym`) For every concrete trait getter * * def x(): T = expr * - * to the pair of definitions: + * make it non-private, and add the definition of its trait setter: * - * def x(): T - * protected def initial$x(): T = { stats; expr } - * - * where `stats` comprises all statements between either the start of the trait - * or the previous field definition which are not definitions (i.e. are executed for - * their side effects). + * def TraitName$_setter_$x(v: T): Unit * * 2. (done in `traitDefs`) Make every concrete trait setter * @@ -51,12 +46,16 @@ object Mixin { * * def x_=(y: T) * - * 3. For a non-trait class C: + * 3. (done in `transformSym`) For every module class constructor in traits, + * remove its Private flag (but do not expand its name), since it will have + * to be instantiated in the classes that mix in the trait. + * + * 4. For a non-trait class C: * * For every trait M directly implemented by the class (see SymUtils.mixin), in * reverse linearization order, add the following definitions to C: * - * 3.1 (done in `traitInits`) For every parameter accessor ` def x(): T` in M, + * 4.1 (done in `traitInits`) For every parameter accessor ` def x(): T` in M, * in order of textual occurrence, add * * def x() = e @@ -64,37 +63,32 @@ object Mixin { * where `e` is the constructor argument in C that corresponds to `x`. Issue * an error if no such argument exists. * - * 3.2 (done in `traitInits`) For every concrete trait getter ` def x(): T` in M + * 4.2 (done in `traitInits`) For every concrete trait getter ` def x(): T` in M * which is not a parameter accessor, in order of textual occurrence, produce the following: * - * 3.2.1 If `x` is also a member of `C`, and is a lazy val, + * 4.2.1 If `x` is also a member of `C`, and is a lazy val, * * lazy val x: T = super[M].x * - * 3.2.2 If `x` is also a member of `C`, and M is a Dotty trait, + * 4.2.2 If `x` is also a member of `C`, and is a module, * - * def x(): T = super[M].initial$x() + * lazy module val x: T = new T$(this) * - * 3.2.3 If `x` is also a member of `C`, and M is a Scala 2.x trait: + * 4.2.3 If `x` is also a member of `C`, and is something else: * * def x(): T = _ * - * 3.2.4 If `x` is not a member of `C`, and M is a Dotty trait: - * - * super[M].initial$x() - * - * 3.2.5 If `x` is not a member of `C`, and M is a Scala2.x trait, nothing gets added. - * + * 4.2.5 If `x` is not a member of `C`, nothing gets added. * - * 3.3 (done in `superCallOpt`) The call: + * 4.3 (done in `superCallOpt`) The call: * - * super[M]. + * super[M].$init$() * - * 3.4 (done in `setters`) For every concrete setter ` def x_=(y: T)` in M: + * 4.4 (done in `setters`) For every concrete setter ` def x_=(y: T)` in M: * * def x_=(y: T) = () * - * 3.5 (done in `mixinForwarders`) For every method + * 4.5 (done in `mixinForwarders`) For every method * ` def f[Ts](ps1)...(psN): U` imn M` that needs to be disambiguated: * * def f[Ts](ps1)...(psN): U = super[M].f[Ts](ps1)...(psN) @@ -102,10 +96,10 @@ object Mixin { * A method in M needs to be disambiguated if it is concrete, not overridden in C, * and if it overrides another concrete method. * - * 4. (done in `transformTemplate` and `transformSym`) Drop all parameters from trait - * constructors. + * 5. (done in `transformTemplate` and `transformSym`) Drop all parameters from trait + * constructors, and rename them to `nme.TRAIT_CONSTRUCTOR`. * - * 5. (done in `transformSym`) Drop ParamAccessor flag from all parameter accessors in traits. + * 6. (done in `transformSym`) Drop ParamAccessor flag from all parameter accessors in traits. * * Conceptually, this is the second half of the previous mixin phase. It needs to run * after erasure because it copies references to possibly private inner classes and objects From a754f24a32e6bea84f4ab33c37699e73c455983c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Sun, 19 Jul 2020 14:02:42 +0200 Subject: [PATCH 23/24] Do a bit less work in Mixin.transformSym. * Avoid transforming members of Java interfaces. * Avoid cloning a scope if we do not need it. --- compiler/src/dotty/tools/dotc/transform/Mixin.scala | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/Mixin.scala b/compiler/src/dotty/tools/dotc/transform/Mixin.scala index 7e4d3f0c3217..dbd53e09dc67 100644 --- a/compiler/src/dotty/tools/dotc/transform/Mixin.scala +++ b/compiler/src/dotty/tools/dotc/transform/Mixin.scala @@ -122,7 +122,7 @@ class Mixin extends MiniPhase with SymTransformer { thisPhase => override def changesMembers: Boolean = true // the phase adds implementions of mixin accessors override def transformSym(sym: SymDenotation)(using Context): SymDenotation = - def ownerIsTrait: Boolean = wasOneOf(sym.owner, Trait) + def ownerIsTrait: Boolean = was(sym.owner, Trait, butNot = JavaDefined) if (sym.is(Accessor, butNot = Deferred) && ownerIsTrait) { val sym1 = @@ -140,7 +140,7 @@ class Mixin extends MiniPhase with SymTransformer { thisPhase => info = MethodType(Nil, sym.info.resultType)) else if sym.is(Trait, butNot = JavaDefined) then val classInfo = sym.asClass.classInfo - val decls1 = classInfo.decls.cloneScope + lazy val decls1 = classInfo.decls.cloneScope var modified: Boolean = false for (decl <- classInfo.decls) // !decl.isClass avoids forcing nested traits, preventing cycles @@ -160,6 +160,9 @@ class Mixin extends MiniPhase with SymTransformer { thisPhase => private def wasOneOf(sym: Symbol, flags: FlagSet)(using Context): Boolean = atPhase(thisPhase) { sym.isOneOf(flags) } + private def was(sym: Symbol, flag: Flag, butNot: FlagSet)(using Context): Boolean = + atPhase(thisPhase) { sym.is(flag, butNot) } + private def needsTraitSetter(sym: Symbol)(using Context): Boolean = sym.isGetter && !wasOneOf(sym, DeferredOrLazy | ParamAccessor) && atPhase(thisPhase) { !sym.setter.exists } From 77e29253e1443d26a9bb4852e36c9d5f3da15d48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Mon, 20 Jul 2020 09:48:31 +0200 Subject: [PATCH 24/24] Integrate the remaining job of AugmentScala2Traits into Scala2Unpickler. This allows to entirely get rid of the phase as well as the `Scala2xAugmented` flag. --- compiler/src/dotty/tools/dotc/Compiler.scala | 1 - .../src/dotty/tools/dotc/core/Flags.scala | 8 +-- .../core/unpickleScala2/Scala2Unpickler.scala | 9 ++-- .../dotc/transform/AugmentScala2Traits.scala | 54 ------------------- .../tools/dotc/transform/ResolveSuper.scala | 1 - 5 files changed, 8 insertions(+), 65 deletions(-) delete mode 100644 compiler/src/dotty/tools/dotc/transform/AugmentScala2Traits.scala diff --git a/compiler/src/dotty/tools/dotc/Compiler.scala b/compiler/src/dotty/tools/dotc/Compiler.scala index f5a2bb4fec81..84e81801a495 100644 --- a/compiler/src/dotty/tools/dotc/Compiler.scala +++ b/compiler/src/dotty/tools/dotc/Compiler.scala @@ -86,7 +86,6 @@ class Compiler { new LiftTry, // Put try expressions that might execute on non-empty stacks into their own methods new CollectNullableFields, // Collect fields that can be nulled out after use in lazy initialization new ElimOuterSelect, // Expand outer selections - new AugmentScala2Traits, // Augments Scala2 traits so that super accessors are made non-private new ResolveSuper, // Implement super accessors new FunctionXXLForwarders, // Add forwarders for FunctionXXL apply method new ParamForwarding, // Add forwarders for aliases of superclass parameters diff --git a/compiler/src/dotty/tools/dotc/core/Flags.scala b/compiler/src/dotty/tools/dotc/core/Flags.scala index 8fc9f1920b3c..7fd3042f67d5 100644 --- a/compiler/src/dotty/tools/dotc/core/Flags.scala +++ b/compiler/src/dotty/tools/dotc/core/Flags.scala @@ -378,12 +378,8 @@ object Flags { /** Children were queried on this class */ val (_, _, ChildrenQueried @ _) = newFlags(56, "") - /** A module variable (Scala 2.x only) - * / - * A Scala 2.x trait that has been augmented. - * This is set in `AugmentScala2Trait` when the trait is augmented. - */ - val (_, Scala2ModuleVar @ _, Scala2xAugmented @ _) = newFlags(57, "", "") + /** A module variable (Scala 2.x only) */ + val (_, Scala2ModuleVar @ _, _) = newFlags(57, "") /** A macro */ val (Macro @ _, _, _) = newFlags(58, "") diff --git a/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala b/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala index 75bb306aebe3..48acfa480070 100644 --- a/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala +++ b/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala @@ -454,8 +454,12 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas flags = flags &~ Scala2ExpandedName } if (flags.is(Scala2SuperAccessor)) { - name = name.asTermName.unmangle(SuperAccessorName) - flags = flags &~ Scala2SuperAccessor + /* Scala 2 super accessors are pickled as private, but are compiled as public expanded. + * Dotty super accessors, however, are already pickled as public expanded. + * We bridge the gap right now. + */ + name = name.asTermName.unmangle(SuperAccessorName).expandedName(owner) + flags = flags &~ (Scala2SuperAccessor | Private) } name = name.mapLast(_.decode) @@ -1310,4 +1314,3 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas errorBadSignature("expected an TypeDef (" + other + ")") } } - diff --git a/compiler/src/dotty/tools/dotc/transform/AugmentScala2Traits.scala b/compiler/src/dotty/tools/dotc/transform/AugmentScala2Traits.scala deleted file mode 100644 index 8cd35da5361e..000000000000 --- a/compiler/src/dotty/tools/dotc/transform/AugmentScala2Traits.scala +++ /dev/null @@ -1,54 +0,0 @@ -package dotty.tools.dotc -package transform - -import core._ -import MegaPhase._ -import Contexts._ -import Flags._ -import SymUtils._ -import Symbols._ -import Types._ -import Decorators._ -import DenotTransformers._ -import Annotations._ -import StdNames._ -import NameOps._ -import NameKinds.{ExpandedName, TraitSetterName} -import ast.Trees._ - -object AugmentScala2Traits { - val name: String = "augmentScala2Traits" -} - -/** This phase augments Scala2 traits to fix up super accessors. - * - * Strangely, Scala 2 super accessors are pickled as private, but are compiled as public expanded. - * In this phase we expand them and make them non-private, so that `ResolveSuper` does something meaningful. - * - * TODO Should we already drop the Private flag when reading from Scala2 pickles in Scala2Unpickler? - */ -class AugmentScala2Traits extends MiniPhase with IdentityDenotTransformer { thisPhase => - import ast.tpd._ - - override def changesMembers: Boolean = true - - override def phaseName: String = AugmentScala2Traits.name - - override def transformTemplate(impl: Template)(using Context): Template = { - val cls = impl.symbol.owner.asClass - for (mixin <- cls.mixins) { - val erasedMixin = TypeErasure.normalizeClass(mixin) - if (erasedMixin.is(Scala2x) && !erasedMixin.is(Scala2xAugmented)) - augmentScala2Trait(erasedMixin) - } - impl - } - - private def augmentScala2Trait(mixin: ClassSymbol)(using Context): Unit = { - for (sym <- mixin.info.decls) { - if (sym.isSuperAccessor) - sym.ensureNotPrivate.installAfter(thisPhase) - } - mixin.setFlagFrom(thisPhase, Scala2xAugmented) - } -} diff --git a/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala b/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala index 1f3600b471cd..5a73402b088d 100644 --- a/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala +++ b/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala @@ -34,7 +34,6 @@ class ResolveSuper extends MiniPhase with IdentityDenotTransformer { thisPhase = override def phaseName: String = ResolveSuper.name override def runsAfter: Set[String] = Set(ElimByName.name, // verified empirically, need to figure out what the reason is. - AugmentScala2Traits.name, PruneErasedDefs.name) // Erased decls make `isCurrent` work incorrectly override def changesMembers: Boolean = true // the phase adds super accessors