From ce8eac147c877907c422bb84d5cb645e9fd8bbd3 Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Wed, 13 Jan 2021 10:51:23 +0100 Subject: [PATCH 1/3] Fix #8022: Refactor MoveStatics We still keep two phases, but the code is simplified. We cannot merge MoveStatics with CheckStatic because the early phases are not able to handle static calls, thus it has to be at the end of the pipeline. Meanwhile, we want checks to happen as early as possible, thus CheckStatic has to be in the beginning of the pipeline. --- .../dotty/tools/dotc/reporting/messages.scala | 2 +- .../tools/dotc/transform/CheckStatic.scala | 19 +---- .../tools/dotc/transform/MoveStatics.scala | 84 ++++++++++++------- tests/run/statics.scala | 25 +++--- 4 files changed, 70 insertions(+), 60 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index 2431fc969675..f79ae8e2c929 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -1943,7 +1943,7 @@ import transform.SymUtils._ } class StaticFieldsOnlyAllowedInObjects(member: Symbol)(using Context) extends SyntaxMsg(StaticFieldsOnlyAllowedInObjectsID) { - def msg = em"${hl("@static")} $member in ${member.owner} must be defined inside an ${hl("object")}." + def msg = em"${hl("@static")} $member in ${member.owner} must be defined inside a static ${hl("object")}." def explain = em"${hl("@static")} members are only allowed inside objects." } diff --git a/compiler/src/dotty/tools/dotc/transform/CheckStatic.scala b/compiler/src/dotty/tools/dotc/transform/CheckStatic.scala index dc931813b1f9..3edddadfb2ae 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckStatic.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckStatic.scala @@ -34,7 +34,7 @@ class CheckStatic extends MiniPhase { var hadNonStaticField = false for (defn <- defns) if (defn.symbol.isScalaStatic) { - if (!ctx.owner.is(Module)) + if (!ctx.owner.isStaticOwner) report.error(StaticFieldsOnlyAllowedInObjects(defn.symbol), defn.srcPos) defn.symbol.resetFlag(JavaStatic) @@ -59,23 +59,6 @@ class CheckStatic extends MiniPhase { tree } - - override def transformSelect(tree: tpd.Select)(using Context): tpd.Tree = - if (tree.symbol.hasAnnotation(defn.ScalaStaticAnnot)) { - val symbolWhitelist = tree.symbol.ownersIterator.flatMap(x => if (x.is(Flags.Module)) List(x, x.companionModule) else List(x)).toSet - def isSafeQual(t: Tree): Boolean = // follow the desugared paths created by typer - t match { - case t: This => true - case t: Select => isSafeQual(t.qualifier) && symbolWhitelist.contains(t.symbol) - case t: Ident => symbolWhitelist.contains(t.symbol) - case t: Block => t.stats.forall(tpd.isPureExpr) && isSafeQual(t.expr) - case _ => false - } - if (isSafeQual(tree.qualifier)) - ref(tree.symbol) - else tree - } - else tree } object CheckStatic { diff --git a/compiler/src/dotty/tools/dotc/transform/MoveStatics.scala b/compiler/src/dotty/tools/dotc/transform/MoveStatics.scala index 6883c5c1086b..107c252e637b 100644 --- a/compiler/src/dotty/tools/dotc/transform/MoveStatics.scala +++ b/compiler/src/dotty/tools/dotc/transform/MoveStatics.scala @@ -1,26 +1,29 @@ -package dotty.tools.dotc.transform - -import dotty.tools.dotc.ast.{Trees, tpd} -import dotty.tools.dotc.core.Annotations.Annotation -import dotty.tools.dotc.core.Contexts._ -import dotty.tools.dotc.core.DenotTransformers.SymTransformer -import dotty.tools.dotc.core.SymDenotations.SymDenotation -import dotty.tools.dotc.core.NameOps._ -import dotty.tools.dotc.core.Flags -import dotty.tools.dotc.core.Names.Name -import dotty.tools.dotc.core.StdNames.nme -import dotty.tools.dotc.core.Symbols._ -import dotty.tools.dotc.core.Types.MethodType -import dotty.tools.dotc.transform.MegaPhase.MiniPhase +package dotty.tools.dotc +package transform -object MoveStatics { - val name: String = "moveStatic" -} +import core._ +import Flags._ +import Contexts._ +import Symbols._ +import Decorators._ +import DenotTransformers.SymTransformer +import Types.MethodType +import Annotations.Annotation +import SymDenotations.SymDenotation +import Names.Name +import StdNames.nme +import NameOps._ + +import reporting._ +import ast._ + +import SymUtils._ +import MegaPhase._ /** Move static methods from companion to the class itself */ class MoveStatics extends MiniPhase with SymTransformer { + import ast.tpd._ - import tpd._ override def phaseName: String = MoveStatics.name def transformSym(sym: SymDenotation)(using Context): SymDenotation = @@ -32,14 +35,30 @@ class MoveStatics extends MiniPhase with SymTransformer { } else sym + override def transformSelect(tree: tpd.Select)(using Context): tpd.Tree = + if (tree.symbol.hasAnnotation(defn.ScalaStaticAnnot)) { + def isSafeQual(t: Tree): Boolean = // follow the desugared paths created by typer + t match { + case t: This => true + case t: Select => isSafeQual(t.qualifier) + case t: Block => t.stats.forall(tpd.isPureExpr) && isSafeQual(t.expr) + case _ => false + } + + if (isSafeQual(tree.qualifier)) + ref(tree.symbol) + else + Block(tree.qualifier :: Nil, ref(tree.symbol)) + } + else tree + + override def transformStats(trees: List[Tree])(using Context): List[Tree] = if (ctx.owner.is(Flags.Package)) { val (classes, others) = trees.partition(x => x.isInstanceOf[TypeDef] && x.symbol.isClass) val pairs = classes.groupBy(_.symbol.name.stripModuleClassSuffix).asInstanceOf[Map[Name, List[TypeDef]]] def rebuild(orig: TypeDef, newBody: List[Tree]): Tree = { - if (orig eq null) return EmptyTree - val staticFields = newBody.filter(x => x.isInstanceOf[ValDef] && x.symbol.hasAnnotation(defn.ScalaStaticAnnot)).asInstanceOf[List[ValDef]] val newBodyWithStaticConstr = if (staticFields.nonEmpty) { @@ -61,21 +80,30 @@ class MoveStatics extends MiniPhase with SymTransformer { assert(companion != module) if (!module.symbol.is(Flags.Module)) move(companion, module) else { - val allMembers = - (if (companion != null) {companion.rhs.asInstanceOf[Template].body} else Nil) ++ - module.rhs.asInstanceOf[Template].body - val (newModuleBody, newCompanionBody) = allMembers.partition(x => {assert(x.symbol.exists); x.symbol.owner == module.symbol}) - Trees.flatten(rebuild(companion, newCompanionBody) :: rebuild(module, newModuleBody) :: Nil) + val moduleTmpl = module.rhs.asInstanceOf[Template] + val companionTmpl = companion.rhs.asInstanceOf[Template] + val (staticDefs, remainingDefs) = moduleTmpl.body.partition { + case memberDef: MemberDef => memberDef.symbol.isScalaStatic + case _ => false + } + + rebuild(companion, companionTmpl.body ++ staticDefs) :: rebuild(module, remainingDefs) :: Nil } } val newPairs = for ((name, classes) <- pairs) yield - if (classes.tail.isEmpty) - if (classes.head.symbol.is(Flags.Module)) move(classes.head, null) - else List(rebuild(classes.head, classes.head.rhs.asInstanceOf[Template].body)) + if (classes.tail.isEmpty) { + val classDef = classes.head + val tmpl = classDef.rhs.asInstanceOf[Template] + rebuild(classDef, tmpl.body) :: Nil + } else move(classes.head, classes.tail.head) Trees.flatten(newPairs.toList.flatten ++ others) } else trees } + +object MoveStatics { + val name: String = "moveStatic" +} diff --git a/tests/run/statics.scala b/tests/run/statics.scala index d4d635400d93..85f467cabe8c 100644 --- a/tests/run/statics.scala +++ b/tests/run/statics.scala @@ -1,30 +1,29 @@ import scala.annotation.static -class Foo{ +class Foo { class Bar { def qwa = - Bar.field + Bar.field // 0: invokestatic #31 // Method Foo$Bar$.field:()I // 3: ireturn } object Bar { - @static val field = 1 } } -object Foo{ - @static - def method = 1 +object Foo { + @static + def method = 1 - @static - val field = 2 + @static + val field = 2 - @static - var mutable = 3 + @static + var mutable = 3 - @static - def accessor = field + @static + def accessor = field } object Test { @@ -34,7 +33,7 @@ object Test { } } -class WithLazies{ +class WithLazies { lazy val s = 1 // 98: getstatic #30 // Field WithLazies$.OFFSET$0:J } From 47f2fc45f3e7d34965d2bf34715b541f3fa462d6 Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Thu, 14 Jan 2021 17:25:03 +0100 Subject: [PATCH 2/3] Fix #11100: add test --- tests/neg/i11100.scala | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 tests/neg/i11100.scala diff --git a/tests/neg/i11100.scala b/tests/neg/i11100.scala new file mode 100644 index 000000000000..b27dfbb5e9b1 --- /dev/null +++ b/tests/neg/i11100.scala @@ -0,0 +1,14 @@ +import scala.annotation.static + +class C { + val a: Int = 3 + class D + object D { + @static def foo: Int = a * a // error + } +} + +@main +def Test = + val c = new C + println(c.D.foo) From cdebeb620c4d63716fcc8383d1335d8dca60a4dc Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Fri, 15 Jan 2021 10:50:58 +0100 Subject: [PATCH 3/3] Remove unnecessary transformSelect The logic is handled by the phase `SelectStatic` --- .../tools/dotc/transform/MoveStatics.scala | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/MoveStatics.scala b/compiler/src/dotty/tools/dotc/transform/MoveStatics.scala index 107c252e637b..ce71734e16d9 100644 --- a/compiler/src/dotty/tools/dotc/transform/MoveStatics.scala +++ b/compiler/src/dotty/tools/dotc/transform/MoveStatics.scala @@ -35,24 +35,6 @@ class MoveStatics extends MiniPhase with SymTransformer { } else sym - override def transformSelect(tree: tpd.Select)(using Context): tpd.Tree = - if (tree.symbol.hasAnnotation(defn.ScalaStaticAnnot)) { - def isSafeQual(t: Tree): Boolean = // follow the desugared paths created by typer - t match { - case t: This => true - case t: Select => isSafeQual(t.qualifier) - case t: Block => t.stats.forall(tpd.isPureExpr) && isSafeQual(t.expr) - case _ => false - } - - if (isSafeQual(tree.qualifier)) - ref(tree.symbol) - else - Block(tree.qualifier :: Nil, ref(tree.symbol)) - } - else tree - - override def transformStats(trees: List[Tree])(using Context): List[Tree] = if (ctx.owner.is(Flags.Package)) { val (classes, others) = trees.partition(x => x.isInstanceOf[TypeDef] && x.symbol.isClass)