diff --git a/compiler/src/dotty/tools/dotc/transform/init/CheckGlobal.scala b/compiler/src/dotty/tools/dotc/transform/init/CheckGlobal.scala new file mode 100644 index 000000000000..11a88b5d348c --- /dev/null +++ b/compiler/src/dotty/tools/dotc/transform/init/CheckGlobal.scala @@ -0,0 +1,170 @@ +package dotty.tools.dotc +package transform +package init + +import core._ +import Flags._ +import Contexts._ +import Types._ +import Symbols._ +import Decorators._ +import printing.SyntaxHighlighting +import reporting.trace +import config.Printers.init + +import ast.tpd._ + +import scala.collection.mutable + + +/** Check that static objects can be initialized without cycles + * + * For the check to be fast, the algorithm uses coarse approximation. + * We construct a dependency graph as follows: + * + * - if a static object `O` is used in another class/static-object `B`, + * then O -> B + * - if a class `C` is instantiated in a another class/static-object `B`, + * then C -> B + * - if a static-object/class `A` extends another class `B`, + * then A -> B + * + * Given the graph above, we check if there exists cycles. + * + * This check does not need to care about objects in libraries, as separate + * compilation ensures that there cannot be cyles between two separately + * compiled projects. + */ +class CheckGlobal { + case class Dependency(sym: Symbol, source: Tree) + + /** Checking state */ + case class State(visited: mutable.Set[Symbol], path: Vector[Tree], obj: Symbol) { + def cyclicPath(using Context): String = if (path.isEmpty) "" else " Cyclic path:\n" + { + var indentCount = 0 + var last: String = "" + val sb = new StringBuilder + path.foreach { tree => + indentCount += 1 + val pos = tree.sourcePos + val prefix = s"${ " " * indentCount }-> " + val line = + if pos.source.exists then + val loc = "[ " + pos.source.file.name + ":" + (pos.line + 1) + " ]" + val code = SyntaxHighlighting.highlight(pos.lineContent.trim) + i"$code\t$loc" + else + tree.show + + if (last != line) sb.append(prefix + line + "\n") + + last = line + } + sb.toString + } + } + + case class Error(state: State) { + def issue(using Context): Unit = + report.warning("Cylic object dependencies detected." + state.cyclicPath, state.obj.defTree.srcPos) + } + + /** Summary of dependencies */ + private val summaryCache = mutable.Map.empty[Symbol, List[Dependency]] + + def check(obj: Symbol)(using Context): Unit = trace("checking " + obj.show, init) { + checkDependencies(obj, State(visited = mutable.Set.empty, path = Vector.empty, obj)) match + case Some(err) => err.issue + case _ => + } + + private def check(sym: Symbol, state: State)(using Context): Option[Error] = trace("checking " + sym.show, init) { + if sym == state.obj then + Some(Error(state)) + else if state.visited.contains(sym) then + None + else + state.visited += sym + checkDependencies(sym, state) + } + + private def checkDependencies(sym: Symbol, state: State)(using Context): Option[Error] = trace("checking dependencies of " + sym.show, init) { + val cls = if sym.is(Module) then sym.moduleClass.asClass else sym.asClass + val deps = analyze(cls) + Util.traceIndented("dependencies of " + sym.show + " = " + deps.map(_.sym.show).mkString(","), init) + var res: Option[Error] = None + // TODO: stop early + deps.foreach { dep => + if res.isEmpty then + val state2: State = state.copy(path = state.path :+ dep.source) + res = check(dep.sym, state2) + } + res + } + + private def analyze(cls: ClassSymbol)(using Context): List[Dependency] = + def isStaticObjectRef(sym: Symbol) = + sym.isTerm && !sym.is(Package) && sym.is(Module) + && sym.isStatic && sym.moduleClass != cls + + if (cls.defTree.isEmpty) Nil + else if (summaryCache.contains(cls)) summaryCache(cls) + else { + val cdef = cls.defTree.asInstanceOf[TypeDef] + val tpl = cdef.rhs.asInstanceOf[Template] + + // ignore separately compiled classes + if !tpl.unforced.isInstanceOf[List[_]] then return Nil + + var dependencies: List[Dependency] = Nil + val traverser = new TreeTraverser { + override def traverse(tree: Tree)(using Context): Unit = + tree match { + case tree: RefTree if isStaticObjectRef(tree.symbol) => + dependencies = Dependency(tree.symbol, tree) :: dependencies + + case tdef: TypeDef => + // don't go into nested classes + + case tree: New => + dependencies = Dependency(tree.tpe.classSymbol, tree) :: dependencies + + case _ => + traverseChildren(tree) + } + } + + def typeRefOf(tp: Type): TypeRef = tp.dealias.typeConstructor match { + case tref: TypeRef => tref + case hklambda: HKTypeLambda => typeRefOf(hklambda.resType) + } + + def addStaticOuterDep(tp: Type, source: Tree): Unit = + tp match + case NoPrefix => + case tmref: TermRef => + if isStaticObjectRef(tmref.symbol) then + dependencies = Dependency(tmref.symbol, source) :: dependencies + case ThisType(tref) => + val obj = tref.symbol.sourceModule + if isStaticObjectRef(obj) then + dependencies = Dependency(obj, source) :: dependencies + case _ => + throw new Exception("unexpected type: " + tp) + + // TODO: the traverser might create duplicate entries for parents + tpl.parents.foreach { tree => + val tp = tree.tpe + val tref = typeRefOf(tp) + dependencies = Dependency(tp.classSymbol, tree) :: dependencies + addStaticOuterDep(tref.prefix, tree) + } + + traverser.traverse(tpl) + summaryCache(cls) = dependencies + dependencies + } + + def debugCache(using Context) = + summaryCache.map(_.show + " -> " + _.map(_.sym.show).mkString(",")).mkString("\n") +} \ No newline at end of file diff --git a/compiler/src/dotty/tools/dotc/transform/init/Checker.scala b/compiler/src/dotty/tools/dotc/transform/init/Checker.scala index c7607827f639..43cc94ed60da 100644 --- a/compiler/src/dotty/tools/dotc/transform/init/Checker.scala +++ b/compiler/src/dotty/tools/dotc/transform/init/Checker.scala @@ -26,6 +26,8 @@ class Checker extends MiniPhase { // cache of class summary private val baseEnv = Env(null) + val globalChecker = new CheckGlobal + override val runsAfter = Set(Pickler.name) override def isEnabled(using Context): Boolean = @@ -58,6 +60,10 @@ class Checker extends MiniPhase { ) Checking.checkClassBody(tree) + + // check cycles of object dependencies + if cls.is(Flags.Module) && cls.isStatic then + globalChecker.check(cls.sourceModule) } tree diff --git a/compiler/src/dotty/tools/dotc/transform/init/Checking.scala b/compiler/src/dotty/tools/dotc/transform/init/Checking.scala index e47c3b08d264..8e3cc2d4af5c 100644 --- a/compiler/src/dotty/tools/dotc/transform/init/Checking.scala +++ b/compiler/src/dotty/tools/dotc/transform/init/Checking.scala @@ -39,7 +39,18 @@ object Checking { safePromoted: mutable.Set[Potential], // Potentials that can be safely promoted env: Env ) { - def withOwner(sym: Symbol): State = copy(env = env.withOwner(sym)) + def withOwner[T](sym: Symbol)(op: State ?=> T): T = + val state = this.copy(env = env.withOwner(sym)) + val res = op(using state) + this.visited = state.visited + res + + + def withStep[T](step: Tree)(op: State ?=> T): T = + val state: State = this.copy(path = path :+ step) + val res = op(using state) + this.visited = state.visited + res def test(op: State ?=> Errors): Errors = { val savedVisited = visited @@ -60,11 +71,12 @@ object Checking { } else { state.visited = state.visited + eff - val state2: State = state.copy(path = state.path :+ eff.source) - eff match { - case eff: Promote => Checking.checkPromote(eff)(using state2) - case eff: FieldAccess => Checking.checkFieldAccess(eff)(using state2) - case eff: MethodCall => Checking.checkMethodCall(eff)(using state2) + state.withStep(eff.source) { + eff match { + case eff: Promote => Checking.checkPromote(eff) + case eff: FieldAccess => Checking.checkFieldAccess(eff) + case eff: MethodCall => Checking.checkMethodCall(eff) + } } } } @@ -118,11 +130,11 @@ object Checking { def checkConstructor(ctor: Symbol, tp: Type, source: Tree)(using state: State): Unit = traceOp("checking " + ctor.show, init) { val cls = ctor.owner val classDef = cls.defTree - if (!classDef.isEmpty) { - given State = state.withOwner(cls) - if (ctor.isPrimaryConstructor) checkClassBody(classDef.asInstanceOf[TypeDef]) - else checkSecondaryConstructor(ctor) - } + if (!classDef.isEmpty) + state.withOwner(cls) { + if (ctor.isPrimaryConstructor) checkClassBody(classDef.asInstanceOf[TypeDef]) + else checkSecondaryConstructor(ctor) + } } def checkSecondaryConstructor(ctor: Symbol)(using state: State): Unit = traceOp("checking " + ctor.show, init) { diff --git a/tests/init/neg/i9176.scala b/tests/init/neg/i9176.scala new file mode 100644 index 000000000000..c70e87d1872a --- /dev/null +++ b/tests/init/neg/i9176.scala @@ -0,0 +1,9 @@ +class Foo(val opposite: Foo) +case object A extends Foo(B) // error +case object B extends Foo(A) // error +object Test { + def main(args: Array[String]): Unit = { + println(A.opposite) + println(B.opposite) + } +} \ No newline at end of file diff --git a/tests/init/neg/t5366.scala b/tests/init/neg/t5366.scala new file mode 100644 index 000000000000..0d2fe6e9c384 --- /dev/null +++ b/tests/init/neg/t5366.scala @@ -0,0 +1,15 @@ +class IdAndMsg(val id: Int, val msg: String = "") + +case object ObjA extends IdAndMsg(1) // error +case object ObjB extends IdAndMsg(2) // error + +object IdAndMsg { // error + val values = List(ObjA , ObjB) +} + +object Test { + def main(args: Array[String]): Unit = { + ObjA + println(IdAndMsg.values) + } +} \ No newline at end of file diff --git a/tests/init/neg/t9115.scala b/tests/init/neg/t9115.scala new file mode 100644 index 000000000000..fe204e64c998 --- /dev/null +++ b/tests/init/neg/t9115.scala @@ -0,0 +1,8 @@ +object D { // error + def aaa = 1 //that’s the reason + class Z (depends: Any) + case object D1 extends Z(aaa) // 'null' when calling D.D1 first time // error + case object D2 extends Z(aaa) // 'null' when calling D.D2 first time // error + println(D1) + println(D2) +} \ No newline at end of file diff --git a/tests/init/neg/t9261.scala b/tests/init/neg/t9261.scala new file mode 100644 index 000000000000..378dad51b3a6 --- /dev/null +++ b/tests/init/neg/t9261.scala @@ -0,0 +1,3 @@ +sealed abstract class OrderType(val reverse: OrderType) +case object Buy extends OrderType(Sell) // error +case object Sell extends OrderType(Buy) // error diff --git a/tests/init/neg/t9312.scala b/tests/init/neg/t9312.scala new file mode 100644 index 000000000000..fb3d53c81eb9 --- /dev/null +++ b/tests/init/neg/t9312.scala @@ -0,0 +1,23 @@ +object DeadLockTest { + def main(args: Array[String]): Unit = { + def run(block: => Unit): Unit = + new Thread(new Runnable {def run(): Unit = block}).start() + + run {println(Parent.Child1)} + run {println(Parent.Child2)} + + } + + object Parent { // error + trait Child { + Thread.sleep(2000) // ensure concurrent behavior + val parent = Parent + def siblings = parent.children - this + } + + object Child1 extends Child // error + object Child2 extends Child // error + + final val children = Set(Child1, Child2) // error + } +}