From 506ef7c4b464973b917bb38a2b7745072afd85df Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Fri, 12 Jun 2020 16:43:22 +0200 Subject: [PATCH 01/35] Fix #1441: init $MODULE in This is a port of the following PR from Scala 2: https://github.com/scala/scala/pull/7270 --- .../tools/backend/jvm/BCodeSkelBuilder.scala | 92 +++++++++++++++---- .../src/dotty/tools/dotc/ast/TreeInfo.scala | 12 +++ .../tools/dotc/transform/Constructors.scala | 5 +- 3 files changed, 85 insertions(+), 24 deletions(-) diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala index acc80157238a..1734a5fa33d5 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala @@ -10,15 +10,17 @@ import scala.tools.asm.util.{TraceMethodVisitor, ASMifier} import java.io.PrintWriter import dotty.tools.dotc.ast.tpd +import dotty.tools.dotc.ast.TreeTypeMap 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.{nme, str} +import dotty.tools.dotc.core.StdNames._ import dotty.tools.dotc.core.Symbols._ -import dotty.tools.dotc.core.Types.{MethodType, Type} +import dotty.tools.dotc.core.Types._ import dotty.tools.dotc.util.Spans._ import dotty.tools.dotc.report +import dotty.tools.dotc.transform.SymUtils._ /* * @@ -93,12 +95,12 @@ trait BCodeSkelBuilder extends BCodeHelpers { /* ---------------- helper utils for generating classes and fields ---------------- */ - def genPlainClass(cd: TypeDef) = cd match { - case TypeDef(_, impl) => + def genPlainClass(cd0: TypeDef) = cd0 match { + case TypeDef(_, impl: Template) => assert(cnode == null, "GenBCode detected nested methods.") innerClassBufferASM.clear() - claszSymbol = cd.symbol + claszSymbol = cd0.symbol isCZParcelable = isAndroidParcelableClass(claszSymbol) isCZStaticModule = claszSymbol.isStaticModuleClass thisName = internalName(claszSymbol) @@ -107,14 +109,70 @@ trait BCodeSkelBuilder extends BCodeHelpers { initJClass(cnode) + val cd = if (isCZStaticModule) { + // Move statements from the primary constructor following the superclass constructor call to + // a newly synthesised tree representing the "", which also assigns the MODULE$ field. + // Because the assigments to both the module instance fields, and the fields of the module itself + // are in the , these fields can be static + final. + + // TODO should we do this transformation earlier, say in Constructors? Or would that just cause + // pain for scala-{js, native}? + + for (f <- claszSymbol.info.decls.filter(_.isField)) + f.setFlag(JavaStatic) + + val (uptoSuperStats, remainingConstrStats) = splitAtSuper(impl.constr.rhs.asInstanceOf[Block].stats) + val clInitSymbol = ctx.newSymbol( + claszSymbol, + nme.STATIC_CONSTRUCTOR, + JavaStatic | Method, + MethodType(Nil)(_ => Nil, _ => defn.UnitType), + privateWithin = NoSymbol, + coord = claszSymbol.coord + ) + + // We don't need to enter this field into the decls of claszSymbol.info as this is added manually to the generated class + // in addModuleInstanceField. TODO: try adding it to the decls and making the usual field generation do the right thing. + val moduleField = ctx.newSymbol( + claszSymbol, + str.MODULE_INSTANCE_FIELD.toTermName, + JavaStatic | Private, + claszSymbol.typeRef, + privateWithin = NoSymbol, + coord = claszSymbol.coord + ) + + val thisMap = new TreeTypeMap( + treeMap = { + case tree: This if tree.symbol == claszSymbol => + ref(claszSymbol.sourceModule) + case tree => + tree + }, + oldOwners = claszSymbol.primaryConstructor :: Nil, + newOwners = clInitSymbol :: Nil + ) + + val callConstructor = New(claszSymbol.typeRef).select(claszSymbol.primaryConstructor).appliedToArgs(Nil) + val assignModuleField = Assign(ref(moduleField), callConstructor) + val remainingConstrStatsSubst = remainingConstrStats.map(thisMap(_)) + val clinit = DefDef( + clInitSymbol, + Block(assignModuleField :: remainingConstrStatsSubst, unitLiteral) + ) + + val constr2 = { + val rhs = Block(uptoSuperStats, impl.constr.rhs.asInstanceOf[Block].expr) + cpy.DefDef(impl.constr)(rhs = rhs) + } + + val impl2 = cpy.Template(impl)(constr = constr2, body = clinit :: impl.body) + cpy.TypeDef(cd0)(rhs = impl2) + } else cd0 + val methodSymbols = for (f <- cd.symbol.info.decls.toList if f.is(Method) && f.isTerm && !f.is(Module)) yield f val hasStaticCtor = methodSymbols exists (_.isStaticConstructor) - if (!hasStaticCtor) { - // but needs one ... - if (isCZStaticModule || isCZParcelable) { - fabricateStaticInit() - } - } + if (!hasStaticCtor && isCZParcelable) fabricateStaticInitAndroid() val optSerial: Option[Long] = claszSymbol.getAnnotation(defn.SerialVersionUIDAnnot).flatMap { annot => @@ -223,7 +281,7 @@ trait BCodeSkelBuilder extends BCodeHelpers { /* * must-single-thread */ - private def fabricateStaticInit(): Unit = { + private def fabricateStaticInitAndroid(): Unit = { val clinit: asm.MethodVisitor = cnode.visitMethod( GenBCodeOps.PublicStatic, // TODO confirm whether we really don't want ACC_SYNTHETIC nor ACC_DEPRECATED @@ -234,15 +292,9 @@ trait BCodeSkelBuilder extends BCodeHelpers { ) clinit.visitCode() - /* "legacy static initialization" */ - if (isCZStaticModule) { - clinit.visitTypeInsn(asm.Opcodes.NEW, thisName) - clinit.visitMethodInsn(asm.Opcodes.INVOKESPECIAL, - thisName, INSTANCE_CONSTRUCTOR_NAME, "()V", false) - } if (isCZParcelable) { legacyAddCreatorCode(clinit, cnode, thisName) } - clinit.visitInsn(asm.Opcodes.RETURN) + clinit.visitInsn(asm.Opcodes.RETURN) clinit.visitMaxs(0, 0) // just to follow protocol, dummy arguments clinit.visitEnd() } @@ -679,7 +731,7 @@ trait BCodeSkelBuilder extends BCodeHelpers { /* * must-single-thread * - * TODO document, explain interplay with `fabricateStaticInit()` + * TODO document, explain interplay with `fabricateStaticInitAndroid()` */ private def appendToStaticCtor(dd: DefDef): Unit = { diff --git a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala index 7af09bba98f1..46b9366283ac 100644 --- a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala +++ b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala @@ -828,6 +828,18 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] => loop(tree) } + /** Return a pair consisting of (supercall, rest) + * + * - supercall: the of superclass call, excluding trait constr calls + * + * The supercall is always the first statement (if exists) + */ + final def splitAtSuper(constrStats: List[Tree])(implicit ctx: Context): (List[Tree], List[Tree]) = + constrStats.toList match { + case (sc: Apply) :: rest if sc.symbol.isConstructor => (sc :: Nil, rest) + case stats => (Nil, stats) + } + /** Structural tree comparison (since == on trees is reference equality). * For the moment, only Ident, Select, Literal, Apply and TypeApply are supported */ diff --git a/compiler/src/dotty/tools/dotc/transform/Constructors.scala b/compiler/src/dotty/tools/dotc/transform/Constructors.scala index e7f04570e029..a462cba7a000 100644 --- a/compiler/src/dotty/tools/dotc/transform/Constructors.scala +++ b/compiler/src/dotty/tools/dotc/transform/Constructors.scala @@ -284,10 +284,7 @@ class Constructors extends MiniPhase with IdentityDenotTransformer { thisPhase = // TODO: this happens to work only because Constructors is the last phase in group } - val (superCalls, followConstrStats) = constrStats.toList match { - case (sc: Apply) :: rest if sc.symbol.isConstructor => (sc :: Nil, rest) - case stats => (Nil, stats) - } + val (superCalls, followConstrStats) = splitAtSuper(constrStats.toList) val mappedSuperCalls = vparams match { case (outerParam @ ValDef(nme.OUTER, _, _)) :: _ => From 6b28347c382fa3bf9265f2aaf04e3de27a937e61 Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Sun, 14 Jun 2020 23:02:20 +0200 Subject: [PATCH 02/35] Remove old init code in constructor --- .../tools/backend/jvm/BCodeBodyBuilder.scala | 17 ----------------- .../tools/backend/jvm/BCodeSkelBuilder.scala | 3 --- 2 files changed, 20 deletions(-) diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala index 3007ea2b05cc..1c3a8794aa50 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala @@ -711,22 +711,6 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { else genSynchronized(app, expectedType) case Apply(fun @ DesugaredSelect(Super(_, _), _), args) => - def initModule(): Unit = { - // we initialize the MODULE$ field immediately after the super ctor - if (!isModuleInitialized && - jMethodName == INSTANCE_CONSTRUCTOR_NAME && - fun.symbol.javaSimpleName == INSTANCE_CONSTRUCTOR_NAME && - claszSymbol.isStaticModuleClass) { - isModuleInitialized = true - mnode.visitVarInsn(asm.Opcodes.ALOAD, 0) - mnode.visitFieldInsn( - asm.Opcodes.PUTSTATIC, - thisName, - str.MODULE_INSTANCE_FIELD, - "L" + thisName + ";" - ) - } - } // 'super' call: Note: since constructors are supposed to // return an instance of what they construct, we have to take // special care. On JVM they are 'void', and Scala forbids (syntactically) @@ -736,7 +720,6 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { mnode.visitVarInsn(asm.Opcodes.ALOAD, 0) genLoadArguments(args, paramTKs(app)) generatedType = genCallMethod(fun.symbol, InvokeStyle.Super, app.span) - initModule() // 'new' constructor call: Note: since constructors are // thought to return an instance of what they construct, diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala index 1734a5fa33d5..d69d9b05ddde 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala @@ -333,8 +333,6 @@ trait BCodeSkelBuilder extends BCodeHelpers { var isMethSymStaticCtor = false var returnType: BType = null var methSymbol: Symbol = null - // in GenASM this is local to genCode(), ie should get false whenever a new method is emitted (including fabricated ones eg addStaticInit()) - var isModuleInitialized = false // used by genLoadTry() and genSynchronized() var earlyReturnVar: Symbol = null var shouldEmitCleanup = false @@ -533,7 +531,6 @@ trait BCodeSkelBuilder extends BCodeHelpers { assert(varsInScope == null, "Unbalanced entering/exiting of GenBCode's genBlock().") // check previous invocation of genDefDef unregistered as many cleanups as it registered. assert(cleanups == Nil, "Previous invocation of genDefDef didn't unregister as many cleanups as it registered.") - isModuleInitialized = false earlyReturnVar = null shouldEmitCleanup = false From 7426c78bf00aa6e50e63fd50ea06c7ef279b8fda Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Sun, 14 Jun 2020 23:04:45 +0200 Subject: [PATCH 03/35] Handle class field uniformly --- .../tools/backend/jvm/BCodeSkelBuilder.scala | 30 +++---------------- 1 file changed, 4 insertions(+), 26 deletions(-) diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala index d69d9b05ddde..97a9de017332 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala @@ -129,10 +129,8 @@ trait BCodeSkelBuilder extends BCodeHelpers { MethodType(Nil)(_ => Nil, _ => defn.UnitType), privateWithin = NoSymbol, coord = claszSymbol.coord - ) + ).entered - // We don't need to enter this field into the decls of claszSymbol.info as this is added manually to the generated class - // in addModuleInstanceField. TODO: try adding it to the decls and making the usual field generation do the right thing. val moduleField = ctx.newSymbol( claszSymbol, str.MODULE_INSTANCE_FIELD.toTermName, @@ -140,7 +138,7 @@ trait BCodeSkelBuilder extends BCodeHelpers { claszSymbol.typeRef, privateWithin = NoSymbol, coord = claszSymbol.coord - ) + ).entered val thisMap = new TreeTypeMap( treeMap = { @@ -192,7 +190,7 @@ trait BCodeSkelBuilder extends BCodeHelpers { addClassFields() innerClassBufferASM ++= classBTypeFromSymbol(claszSymbol).info.memberClasses - gen(impl) + gen(cd.rhs) addInnerClassesASM(cnode, innerClassBufferASM.toList) if (AsmUtils.traceClassEnabled && cnode.name.contains(AsmUtils.traceClassPattern)) @@ -237,12 +235,7 @@ trait BCodeSkelBuilder extends BCodeHelpers { cnode.visitAttribute(if (ssa.isDefined) pickleMarkerLocal else pickleMarkerForeign) emitAnnotations(cnode, claszSymbol.annotations ++ ssa) - if (isCZStaticModule || isCZParcelable) { - - if (isCZStaticModule) { addModuleInstanceField() } - - } else { - + if (!isCZStaticModule && !isCZParcelable) { val skipStaticForwarders = (claszSymbol.isInterface || claszSymbol.is(Module) || ctx.settings.XnoForwarders.value) if (!skipStaticForwarders) { val lmoc = claszSymbol.companionModule @@ -263,21 +256,6 @@ trait BCodeSkelBuilder extends BCodeHelpers { } // end of method initJClass - /* - * can-multi-thread - */ - private def addModuleInstanceField(): Unit = { - val fv = - cnode.visitField(GenBCodeOps.PublicStaticFinal, // TODO confirm whether we really don't want ACC_SYNTHETIC nor ACC_DEPRECATED - str.MODULE_INSTANCE_FIELD, - "L" + thisName + ";", - null, // no java-generic-signature - null // no initial value - ) - - fv.visitEnd() - } - /* * must-single-thread */ From 967375df5e35539f83cf5f15f96f88ffc51b9be3 Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Sun, 14 Jun 2020 23:09:51 +0200 Subject: [PATCH 04/35] Fix flags of MODULE field --- compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala index 97a9de017332..0cdb10c620c1 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala @@ -134,7 +134,7 @@ trait BCodeSkelBuilder extends BCodeHelpers { val moduleField = ctx.newSymbol( claszSymbol, str.MODULE_INSTANCE_FIELD.toTermName, - JavaStatic | Private, + JavaStatic | Final, claszSymbol.typeRef, privateWithin = NoSymbol, coord = claszSymbol.coord From b07b47c0fe858b61d18613700f5f9b2a49cf6beb Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Sun, 14 Jun 2020 23:34:52 +0200 Subject: [PATCH 05/35] Remove legacy code --- .../tools/backend/jvm/BCodeSkelBuilder.scala | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala index 0cdb10c620c1..fd6c253d873a 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala @@ -270,7 +270,7 @@ trait BCodeSkelBuilder extends BCodeHelpers { ) clinit.visitCode() - if (isCZParcelable) { legacyAddCreatorCode(clinit, cnode, thisName) } + legacyAddCreatorCode(clinit, cnode, thisName) clinit.visitInsn(asm.Opcodes.RETURN) clinit.visitMaxs(0, 0) // just to follow protocol, dummy arguments @@ -725,21 +725,6 @@ trait BCodeSkelBuilder extends BCodeHelpers { mnode foreachInsn { i => if (i.getOpcode() == asm.Opcodes.RETURN) { rets ::= i } } if (rets.isEmpty) { return } - var insnModA: asm.tree.AbstractInsnNode = null - var insnModB: asm.tree.AbstractInsnNode = null - // call object's private ctor from static ctor - if (isCZStaticModule) { - // NEW `moduleName` - val className = internalName(methSymbol.enclosingClass) - insnModA = new asm.tree.TypeInsnNode(asm.Opcodes.NEW, className) - // INVOKESPECIAL - val callee = methSymbol.enclosingClass.primaryConstructor - val jname = callee.javaSimpleName - val jowner = internalName(callee.owner) - val jtype = asmMethodType(callee).descriptor - insnModB = new asm.tree.MethodInsnNode(asm.Opcodes.INVOKESPECIAL, jowner, jname, jtype, false) - } - var insnParcA: asm.tree.AbstractInsnNode = null var insnParcB: asm.tree.AbstractInsnNode = null // android creator code @@ -765,7 +750,6 @@ trait BCodeSkelBuilder extends BCodeHelpers { // insert a few instructions for initialization before each return instruction for(r <- rets) { - insertBefore(r, insnModA, insnModB) insertBefore(r, insnParcA, insnParcB) } From 7e232e7aa3f4fb0d19f87f047314b1cf6f89492b Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Sun, 14 Jun 2020 23:37:36 +0200 Subject: [PATCH 06/35] Add jvm-9 as target --- compiler/src/dotty/tools/backend/jvm/BCodeIdiomatic.scala | 1 + compiler/src/dotty/tools/dotc/config/ScalaSettings.scala | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeIdiomatic.scala b/compiler/src/dotty/tools/backend/jvm/BCodeIdiomatic.scala index 6e2646c9e051..708a6fee9fff 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeIdiomatic.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeIdiomatic.scala @@ -29,6 +29,7 @@ trait BCodeIdiomatic { case "jvm-1.6" => asm.Opcodes.V1_6 case "jvm-1.7" => asm.Opcodes.V1_7 case "jvm-1.8" => asm.Opcodes.V1_8 + case "jvm-9" => asm.Opcodes.V9 } lazy val majorVersion: Int = (classfileVersion & 0xFF) diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index f875452c7252..dc6ec6aaf65d 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -36,7 +36,7 @@ class ScalaSettings extends Settings.SettingGroup { val color: Setting[String] = ChoiceSetting("-color", "mode", "Colored output", List("always", "never"/*, "auto"*/), "always"/* "auto"*/) withAbbreviation "--color" val source: Setting[String] = ChoiceSetting("-source", "source version", "source version", List("3.0", "3.1", "3.0-migration", "3.1-migration"), "3.0").withAbbreviation("--source") val target: Setting[String] = ChoiceSetting("-target", "target", "Target platform for object files. All JVM 1.5 targets are deprecated.", - List("jvm-1.5", "jvm-1.5-fjbg", "jvm-1.5-asm", "jvm-1.6", "jvm-1.7", "jvm-1.8", "msil"), "jvm-1.8") withAbbreviation "--target" + List("jvm-1.5", "jvm-1.5-fjbg", "jvm-1.5-asm", "jvm-1.6", "jvm-1.7", "jvm-1.8", "jvm-9", "msil"), "jvm-1.8") withAbbreviation "--target" val scalajs: Setting[Boolean] = BooleanSetting("-scalajs", "Compile in Scala.js mode (requires scalajs-library.jar on the classpath).") withAbbreviation "--scalajs" val unchecked: Setting[Boolean] = BooleanSetting("-unchecked", "Enable additional warnings where generated code depends on assumptions.") withAbbreviation "--unchecked" val uniqid: Setting[Boolean] = BooleanSetting("-uniqid", "Uniquely tag all identifiers in debugging output.") withAbbreviation "--unique-id" From 4cd9626fa918e0bb8cf57c6c2f8578889a6d179d Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Mon, 15 Jun 2020 01:30:32 +0200 Subject: [PATCH 07/35] Avoid Ycheck errors Ycheck will check that each class member has a corresponding tree in the template. We thus don't enter `clinit` as a member as we cannot update the template. --- compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala index fd6c253d873a..87189a908437 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala @@ -129,7 +129,7 @@ trait BCodeSkelBuilder extends BCodeHelpers { MethodType(Nil)(_ => Nil, _ => defn.UnitType), privateWithin = NoSymbol, coord = claszSymbol.coord - ).entered + ) val moduleField = ctx.newSymbol( claszSymbol, From a2d9003bae9d561577c1121bcf8124bbcc8fbafd Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Mon, 15 Jun 2020 01:41:00 +0200 Subject: [PATCH 08/35] Pass -Ycheck --- compiler/src/dotty/tools/dotc/transform/TreeChecker.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala index df1ec46bb4f6..739abaa71fba 100644 --- a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala @@ -433,7 +433,8 @@ class TreeChecker extends Phase with SymTransformer { def isNonMagicalMember(x: Symbol) = !x.isValueClassConvertMethod && - !x.name.is(DocArtifactName) + !x.name.is(DocArtifactName) && + !(ctx.phase.id >= ctx.genBCodePhase.id && x.name == str.MODULE_INSTANCE_FIELD.toTermName) val decls = cls.classInfo.decls.toList.toSet.filter(isNonMagicalMember) val defined = impl.body.map(_.symbol) From 3adfcea0d25996aef14d283301a201295d02b434 Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Mon, 15 Jun 2020 02:09:12 +0200 Subject: [PATCH 09/35] Handle undesugared ident --- .../tools/backend/jvm/BCodeSkelBuilder.scala | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala index 87189a908437..1d6a1f366e2e 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala @@ -18,6 +18,7 @@ import dotty.tools.dotc.core.Flags._ import dotty.tools.dotc.core.StdNames._ import dotty.tools.dotc.core.Symbols._ import dotty.tools.dotc.core.Types._ +import dotty.tools.dotc.core.Contexts._ import dotty.tools.dotc.util.Spans._ import dotty.tools.dotc.report import dotty.tools.dotc.transform.SymUtils._ @@ -140,20 +141,22 @@ trait BCodeSkelBuilder extends BCodeHelpers { coord = claszSymbol.coord ).entered - val thisMap = new TreeTypeMap( - treeMap = { + val thisMap = new TreeMap { + override def transform(tree: Tree)(using Context) = tree match { case tree: This if tree.symbol == claszSymbol => ref(claszSymbol.sourceModule) + case ident: Ident => + super.transform(desugarIdent(ident)) case tree => - tree - }, - oldOwners = claszSymbol.primaryConstructor :: Nil, - newOwners = clInitSymbol :: Nil - ) + super.transform(tree) + } + } + + def rewire(stat: Tree) = thisMap.transform(stat).changeOwner(claszSymbol.primaryConstructor, clInitSymbol) val callConstructor = New(claszSymbol.typeRef).select(claszSymbol.primaryConstructor).appliedToArgs(Nil) val assignModuleField = Assign(ref(moduleField), callConstructor) - val remainingConstrStatsSubst = remainingConstrStats.map(thisMap(_)) + val remainingConstrStatsSubst = remainingConstrStats.map(rewire) val clinit = DefDef( clInitSymbol, Block(assignModuleField :: remainingConstrStatsSubst, unitLiteral) From 04d0dc93926c062ab6c3277213b0e657e572622d Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Mon, 15 Jun 2020 11:15:15 +0200 Subject: [PATCH 10/35] Performance tweak --- compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala index 1d6a1f366e2e..ba4cb047b271 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala @@ -171,8 +171,7 @@ trait BCodeSkelBuilder extends BCodeHelpers { cpy.TypeDef(cd0)(rhs = impl2) } else cd0 - val methodSymbols = for (f <- cd.symbol.info.decls.toList if f.is(Method) && f.isTerm && !f.is(Module)) yield f - val hasStaticCtor = methodSymbols exists (_.isStaticConstructor) + val hasStaticCtor = isCZStaticModule || cd.symbol.info.decls.exists(_.isStaticConstructor) if (!hasStaticCtor && isCZParcelable) fabricateStaticInitAndroid() val optSerial: Option[Long] = From 7ad5beb282e666e74534a6551176c84651f2596e Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Mon, 15 Jun 2020 11:38:11 +0200 Subject: [PATCH 11/35] Handle existing in template --- .../tools/backend/jvm/BCodeSkelBuilder.scala | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala index ba4cb047b271..92068c0d2c44 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala @@ -122,8 +122,12 @@ trait BCodeSkelBuilder extends BCodeHelpers { for (f <- claszSymbol.info.decls.filter(_.isField)) f.setFlag(JavaStatic) + val (clinits, body) = impl.body.partition(stat => stat.isInstanceOf[DefDef] && stat.symbol.isStaticConstructor) + val (uptoSuperStats, remainingConstrStats) = splitAtSuper(impl.constr.rhs.asInstanceOf[Block].stats) - val clInitSymbol = ctx.newSymbol( + val clInitSymbol: TermSymbol = + if (clinits.nonEmpty) clinits.head.symbol.asTerm + else ctx.newSymbol( claszSymbol, nme.STATIC_CONSTRUCTOR, JavaStatic | Method, @@ -157,17 +161,19 @@ trait BCodeSkelBuilder extends BCodeHelpers { val callConstructor = New(claszSymbol.typeRef).select(claszSymbol.primaryConstructor).appliedToArgs(Nil) val assignModuleField = Assign(ref(moduleField), callConstructor) val remainingConstrStatsSubst = remainingConstrStats.map(rewire) - val clinit = DefDef( - clInitSymbol, - Block(assignModuleField :: remainingConstrStatsSubst, unitLiteral) - ) + val clinit = clinits match { + case (ddef: DefDef) :: _ => + cpy.DefDef(ddef)(rhs = Block(ddef.rhs :: assignModuleField :: remainingConstrStatsSubst, unitLiteral)) + case _ => + DefDef(clInitSymbol, Block(assignModuleField :: remainingConstrStatsSubst, unitLiteral)) + } val constr2 = { val rhs = Block(uptoSuperStats, impl.constr.rhs.asInstanceOf[Block].expr) cpy.DefDef(impl.constr)(rhs = rhs) } - val impl2 = cpy.Template(impl)(constr = constr2, body = clinit :: impl.body) + val impl2 = cpy.Template(impl)(constr = constr2, body = clinit :: body) cpy.TypeDef(cd0)(rhs = impl2) } else cd0 From 3e6fb43cd7e88efb39f9c7ae0be92dd1187559b6 Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Mon, 15 Jun 2020 14:45:00 +0200 Subject: [PATCH 12/35] Handle super calls properly --- .../tools/backend/jvm/BCodeSkelBuilder.scala | 29 +++++++++++++------ 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala index 92068c0d2c44..2f30db22ccca 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala @@ -119,8 +119,14 @@ trait BCodeSkelBuilder extends BCodeHelpers { // TODO should we do this transformation earlier, say in Constructors? Or would that just cause // pain for scala-{js, native}? - for (f <- claszSymbol.info.decls.filter(_.isField)) - f.setFlag(JavaStatic) + // TODO: enable once we change lazy val encoding + // + // Lazy val encoding assumes bitmap fields are non-static + // + // See `tests/run/given-var.scala` + // + // for (f <- claszSymbol.info.decls.filter(_.isField)) + // f.setFlag(JavaStatic) val (clinits, body) = impl.body.partition(stat => stat.isInstanceOf[DefDef] && stat.symbol.isStaticConstructor) @@ -146,13 +152,18 @@ trait BCodeSkelBuilder extends BCodeHelpers { ).entered val thisMap = new TreeMap { - override def transform(tree: Tree)(using Context) = tree match { - case tree: This if tree.symbol == claszSymbol => - ref(claszSymbol.sourceModule) - case ident: Ident => - super.transform(desugarIdent(ident)) - case tree => - super.transform(tree) + override def transform(tree: Tree)(using Context) = { + val tp = tree.tpe.substThis(claszSymbol.asClass, claszSymbol.sourceModule.termRef) + tree.withType(tp) match { + case tree: This if tree.symbol == claszSymbol => + ref(claszSymbol.sourceModule) + case Apply(fun @ Select(Super(qual, _), _), args) if qual.symbol == claszSymbol => + ref(claszSymbol.sourceModule).select(fun.symbol).appliedToArgs(args) + // case ident: Ident => + // super.transform(desugarIdent(ident)) + case tree => + super.transform(tree) + } } } From fff01624b4bd3f747a073ece411ee03fe879a6fb Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Fri, 10 Jul 2020 16:36:35 +0200 Subject: [PATCH 13/35] Use backend magic to handle super calls The problem can be seen from the following example: trait A { def foo() = ??? } trait B { def foo() = ??? } object C extends A with B { super[A].foo() super[B].foo() } In the code above, we cannot translate the following calls from to : super[A].foo() super[B].foo() super[A].$iinit$() super[B].$init$() More details can be found here: #5928 A principled way would be to generage super accessors as it is done in posttyper. However, the backend has a magic to support prefix to super trees in [1], which is exploited in the Scala 2 fix [2]. [1] https://github.com/scala/scala/pull/5944 [2] https://github.com/scala/scala/pull/7270 --- compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala | 6 ++++-- compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala | 4 ---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala index 1c3a8794aa50..2efba7b83f4a 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala @@ -710,14 +710,16 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { if (t.symbol ne defn.Object_synchronized) genTypeApply(t) else genSynchronized(app, expectedType) - case Apply(fun @ DesugaredSelect(Super(_, _), _), args) => + case Apply(fun @ DesugaredSelect(Super(superQual, _), _), args) => // 'super' call: Note: since constructors are supposed to // return an instance of what they construct, we have to take // special care. On JVM they are 'void', and Scala forbids (syntactically) // to call super constructors explicitly and/or use their 'returned' value. // therefore, we can ignore this fact, and generate code that leaves nothing // on the stack (contrary to what the type in the AST says). - mnode.visitVarInsn(asm.Opcodes.ALOAD, 0) + + // scala/bug#10290: qual can be `this.$outer()` (not just `this`), so we call genLoad (not just ALOAD_0) + genLoad(superQual) genLoadArguments(args, paramTKs(app)) generatedType = genCallMethod(fun.symbol, InvokeStyle.Super, app.span) diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala index 2f30db22ccca..9dedad27b826 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala @@ -157,10 +157,6 @@ trait BCodeSkelBuilder extends BCodeHelpers { tree.withType(tp) match { case tree: This if tree.symbol == claszSymbol => ref(claszSymbol.sourceModule) - case Apply(fun @ Select(Super(qual, _), _), args) if qual.symbol == claszSymbol => - ref(claszSymbol.sourceModule).select(fun.symbol).appliedToArgs(args) - // case ident: Ident => - // super.transform(desugarIdent(ident)) case tree => super.transform(tree) } From 93e1c9e900df858650e20cbacf56efa0c7e7fabf Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Wed, 15 Jul 2020 14:29:39 +0200 Subject: [PATCH 14/35] Fix usage of ctx --- compiler/src/dotty/tools/dotc/transform/TreeChecker.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala index 739abaa71fba..4f8ae9cb757a 100644 --- a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala @@ -434,7 +434,7 @@ class TreeChecker extends Phase with SymTransformer { def isNonMagicalMember(x: Symbol) = !x.isValueClassConvertMethod && !x.name.is(DocArtifactName) && - !(ctx.phase.id >= ctx.genBCodePhase.id && x.name == str.MODULE_INSTANCE_FIELD.toTermName) + !(ctx.phase.id >= genBCodePhase.id && x.name == str.MODULE_INSTANCE_FIELD.toTermName) val decls = cls.classInfo.decls.toList.toSet.filter(isNonMagicalMember) val defined = impl.body.map(_.symbol) From 0cc4dfa22de70a621c9420efb450f486dd35efd2 Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Wed, 15 Jul 2020 15:09:39 +0200 Subject: [PATCH 15/35] Generate super accessors for inner classes This is a port of the following PR in Scala 2: https://github.com/scala/scala/pull/3935 --- .../tools/dotc/transform/SuperAccessors.scala | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/SuperAccessors.scala b/compiler/src/dotty/tools/dotc/transform/SuperAccessors.scala index ef0b41bc8861..3b9547e10532 100644 --- a/compiler/src/dotty/tools/dotc/transform/SuperAccessors.scala +++ b/compiler/src/dotty/tools/dotc/transform/SuperAccessors.scala @@ -5,7 +5,7 @@ import dotty.tools.dotc.ast.{Trees, tpd} import scala.collection.mutable import ValueClasses.isMethodWithExtension import core._ -import Contexts._, Flags._, Symbols._, NameOps._, Trees._ +import Contexts._, Flags._, Symbols._, NameOps._, Trees._, Types.SuperType import TypeUtils._, SymUtils._ import DenotTransformers.DenotTransformer import Symbols._ @@ -99,6 +99,7 @@ class SuperAccessors(thisPhase: DenotTransformer) { val sym = sel.symbol assert(sup.symbol.exists, s"missing symbol in $sel: ${sup.tpe}") val clazz = sup.symbol + val currentClass = ctx.owner.enclosingClass if (sym.isTerm && !sym.is(Method, butNot = Accessor) && !ctx.owner.isAllOf(ParamForwarder)) // ParamForwaders as installed ParamForwarding.scala do use super calls to vals @@ -145,9 +146,20 @@ class SuperAccessors(thisPhase: DenotTransformer) { sel.sourcePos) } } - if (name.isTermName && mix.name.isEmpty && - (clazz.is(Trait) || clazz != ctx.owner.enclosingClass || !validCurrentClass)) - atPhase(thisPhase.next)(superAccessorCall(sel)) + + def mixIsTrait = sup.tpe match { + case SuperType(thisTpe, superTpe) => superTpe.typeSymbol.is(Trait) + } + + val needAccessor = name.isTermName && { + mix.name.isEmpty && (clazz.is(Trait) || clazz != currentClass || !validCurrentClass) || + // SI-8803. If we access super[A] from an inner class (!= currentClass) or closure (validCurrentClass), + // where A is the superclass we need an accessor. If A is a parent trait we don't: in this case mixin + // will re-route the super call directly to the impl class (it's statically known). + !mix.name.isEmpty && (clazz != currentClass || !validCurrentClass) && !mixIsTrait + } + + if (needAccessor) atPhase(thisPhase.next)(superAccessorCall(sel)) else sel } From 1fbd26f4f5a022ef2853e50323eafb77c332f08a Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Wed, 15 Jul 2020 15:17:32 +0200 Subject: [PATCH 16/35] Fix #9341: add test --- tests/run/i9341.scala | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 tests/run/i9341.scala diff --git a/tests/run/i9341.scala b/tests/run/i9341.scala new file mode 100644 index 000000000000..cf589ad88adb --- /dev/null +++ b/tests/run/i9341.scala @@ -0,0 +1,10 @@ +class T { def f: Int = 10 } +class B extends T { + class C { B.super[T].f } + class D { B.super.f } + new C + new D +} + +@main +def Test = new B From 80e6f94a8000586524fc138022280a13e74aa3f7 Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Wed, 15 Jul 2020 15:21:28 +0200 Subject: [PATCH 17/35] Add test --- tests/run/t8803.check | 16 ++++++++++++ tests/run/t8803.scala | 57 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+) create mode 100644 tests/run/t8803.check create mode 100644 tests/run/t8803.scala diff --git a/tests/run/t8803.check b/tests/run/t8803.check new file mode 100644 index 000000000000..bd26a0fb14aa --- /dev/null +++ b/tests/run/t8803.check @@ -0,0 +1,16 @@ +a +b +b +c +a +b +b +c +a +b +b +c +a +b +b +c diff --git a/tests/run/t8803.scala b/tests/run/t8803.scala new file mode 100644 index 000000000000..2e56180502b6 --- /dev/null +++ b/tests/run/t8803.scala @@ -0,0 +1,57 @@ +class A { + def m = "a" + protected def n = "a" +} + +trait B { + def m = "b" + protected def n = "b" +} + +class C extends A with B { + override def m = "c" + override protected def n = "c" + + val f1 = () => super[A].m + val f2 = () => super[B].m + val f3 = () => super.m + val f4 = () => this.m + + val g1 = new runtime.AbstractFunction0[String] { def apply() = C.super[A].m } + val g2 = new runtime.AbstractFunction0[String] { def apply() = C.super[B].m } + val g3 = new runtime.AbstractFunction0[String] { def apply() = C.super.m } + val g4 = new runtime.AbstractFunction0[String] { def apply() = C.this.m } + + val h1 = () => super[A].n + val h2 = () => super[B].n + val h3 = () => super.n + val h4 = () => this.n + + val i1 = new runtime.AbstractFunction0[String] { def apply() = C.super[A].n } + val i2 = new runtime.AbstractFunction0[String] { def apply() = C.super[B].n } + val i3 = new runtime.AbstractFunction0[String] { def apply() = C.super.n } + val i4 = new runtime.AbstractFunction0[String] { def apply() = C.this.n } +} + +object Test extends App { + val c = new C + println(c.f1()) + println(c.f2()) + println(c.f3()) + println(c.f4()) + + println(c.g1()) + println(c.g2()) + println(c.g3()) + println(c.g4()) + + println(c.h1()) + println(c.h2()) + println(c.h3()) + println(c.h4()) + + println(c.i1()) + println(c.i2()) + println(c.i3()) + println(c.i4()) +} From 1f18857d25dd8c85e0cbbcd5392702e6fdf6e967 Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Wed, 15 Jul 2020 16:10:08 +0200 Subject: [PATCH 18/35] Add test case t10290 --- tests/run/t10290.scala | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 tests/run/t10290.scala diff --git a/tests/run/t10290.scala b/tests/run/t10290.scala new file mode 100644 index 000000000000..262e66cd7234 --- /dev/null +++ b/tests/run/t10290.scala @@ -0,0 +1,28 @@ +trait A1 { + private val s = "A1" + def f = s +} + +trait A2 { + private val s = "A2" + def f = s +} + +class B extends A1 with A2 { + override def f = "B" + class C { + def t1 = B.super[A1].f + def t2 = B.super[A2].f + def t3 = B.this.f + } +} + +object Test { + def main(args : Array[String]) : Unit = { + val b = new B + val c = new b.C + assert(c.t1 == "A1") + assert(c.t2 == "A2") + assert(c.t3 == "B") + } +} From a79c8358aaebfcb371c0a7a7262780a84b9b153c Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Wed, 15 Jul 2020 19:30:03 +0200 Subject: [PATCH 19/35] Generate super accessor for traits Otherwise, we will need to support trees like `C.this.$outer.super[D]` as in Scala 2, which complicates invariants about super. --- .../tools/dotc/transform/ResolveSuper.scala | 14 ++++++++++++- .../tools/dotc/transform/SuperAccessors.scala | 20 ++++++++----------- tests/run/t10290.scala | 6 +++--- 3 files changed, 24 insertions(+), 16 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala b/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala index 5a73402b088d..7657ad21e31d 100644 --- a/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala +++ b/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala @@ -9,6 +9,8 @@ import SymUtils._ import Symbols._ import Decorators._ import DenotTransformers._ +import Names._ +import StdNames._ import NameOps._ import NameKinds._ import ResolveSuper._ @@ -79,10 +81,20 @@ object ResolveSuper { def rebindSuper(base: Symbol, acc: Symbol)(using Context): Symbol = { var bcs = base.info.baseClasses.dropWhile(acc.owner != _).tail var sym: Symbol = NoSymbol - val SuperAccessorName(memberName) = acc.name.unexpandedName + + var mix: Name = nme.EMPTY + val memberName = acc.name.unexpandedName match + case SuperAccessorName(ExpandPrefixName(name, mixName)) => + mix = mixName.toTypeName + name + case SuperAccessorName(name) => + name + report.debuglog(i"starting rebindsuper from $base of ${acc.showLocated}: ${acc.info} in $bcs, name = $memberName") + while (bcs.nonEmpty && sym == NoSymbol) { val other = bcs.head.info.nonPrivateDecl(memberName) + .filterWithPredicate(denot => mix.isEmpty || denot.symbol.owner.name == mix) .matchingDenotation(base.thisType, base.thisType.memberInfo(acc)) report.debuglog(i"rebindsuper ${bcs.head} $other deferred = ${other.symbol.is(Deferred)}") if other.exists && !other.symbol.is(Deferred) then diff --git a/compiler/src/dotty/tools/dotc/transform/SuperAccessors.scala b/compiler/src/dotty/tools/dotc/transform/SuperAccessors.scala index 3b9547e10532..03b87c1cf357 100644 --- a/compiler/src/dotty/tools/dotc/transform/SuperAccessors.scala +++ b/compiler/src/dotty/tools/dotc/transform/SuperAccessors.scala @@ -5,13 +5,13 @@ import dotty.tools.dotc.ast.{Trees, tpd} import scala.collection.mutable import ValueClasses.isMethodWithExtension import core._ -import Contexts._, Flags._, Symbols._, NameOps._, Trees._, Types.SuperType +import Contexts._, Flags._, Symbols._, Names._, StdNames._, NameOps._, Trees._ import TypeUtils._, SymUtils._ import DenotTransformers.DenotTransformer import Symbols._ import util.Spans._ import Decorators._ -import NameKinds.SuperAccessorName +import NameKinds.{ SuperAccessorName, ExpandPrefixName } /** This class adds super accessors for all super calls that either * appear in a trait or have as a target a member of some outer class. @@ -62,11 +62,12 @@ class SuperAccessors(thisPhase: DenotTransformer) { private val accDefs = newMutableSymbolMap[mutable.ListBuffer[Tree]] /** A super accessor call corresponding to `sel` */ - private def superAccessorCall(sel: Select)(using Context) = { + private def superAccessorCall(sel: Select, mixName: Name = nme.EMPTY)(using Context) = { val Select(qual, name) = sel val sym = sel.symbol val clazz = qual.symbol.asClass - var superName = SuperAccessorName(name.asTermName) + val preName = if (mixName.isEmpty) name.toTermName else ExpandPrefixName(name.toTermName, mixName.toTermName) + var superName = SuperAccessorName(preName) if (clazz.is(Trait)) superName = superName.expandedName(clazz) val superInfo = sel.tpe.widenSingleton.ensureMethodic @@ -147,19 +148,14 @@ class SuperAccessors(thisPhase: DenotTransformer) { } } - def mixIsTrait = sup.tpe match { - case SuperType(thisTpe, superTpe) => superTpe.typeSymbol.is(Trait) - } - val needAccessor = name.isTermName && { mix.name.isEmpty && (clazz.is(Trait) || clazz != currentClass || !validCurrentClass) || // SI-8803. If we access super[A] from an inner class (!= currentClass) or closure (validCurrentClass), - // where A is the superclass we need an accessor. If A is a parent trait we don't: in this case mixin - // will re-route the super call directly to the impl class (it's statically known). - !mix.name.isEmpty && (clazz != currentClass || !validCurrentClass) && !mixIsTrait + // where A is the superclass we need an accessor. + !mix.name.isEmpty && (clazz != currentClass || !validCurrentClass) } - if (needAccessor) atPhase(thisPhase.next)(superAccessorCall(sel)) + if (needAccessor) atPhase(thisPhase.next)(superAccessorCall(sel, mix.name)) else sel } diff --git a/tests/run/t10290.scala b/tests/run/t10290.scala index 262e66cd7234..eb345f5d6114 100644 --- a/tests/run/t10290.scala +++ b/tests/run/t10290.scala @@ -21,8 +21,8 @@ object Test { def main(args : Array[String]) : Unit = { val b = new B val c = new b.C - assert(c.t1 == "A1") - assert(c.t2 == "A2") - assert(c.t3 == "B") + assert(c.t1 == "A1", c.t1) + assert(c.t2 == "A2", c.t2) + assert(c.t3 == "B", c.t3) } } From b9502e31ba12a41e9e643cdc3027007e6a33e5db Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Wed, 15 Jul 2020 23:40:03 +0200 Subject: [PATCH 20/35] Special case Array.scala compilation Ported from https://github.com/scala/scala/pull/5944 --- .../src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala index 2efba7b83f4a..593645f0113e 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala @@ -364,9 +364,13 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { } else { mnode.visitVarInsn(asm.Opcodes.ALOAD, 0) - generatedType = - if (tree.symbol == defn.ArrayClass) ObjectReference - else classBTypeFromSymbol(claszSymbol) + // When compiling Array.scala, the constructor invokes `Array.this.super.`. The expectedType + // is `[Object` (computed by typeToBType, the type of This(Array) is `Array[T]`). If we would set + // the generatedType to `Array` below, the call to adapt at the end would fail. The situation is + // similar for primitives (`I` vs `Int`). + if (tree.symbol != defn.ArrayClass && !tree.symbol.isPrimitiveValueClass) { + generatedType = classBTypeFromSymbol(claszSymbol) + } } case DesugaredSelect(Ident(nme.EMPTY_PACKAGE), module) => From 90d00ec477274b30cc9153d75ffddeb548d057ce Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Wed, 15 Jul 2020 23:52:17 +0200 Subject: [PATCH 21/35] Handle super calls in blocks --- compiler/src/dotty/tools/dotc/ast/TreeInfo.scala | 1 + tests/run/i1441.scala | 10 ++++++++++ 2 files changed, 11 insertions(+) create mode 100644 tests/run/i1441.scala diff --git a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala index 46b9366283ac..bcd1a0661da8 100644 --- a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala +++ b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala @@ -837,6 +837,7 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] => final def splitAtSuper(constrStats: List[Tree])(implicit ctx: Context): (List[Tree], List[Tree]) = constrStats.toList match { case (sc: Apply) :: rest if sc.symbol.isConstructor => (sc :: Nil, rest) + case (block @ Block(_, sc: Apply)) :: rest if sc.symbol.isConstructor => (block :: Nil, rest) case stats => (Nil, stats) } diff --git a/tests/run/i1441.scala b/tests/run/i1441.scala new file mode 100644 index 000000000000..01a0e611f6bc --- /dev/null +++ b/tests/run/i1441.scala @@ -0,0 +1,10 @@ +class FrameworkTest(val cls: Class[_], val format: Int = 1, val expected: String) + +class FixtureFrameworkSuite +object FixtureFrameworkSuite extends FrameworkTest( + classOf[FixtureFrameworkSuite], + expected = "test" +) + +@main +def Test = FixtureFrameworkSuite From d002c7828d81b8a4bbdc64c66f58e40358f32fbd Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Tue, 21 Jul 2020 14:33:39 +0200 Subject: [PATCH 22/35] Remove unsupported options --- compiler/src/dotty/tools/dotc/config/ScalaSettings.scala | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index dc6ec6aaf65d..2aa0e6bedb79 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -35,8 +35,7 @@ class ScalaSettings extends Settings.SettingGroup { val help: Setting[Boolean] = BooleanSetting("-help", "Print a synopsis of standard options.") withAbbreviation "--help" val color: Setting[String] = ChoiceSetting("-color", "mode", "Colored output", List("always", "never"/*, "auto"*/), "always"/* "auto"*/) withAbbreviation "--color" val source: Setting[String] = ChoiceSetting("-source", "source version", "source version", List("3.0", "3.1", "3.0-migration", "3.1-migration"), "3.0").withAbbreviation("--source") - val target: Setting[String] = ChoiceSetting("-target", "target", "Target platform for object files. All JVM 1.5 targets are deprecated.", - List("jvm-1.5", "jvm-1.5-fjbg", "jvm-1.5-asm", "jvm-1.6", "jvm-1.7", "jvm-1.8", "jvm-9", "msil"), "jvm-1.8") withAbbreviation "--target" + val target: Setting[String] = ChoiceSetting("-target", "target", "Target platform for object files. All JVM 1.5 targets are deprecated.", List("jvm-1.8", "jvm-9"), "jvm-1.8") withAbbreviation "--target" val scalajs: Setting[Boolean] = BooleanSetting("-scalajs", "Compile in Scala.js mode (requires scalajs-library.jar on the classpath).") withAbbreviation "--scalajs" val unchecked: Setting[Boolean] = BooleanSetting("-unchecked", "Enable additional warnings where generated code depends on assumptions.") withAbbreviation "--unchecked" val uniqid: Setting[Boolean] = BooleanSetting("-uniqid", "Uniquely tag all identifiers in debugging output.") withAbbreviation "--unique-id" From b1a5234599b95fdaeed3a75b8aec93ba796d7c39 Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Tue, 21 Jul 2020 14:40:39 +0200 Subject: [PATCH 23/35] Add comment as doc --- .../dotty/tools/backend/jvm/BCodeSkelBuilder.scala | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala index 9dedad27b826..a43bbc10a815 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala @@ -116,8 +116,18 @@ trait BCodeSkelBuilder extends BCodeHelpers { // Because the assigments to both the module instance fields, and the fields of the module itself // are in the , these fields can be static + final. - // TODO should we do this transformation earlier, say in Constructors? Or would that just cause + // Should we do this transformation earlier, say in Constructors? Or would that just cause // pain for scala-{js, native}? + // + // @sjrd (https://github.com/lampepfl/dotty/pull/9181#discussion_r457458205): + // moving that before the back-end would make things significantly more complicated for + // Scala.js and Native. Both have a first-class concept of ModuleClass, and encode the + // singleton pattern of MODULE$ in a completely different way. In the Scala.js IR, there + // even isn't anything that corresponds to MODULE$ per se. + // + // So if you move this before the back-end, then Scala.js and Scala Native will have to + // reverse all the effects of this transformation, which would be counter-productive. + // TODO: enable once we change lazy val encoding // From d2419fc3b43e0b4acbf4a3ac27187f3445a614bb Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Tue, 21 Jul 2020 14:47:00 +0200 Subject: [PATCH 24/35] Fix usage of context --- compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala index a43bbc10a815..38bee90ea967 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala @@ -143,7 +143,7 @@ trait BCodeSkelBuilder extends BCodeHelpers { val (uptoSuperStats, remainingConstrStats) = splitAtSuper(impl.constr.rhs.asInstanceOf[Block].stats) val clInitSymbol: TermSymbol = if (clinits.nonEmpty) clinits.head.symbol.asTerm - else ctx.newSymbol( + else newSymbol( claszSymbol, nme.STATIC_CONSTRUCTOR, JavaStatic | Method, @@ -152,7 +152,7 @@ trait BCodeSkelBuilder extends BCodeHelpers { coord = claszSymbol.coord ) - val moduleField = ctx.newSymbol( + val moduleField = newSymbol( claszSymbol, str.MODULE_INSTANCE_FIELD.toTermName, JavaStatic | Final, From b05ce6c5cafa063d06a7d7d6e6445efda46723af Mon Sep 17 00:00:00 2001 From: Fengyun Liu Date: Tue, 21 Jul 2020 15:19:00 +0200 Subject: [PATCH 25/35] Update compiler/src/dotty/tools/dotc/config/ScalaSettings.scala Co-authored-by: Guillaume Martres --- compiler/src/dotty/tools/dotc/config/ScalaSettings.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index 2aa0e6bedb79..7a81a33ace66 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -35,7 +35,7 @@ class ScalaSettings extends Settings.SettingGroup { val help: Setting[Boolean] = BooleanSetting("-help", "Print a synopsis of standard options.") withAbbreviation "--help" val color: Setting[String] = ChoiceSetting("-color", "mode", "Colored output", List("always", "never"/*, "auto"*/), "always"/* "auto"*/) withAbbreviation "--color" val source: Setting[String] = ChoiceSetting("-source", "source version", "source version", List("3.0", "3.1", "3.0-migration", "3.1-migration"), "3.0").withAbbreviation("--source") - val target: Setting[String] = ChoiceSetting("-target", "target", "Target platform for object files. All JVM 1.5 targets are deprecated.", List("jvm-1.8", "jvm-9"), "jvm-1.8") withAbbreviation "--target" + val target: Setting[String] = ChoiceSetting("-target", "target", "Target platform for object files.", List("jvm-1.8", "jvm-9"), "jvm-1.8") withAbbreviation "--target" val scalajs: Setting[Boolean] = BooleanSetting("-scalajs", "Compile in Scala.js mode (requires scalajs-library.jar on the classpath).") withAbbreviation "--scalajs" val unchecked: Setting[Boolean] = BooleanSetting("-unchecked", "Enable additional warnings where generated code depends on assumptions.") withAbbreviation "--unchecked" val uniqid: Setting[Boolean] = BooleanSetting("-uniqid", "Uniquely tag all identifiers in debugging output.") withAbbreviation "--unique-id" From 5acfdbbedb09432c8edafb99b3bb3fc6e4e4799d Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Tue, 21 Jul 2020 15:23:08 +0200 Subject: [PATCH 26/35] Check that qual of Super is always This --- compiler/src/dotty/tools/dotc/transform/TreeChecker.scala | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala index 4f8ae9cb757a..d0d2afa53319 100644 --- a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala @@ -413,6 +413,10 @@ class TreeChecker extends Phase with SymTransformer { res } + override def typedSuper(tree: untpd.Super, pt: Type)(using Context): Tree = + assert(tree.qual.isInstanceOf[untpd.This], i"expect prefix of Super to be This, actual = ${tree.qual}") + super.typedSuper(tree, pt) + private def checkOwner(tree: untpd.Tree)(using Context): Unit = { def ownerMatches(symOwner: Symbol, ctxOwner: Symbol): Boolean = symOwner == ctxOwner || From 7e3c61b0d53eae1c7b8cbde4e1b3df344760d5e0 Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Tue, 21 Jul 2020 17:03:21 +0200 Subject: [PATCH 27/35] The qual of This could be Ident We should check the type instead, which is more semantic. --- compiler/src/dotty/tools/dotc/transform/TreeChecker.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala index d0d2afa53319..10f48ad182ea 100644 --- a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala @@ -414,7 +414,7 @@ class TreeChecker extends Phase with SymTransformer { } override def typedSuper(tree: untpd.Super, pt: Type)(using Context): Tree = - assert(tree.qual.isInstanceOf[untpd.This], i"expect prefix of Super to be This, actual = ${tree.qual}") + assert(tree.qual.tpe.isInstanceOf[ThisType], i"expect prefix of Super to be This, actual = ${tree.qual}") super.typedSuper(tree, pt) private def checkOwner(tree: untpd.Tree)(using Context): Unit = { From 867e56e9319af32e38149dcff9fd3ec033ccdbf1 Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Tue, 21 Jul 2020 15:36:40 +0200 Subject: [PATCH 28/35] Port scala.js changes More details: https://github.com/scala-js/scala-js/commit/e02799b636b577eb56593d08c21a7cb2e68719e8#diff-c9a195a739d51194e79f0655bc93bd61R2343-R2363 --- compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala b/compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala index d47042ca7480..75f2451374f3 100644 --- a/compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala +++ b/compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala @@ -1410,7 +1410,7 @@ class JSCodeGen()(using genCtx: Context) { */ private def genSuperCall(tree: Apply, isStat: Boolean): js.Tree = { implicit val pos = tree.span - val Apply(fun @ Select(sup @ Super(_, mix), _), args) = tree + val Apply(fun @ Select(sup @ Super(qual, _), _), args) = tree val sym = fun.symbol if (sym == defn.Any_getClass) { @@ -1419,8 +1419,11 @@ class JSCodeGen()(using genCtx: Context) { } else /*if (isScalaJSDefinedJSClass(currentClassSym)) { genJSSuperCall(tree, isStat) } else*/ { + /* #3013 `qual` can be `this.$outer()` in some cases since Scala 2.12, + * so we call `genExpr(qual)`, not just `genThis()`. + */ val superCall = genApplyMethodStatically( - genThis()(sup.span), sym, genActualArgs(sym, args)) + genExpr(qual), sym, genActualArgs(sym, args)) // Initialize the module instance just after the super constructor call. if (isStaticModule(currentClassSym) && !isModuleInitialized && From ef11f9b3da09cf2f1786267023185c079f79a251 Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Mon, 3 Aug 2020 13:12:01 +0200 Subject: [PATCH 29/35] Change default target to jvm-9 in test --- .../src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala | 2 +- .../test/dotty/tools/vulpix/TestConfiguration.scala | 10 +++++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala index 38bee90ea967..f47940797c41 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala @@ -129,7 +129,7 @@ trait BCodeSkelBuilder extends BCodeHelpers { // reverse all the effects of this transformation, which would be counter-productive. - // TODO: enable once we change lazy val encoding + // TODO: enable once we change lazy val encoding (https://github.com/lampepfl/dotty/issues/7140) // // Lazy val encoding assumes bitmap fields are non-static // diff --git a/compiler/test/dotty/tools/vulpix/TestConfiguration.scala b/compiler/test/dotty/tools/vulpix/TestConfiguration.scala index 01775432ca12..c067fdd8399b 100644 --- a/compiler/test/dotty/tools/vulpix/TestConfiguration.scala +++ b/compiler/test/dotty/tools/vulpix/TestConfiguration.scala @@ -8,7 +8,8 @@ object TestConfiguration { val noCheckOptions = Array( "-pagewidth", "120", - "-color:never" + "-color:never", + "-target", defaultTarget ) val checkOptions = Array( @@ -76,4 +77,11 @@ object TestConfiguration { /** Enables explicit nulls */ val explicitNullsOptions = defaultOptions and "-Yexplicit-nulls" + + /** Default target of the generated class files */ + private def defaultTarget: String = { + import scala.util.Properties.isJavaAtLeast + + if isJavaAtLeast("9") then "jvm-9" else "jvm-1.8" + } } From 38a5aec682324582f7e3ddafa7f0b09b0df84222 Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Mon, 3 Aug 2020 22:48:59 +0200 Subject: [PATCH 30/35] Try use JDK 11 as default --- .github/workflows/ci.yaml | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 4074bd8ab298..26a83dcd6efb 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -15,6 +15,9 @@ jobs: container: lampepfl/dotty:2020-04-24 steps: + - name: Set up JDK version + run: echo "::add-path::/usr/lib/jvm/java-11-openjdk-amd64/bin" + - name: Checkout cleanup script uses: actions/checkout@v2 @@ -55,6 +58,9 @@ jobs: container: lampepfl/dotty:2020-04-24 steps: + - name: Set up JDK version + run: echo "::add-path::/usr/lib/jvm/java-11-openjdk-amd64/bin" + - name: Checkout cleanup script uses: actions/checkout@v2 @@ -95,6 +101,9 @@ jobs: container: lampepfl/dotty:2020-04-24 steps: + - name: Set up JDK version + run: echo "::add-path::/usr/lib/jvm/java-11-openjdk-amd64/bin" + - name: Checkout cleanup script uses: actions/checkout@v2 @@ -174,7 +183,7 @@ jobs: - name: Test run: ./project/scripts/sbt sbt-dotty/scripted - test_java11: + test_java8: runs-on: self-hosted container: lampepfl/dotty:2020-04-24 if: ( @@ -216,13 +225,13 @@ jobs: - name: Test run: | - export PATH="/usr/lib/jvm/java-11-openjdk-amd64/bin:$PATH" + export PATH="/usr/lib/jvm/java-1.8.0-openjdk-amd64/bin:$PATH" ./project/scripts/sbt ";compile ;test" publish_nightly: runs-on: self-hosted container: lampepfl/dotty:2020-04-24 - needs: [test, test_bootstrapped, community_build, test_sbt, test_java11] + needs: [test, test_bootstrapped, community_build, test_sbt, test_java8] if: github.event_name == 'schedule' env: NIGHTLYBUILD: yes @@ -323,7 +332,7 @@ jobs: publish_release: runs-on: self-hosted container: lampepfl/dotty:2020-04-24 - needs: [test, test_bootstrapped, community_build, test_sbt, test_java11] + needs: [test, test_bootstrapped, community_build, test_sbt, test_java8] if: github.event_name == 'push' && startsWith(github.event.ref, 'refs/tags/') && !startsWith(github.event.ref, 'refs/tags/sbt-dotty-') @@ -475,7 +484,7 @@ jobs: publish_sbt_release: runs-on: self-hosted container: lampepfl/dotty:2020-04-24 - needs: [test, test_bootstrapped, community_build, test_sbt, test_java11] + needs: [test, test_bootstrapped, community_build, test_sbt, test_java8] if: github.event_name == 'push' && startsWith(github.event.ref, 'refs/tags/sbt-dotty-') From 479fd138944167979a016a8c61ed3f959e3a4f8e Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Mon, 3 Aug 2020 23:24:57 +0200 Subject: [PATCH 31/35] Set module class fields to static except bitmap fields Still we have a problem with the following magic: https://github.com/scala/scala/pull/7270/files#r221195225 It is a magic for two reasons: 1. It generates `getstatic` for `Select(This(O$), x)` 2. It is not affected by compilation order of classes For the second, suppose the backend compiles `A$B` before `A`. Then the class `A$B` does not see that `x` is static, which should cause problem at runtime: ``` object A { private[this] val x: Int = 10 class B { val y = x } } ``` The following are the reasons why it works in scalac: 1. inner classes always come after the outer class 2. non `private[this]` fields are accessed via accessor methods, which are not static 3. inside the module class, access to the fields are compiled after setting the static flag 4. the code generation specialize for trees like `Select(This(O$), x)`, where `x` is a static member --- .../dotty/tools/backend/jvm/BCodeSkelBuilder.scala | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala index f47940797c41..d9a6346168e9 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala @@ -16,6 +16,7 @@ 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._ +import dotty.tools.dotc.core.NameKinds._ import dotty.tools.dotc.core.Symbols._ import dotty.tools.dotc.core.Types._ import dotty.tools.dotc.core.Contexts._ @@ -129,14 +130,18 @@ trait BCodeSkelBuilder extends BCodeHelpers { // reverse all the effects of this transformation, which would be counter-productive. - // TODO: enable once we change lazy val encoding (https://github.com/lampepfl/dotty/issues/7140) + // TODO: remove `!f.name.is(LazyBitMapName)` once we change lazy val encoding + // https://github.com/lampepfl/dotty/issues/7140 // // Lazy val encoding assumes bitmap fields are non-static // // See `tests/run/given-var.scala` // - // for (f <- claszSymbol.info.decls.filter(_.isField)) - // f.setFlag(JavaStatic) + + claszSymbol.info.decls.foreach { f => + if f.isField && !f.name.is(LazyBitMapName) then + f.setFlag(JavaStatic) + } val (clinits, body) = impl.body.partition(stat => stat.isInstanceOf[DefDef] && stat.symbol.isStaticConstructor) From eaedbcadb1d83af84db47442f5c568b5dac39de8 Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Tue, 4 Aug 2020 16:59:27 +0200 Subject: [PATCH 32/35] Make flag order-independent at backend The magic in scalac is order-dependent: https://github.com/scala/scala/pull/7270/files#r221195225 See the previous commit for more detailed info. --- .../dotty/tools/backend/jvm/DottyBackendInterface.scala | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/backend/jvm/DottyBackendInterface.scala b/compiler/src/dotty/tools/backend/jvm/DottyBackendInterface.scala index 2458de77ba9c..15da9c4fcb9f 100644 --- a/compiler/src/dotty/tools/backend/jvm/DottyBackendInterface.scala +++ b/compiler/src/dotty/tools/backend/jvm/DottyBackendInterface.scala @@ -31,7 +31,7 @@ import tpd._ import scala.tools.asm import StdNames.{nme, str} -import NameKinds.{DefaultGetterName, ExpandedName} +import NameKinds.{DefaultGetterName, ExpandedName, LazyBitMapName} import Names.TermName import Annotations.Annotation import Names.Name @@ -131,8 +131,12 @@ object DottyBackendInterface { def isStaticConstructor(using Context): Boolean = (sym.isStaticMember && sym.isClassConstructor) || (sym.name eq nme.STATIC_CONSTRUCTOR) + /** Fields of static modules will be static at backend */ + def isStaticModuleField(using Context): Boolean = + sym.owner.isStaticModuleClass && sym.isField && !sym.name.is(LazyBitMapName) + def isStaticMember(using Context): Boolean = (sym ne NoSymbol) && - (sym.is(JavaStatic) || sym.isScalaStatic) + (sym.is(JavaStatic) || sym.isScalaStatic || sym.isStaticModuleField) // guard against no sumbol cause this code is executed to select which call type(static\dynamic) to use to call array.clone /** From 51783fe73fd48b01b0421a6cda6b3af7e9e20501 Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Tue, 4 Aug 2020 23:19:14 +0200 Subject: [PATCH 33/35] Use semantic names for bitmaps --- compiler/src/dotty/tools/dotc/transform/LazyVals.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/LazyVals.scala b/compiler/src/dotty/tools/dotc/transform/LazyVals.scala index 8ac80b3310d3..719e298fe067 100644 --- a/compiler/src/dotty/tools/dotc/transform/LazyVals.scala +++ b/compiler/src/dotty/tools/dotc/transform/LazyVals.scala @@ -401,7 +401,7 @@ class LazyVals extends MiniPhase with IdentityDenotTransformer { else { // need to create a new flag offsetSymbol = newSymbol(claz, offsetById, Synthetic, defn.LongType).enteredAfter(this) offsetSymbol.addAnnotation(Annotation(defn.ScalaStaticAnnot)) - val flagName = s"${StdNames.nme.BITMAP_PREFIX}$id".toTermName + val flagName = LazyBitMapName.fresh(id.toString.toTermName) val flagSymbol = newSymbol(claz, flagName, containerFlags, defn.LongType).enteredAfter(this) flag = ValDef(flagSymbol, Literal(Constant(0L))) val offsetTree = ValDef(offsetSymbol, getOffset.appliedTo(thizClass, Literal(Constant(flagName.toString)))) @@ -411,7 +411,7 @@ class LazyVals extends MiniPhase with IdentityDenotTransformer { case None => offsetSymbol = newSymbol(claz, offsetName(0), Synthetic, defn.LongType).enteredAfter(this) offsetSymbol.addAnnotation(Annotation(defn.ScalaStaticAnnot)) - val flagName = s"${StdNames.nme.BITMAP_PREFIX}0".toTermName + val flagName = LazyBitMapName.fresh("0".toTermName) val flagSymbol = newSymbol(claz, flagName, containerFlags, defn.LongType).enteredAfter(this) flag = ValDef(flagSymbol, Literal(Constant(0L))) val offsetTree = ValDef(offsetSymbol, getOffset.appliedTo(thizClass, Literal(Constant(flagName.toString)))) From ab59eeea8df9f5ca5906baf49f76c2c602580df8 Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Wed, 5 Aug 2020 01:04:47 +0200 Subject: [PATCH 34/35] Don't use Java 11 for community projects There are compilation errors due to changes between 8 and 11 - addition of `String.lines` shadows the deprecated `StringOps.lines` in stdlib * probblem for ScalaTest - JAXB APIs are removed * problem for BetterFiles --- .github/workflows/ci.yaml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 26a83dcd6efb..b87be0675067 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -101,9 +101,6 @@ jobs: container: lampepfl/dotty:2020-04-24 steps: - - name: Set up JDK version - run: echo "::add-path::/usr/lib/jvm/java-11-openjdk-amd64/bin" - - name: Checkout cleanup script uses: actions/checkout@v2 From 745d53c78e040aea398c38754bcc8ff3ad2df91d Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Wed, 5 Aug 2020 01:21:27 +0200 Subject: [PATCH 35/35] Address reviews --- .github/workflows/ci.yaml | 11 ++++++----- .../tools/backend/jvm/DottyBackendInterface.scala | 9 ++++++++- compiler/src/dotty/tools/dotc/ast/TreeInfo.scala | 4 ++-- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index b87be0675067..bbde24de7f90 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -15,7 +15,7 @@ jobs: container: lampepfl/dotty:2020-04-24 steps: - - name: Set up JDK version + - name: Set JDK 11 as default run: echo "::add-path::/usr/lib/jvm/java-11-openjdk-amd64/bin" - name: Checkout cleanup script @@ -58,7 +58,7 @@ jobs: container: lampepfl/dotty:2020-04-24 steps: - - name: Set up JDK version + - name: Set JDK 11 as default run: echo "::add-path::/usr/lib/jvm/java-11-openjdk-amd64/bin" - name: Checkout cleanup script @@ -190,6 +190,9 @@ jobs: github.event_name == 'schedule' steps: + - name: Set JDK 8 as default + run: echo "::add-path::/usr/lib/jvm/java-1.8.0-openjdk-amd64/bin" + - name: Checkout cleanup script uses: actions/checkout@v2 @@ -221,9 +224,7 @@ jobs: restore-keys: ${{ runner.os }}-general- - name: Test - run: | - export PATH="/usr/lib/jvm/java-1.8.0-openjdk-amd64/bin:$PATH" - ./project/scripts/sbt ";compile ;test" + run: ./project/scripts/sbt ";compile ;test" publish_nightly: runs-on: self-hosted diff --git a/compiler/src/dotty/tools/backend/jvm/DottyBackendInterface.scala b/compiler/src/dotty/tools/backend/jvm/DottyBackendInterface.scala index 15da9c4fcb9f..f983743281cc 100644 --- a/compiler/src/dotty/tools/backend/jvm/DottyBackendInterface.scala +++ b/compiler/src/dotty/tools/backend/jvm/DottyBackendInterface.scala @@ -131,7 +131,14 @@ object DottyBackendInterface { def isStaticConstructor(using Context): Boolean = (sym.isStaticMember && sym.isClassConstructor) || (sym.name eq nme.STATIC_CONSTRUCTOR) - /** Fields of static modules will be static at backend */ + /** Fields of static modules will be static at backend + * + * Note that lazy val encoding assumes bitmap fields are non-static. + * See also `genPlainClass` in `BCodeSkelBuilder.scala`. + * + * TODO: remove the special handing of `LazyBitMapName` once we swtich to + * the new lazy val encoding: https://github.com/lampepfl/dotty/issues/7140 + */ def isStaticModuleField(using Context): Boolean = sym.owner.isStaticModuleClass && sym.isField && !sym.name.is(LazyBitMapName) diff --git a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala index bcd1a0661da8..0aa3b65437fa 100644 --- a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala +++ b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala @@ -830,9 +830,9 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] => /** Return a pair consisting of (supercall, rest) * - * - supercall: the of superclass call, excluding trait constr calls + * - supercall: the superclass call, excluding trait constr calls * - * The supercall is always the first statement (if exists) + * The supercall is always the first statement (if it exists) */ final def splitAtSuper(constrStats: List[Tree])(implicit ctx: Context): (List[Tree], List[Tree]) = constrStats.toList match {