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..34ec2b2bb747 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,11 @@ 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. + * 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 8721b5a10828..acc80157238a 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,23 @@ trait BCodeSkelBuilder extends BCodeHelpers { case ValDef(name, tpt, rhs) => () // fields are added in `genPlainClass()`, via `addClassFields()` - case dd: DefDef => genDefDef(dd) + 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 + val needsStaticImplMethod = + claszSymbol.isInterface && !dd.rhs.isEmpty && !sym.isPrivate && !sym.isStaticMember + if needsStaticImplMethod then + genStaticForwarderForDefDef(dd) + + genDefDef(dd) case tree: Template => val body = @@ -537,6 +553,37 @@ trait BCodeSkelBuilder extends BCodeHelpers { } // end of method initJMethod + private def genStaticForwarderForDefDef(dd: DefDef): Unit = + val forwarderDef = makeStaticForwarder(dd) + genDefDef(forwarderDef) + + /* 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) + 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 c55fa34d2ebc..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 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 @@ -110,8 +109,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..7fd3042f67d5 100644 --- a/compiler/src/dotty/tools/dotc/core/Flags.scala +++ b/compiler/src/dotty/tools/dotc/core/Flags.scala @@ -378,13 +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 partially augmented. - * This is set in `AugmentScala2Trait` and reset in `LinkScala2Impls` - * when the trait is fully augmented. - */ - val (_, Scala2ModuleVar @ _, Scala2xPartiallyAugmented @ _) = 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/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. 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 af74ba9f1de6..000000000000 --- a/compiler/src/dotty/tools/dotc/transform/AugmentScala2Traits.scala +++ /dev/null @@ -1,69 +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 with additional members needed for mixin composition. - * - * These symbols would have been added between Unpickling and Mixin in the Scala2 pipeline. - * - * 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. - */ -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(Scala2xPartiallyAugmented)) - augmentScala2Trait(erasedMixin) - } - impl - } - - 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 - sym.ensureNotPrivate.installAfter(thisPhase) - } - mixin.setFlagFrom(thisPhase, Scala2xPartiallyAugmented) - } -} diff --git a/compiler/src/dotty/tools/dotc/transform/Constructors.scala b/compiler/src/dotty/tools/dotc/transform/Constructors.scala index 14ac216b556a..e7f04570e029 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._ @@ -204,7 +205,33 @@ 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 + assert(isRetained(sym), sym) + if (!stat.rhs.isEmpty && !isWildcardArg(stat.rhs)) + /* !!! 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. + * 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 (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) 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/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 = 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 3a30ae41fc6e..000000000000 --- a/compiler/src/dotty/tools/dotc/transform/LinkScala2Impls.scala +++ /dev/null @@ -1,101 +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 StdNames._ -import ast.Trees._ -import NameKinds.ImplMethName - -/** Rewrite calls - * - * super[M].f(args) - * - * where M is a Scala 2.x trait implemented by the current class to - * - * M.f$(this, args) - * - * where f$ is a static member of M. - */ -class LinkScala2Impls extends MiniPhase with IdentityDenotTransformer { 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 - - private def addStaticForwarders(mixin: ClassSymbol)(using Context): Unit = { - 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) - } - - 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 - } - - 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 => - 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 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) - 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 -} diff --git a/compiler/src/dotty/tools/dotc/transform/Memoize.scala b/compiler/src/dotty/tools/dotc/transform/Memoize.scala index e8a3c564d16a..0113b06c1618 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._ @@ -117,43 +118,57 @@ class Memoize extends MiniPhase with IdentityDenotTransformer { thisPhase => EmptyTree } - val constantFinalVal = sym.isAllOf(Accessor | Final, butNot = Mutable) && tree.rhs.isInstanceOf[Literal] - - if (sym.is(Accessor, butNot = NoFieldNeeded) && !constantFinalVal) { - 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 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 = + 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 } } diff --git a/compiler/src/dotty/tools/dotc/transform/Mixin.scala b/compiler/src/dotty/tools/dotc/transform/Mixin.scala index 6cdf54d5d305..dbd53e09dc67 100644 --- a/compiler/src/dotty/tools/dotc/transform/Mixin.scala +++ b/compiler/src/dotty/tools/dotc/transform/Mixin.scala @@ -13,28 +13,30 @@ 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: * - * 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: - * - * def x(): T - * protected def initial$x(): T = { stats; expr } + * make it non-private, and add the definition of its trait setter: * - * 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 * @@ -44,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 @@ -57,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. * + * 4.3 (done in `superCallOpt`) The call: * - * 3.3 (done in `superCallOpt`) The call: + * super[M].$init$() * - * super[M]. - * - * 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) @@ -95,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 @@ -121,33 +122,57 @@ 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 = was(sym.owner, Trait, butNot = JavaDefined) + + if (sym.is(Accessor, butNot = Deferred) && ownerIsTrait) { 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.isConstructor && (sym.owner.is(Trait, butNot = Scala2x) || (sym.owner.isAllOf(Trait | Scala2x) && ctx.settings.scalajs.value))) + 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 && ownerIsTrait) sym.copySymDenotation( name = nme.TRAIT_CONSTRUCTOR, info = MethodType(Nil, sym.info.resultType)) + else if sym.is(Trait, butNot = JavaDefined) then + val classInfo = sym.asClass.classInfo + lazy val decls1 = classInfo.decls.cloneScope + var modified: Boolean = false + 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 + 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 - 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) } + + 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 } + && !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 @@ -155,26 +180,15 @@ class Mixin extends MiniPhase with SymTransformer { 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 +235,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 +252,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..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 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 = 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") 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 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 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$ - ") } 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) 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) + } +} 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) + } +}