Merge pull request #9335 from dotty-staging/init-refine

Initialization checker improvement

Author: anatoliykmetyuk

compiler/src/dotty/tools/dotc/transform/init/Checker.scala

@@ -23,7 +23,7 @@ class Checker extends MiniPhase {
   val phaseName = "initChecker"
 
   // cache of class summary
-  private val baseEnv = Env(null, mutable.Map.empty)
+  private val baseEnv = Env(null)
 
   override val runsAfter = Set(Pickler.name)
 
@@ -52,7 +52,7 @@ class Checker extends MiniPhase {
         thisClass = cls,
         fieldsInited = mutable.Set.empty,
         parentsInited = mutable.Set.empty,
-        env = baseEnv.withCtx(ctx)
+        env = baseEnv.withCtx(ctx.withOwner(cls))
       )
 
       Checking.checkClassBody(tree)

compiler/src/dotty/tools/dotc/transform/init/Checking.scala

@@ -30,7 +30,7 @@ object Checking {
    *
    */
   case class State(
-    visited: mutable.Set[Effect],              // effects that have been expanded
+    visited: mutable.Set[Effect],              // effects that have been checked
     path: Vector[Tree],                        // the path that leads to the current effect
     thisClass: ClassSymbol,                    // the concrete class of `this`
     fieldsInited: mutable.Set[Symbol],
@@ -41,6 +41,8 @@ object Checking {
       visited += eff
       copy(path = this.path :+ eff.source)
     }
+
+    def withOwner(sym: Symbol): State = copy(env = env.withOwner(sym))
   }
 
   private implicit def theEnv(implicit state: State): Env = state.env
@@ -51,17 +53,19 @@ object Checking {
    *  However, summarization can be done lazily on-demand to improve
    *  performance.
    */
-  def checkClassBody(cdef: TypeDef)(implicit state: State): Unit = traceOp("checking " + cdef.symbol.show, init) {
+  def checkClassBody(cdef: TypeDef)(implicit state: State): Unit = {
+    traceIndented("\n\n>>>> checking " + cdef.symbol.show, init)
+
     val cls = cdef.symbol.asClass
     val tpl = cdef.rhs.asInstanceOf[Template]
 
     // mark current class as initialized, required for linearization
     state.parentsInited += cls
 
-    def checkClassBodyStat(tree: Tree)(using Context): Unit = traceOp("checking " + tree.show, init) {
+    def checkClassBodyStat(tree: Tree)(implicit state: State): Unit = traceOp("checking " + tree.show, init) {
       tree match {
         case vdef : ValDef =>
-          val (pots, effs) = Summarization.analyze(vdef.rhs)(theEnv.withOwner(vdef.symbol))
+          val (pots, effs) = Summarization.analyze(vdef.rhs)
           theEnv.summaryOf(cls).cacheFor(vdef.symbol, (pots, effs))
           if (!vdef.symbol.is(Flags.Lazy)) {
             checkEffectsIn(effs, cls)
@@ -79,15 +83,31 @@ object Checking {
     // see spec 5.1 about "Template Evaluation".
     // https://www.scala-lang.org/files/archive/spec/2.13/05-classes-and-objects.html
 
-    def checkCtor(ctor: Symbol, tp: Type, source: Tree)(using Context): Unit = {
+    def checkConstructor(ctor: Symbol, tp: Type, source: Tree)(implicit state: State): Unit = traceOp("checking " + ctor.show, init) {
       val cls = ctor.owner
       val classDef = cls.defTree
       if (!classDef.isEmpty) {
-        if (ctor.isPrimaryConstructor) checkClassBody(classDef.asInstanceOf[TypeDef])
-        else checkSecondaryConstructor(ctor)
+        if (ctor.isPrimaryConstructor) checkClassBody(classDef.asInstanceOf[TypeDef])(state.withOwner(cls))
+        else checkSecondaryConstructor(ctor)(state.withOwner(cls))
+      }
+    }
+
+    def checkSecondaryConstructor(ctor: Symbol)(implicit state: State): Unit = traceOp("checking " + ctor.show, init) {
+      val Block(ctorCall :: stats, expr) = ctor.defTree.asInstanceOf[DefDef].rhs
+      val cls = ctor.owner.asClass
+
+      traceOp("check ctor: " + ctorCall.show, init) {
+        val ctor = ctorCall.symbol
+        if (ctor.isPrimaryConstructor)
+          checkClassBody(cls.defTree.asInstanceOf[TypeDef])
+        else
+          checkSecondaryConstructor(ctor)
+      }
+
+      (stats :+ expr).foreach { stat =>
+        val (_, effs) = Summarization.analyze(stat)(theEnv.withOwner(ctor))
+        checkEffectsIn(effs, cls)
       }
-      else if (!cls.isOneOf(Flags.EffectivelyOpenFlags))
-        report.warning("Inheriting non-open class may cause initialization errors", source.srcPos)
     }
 
     cls.paramAccessors.foreach { acc =>
@@ -100,61 +120,42 @@ object Checking {
     tpl.parents.foreach {
       case tree @ Block(_, parent) =>
         val (ctor, _, _) = decomposeCall(parent)
-        checkCtor(ctor.symbol, parent.tpe, tree)
+        checkConstructor(ctor.symbol, parent.tpe, tree)
 
       case tree @ Apply(Block(_, parent), _) =>
         val (ctor, _, _) = decomposeCall(parent)
-        checkCtor(ctor.symbol, tree.tpe, tree)
+        checkConstructor(ctor.symbol, tree.tpe, tree)
 
       case parent : Apply =>
         val (ctor, _, argss) = decomposeCall(parent)
-        checkCtor(ctor.symbol, parent.tpe, parent)
+        checkConstructor(ctor.symbol, parent.tpe, parent)
 
       case ref =>
         val cls = ref.tpe.classSymbol.asClass
         if (!state.parentsInited.contains(cls) && cls.primaryConstructor.exists)
-          checkCtor(cls.primaryConstructor, ref.tpe, ref)
+          checkConstructor(cls.primaryConstructor, ref.tpe, ref)
     }
 
     // check class body
     tpl.body.foreach { checkClassBodyStat(_) }
   }
 
-  def checkSecondaryConstructor(ctor: Symbol)(implicit state: State): Unit = traceOp("checking " + ctor.show, init) {
-    val Block(ctorCall :: stats, expr) = ctor.defTree.asInstanceOf[DefDef].rhs
-    val cls = ctor.owner.asClass
-
-    traceOp("check ctor: " + ctorCall.show, init) {
-      val ctor = ctorCall.symbol
-      if (ctor.isPrimaryConstructor)
-        checkClassBody(cls.defTree.asInstanceOf[TypeDef])
-      else
-        checkSecondaryConstructor(ctor)
-    }
-
-    (stats :+ expr).foreach { stat =>
-      val (_, effs) = Summarization.analyze(stat)(theEnv.withOwner(ctor))
-      checkEffectsIn(effs, cls)
-    }
-  }
-
   private def checkEffectsIn(effs: Effects, cls: ClassSymbol)(implicit state: State): Unit = traceOp("checking effects " + Effects.show(effs), init) {
-    val rebased = Effects.asSeenFrom(effs, ThisRef(state.thisClass)(null), cls, Potentials.empty)
     for {
-      eff <- rebased
+      eff <- effs
       error <- check(eff)
     } error.issue
   }
 
   private def check(eff: Effect)(implicit state: State): Errors =
-    if (state.visited.contains(eff)) Errors.empty else trace("checking effect " + eff.show, init, errs => Errors.show(errs.asInstanceOf[Errors])) {
+    if (state.visited.contains(eff)) Errors.empty
+    else trace("checking effect " + eff.show, init, errs => Errors.show(errs.asInstanceOf[Errors])) {
       implicit val state2: State = state.withVisited(eff)
 
       eff match {
         case Promote(pot) =>
           pot match {
-            case pot @ ThisRef(cls) =>
-              assert(cls == state.thisClass, "unexpected potential " + pot.show)
+            case pot: ThisRef =>
               PromoteThis(pot, eff.source, state2.path).toErrors
 
             case _: Cold =>
@@ -180,18 +181,14 @@ object Checking {
         case FieldAccess(pot, field) =>
 
           pot match {
-            case ThisRef(cls) =>
-              assert(cls == state.thisClass, "unexpected potential " + pot.show)
-
-              val target = resolve(cls, field)
+            case _: ThisRef =>
+              val target = resolve(state.thisClass, field)
               if (target.is(Flags.Lazy)) check(MethodCall(pot, target)(eff.source))
               else if (!state.fieldsInited.contains(target)) AccessNonInit(target, state2.path).toErrors
               else Errors.empty
 
-            case SuperRef(ThisRef(cls), supercls) =>
-              assert(cls == state.thisClass, "unexpected potential " + pot.show)
-
-              val target = resolveSuper(cls, supercls, field)
+            case SuperRef(_: ThisRef, supercls) =>
+              val target = resolveSuper(state.thisClass, supercls, field)
               if (target.is(Flags.Lazy)) check(MethodCall(pot, target)(eff.source))
               else if (!state.fieldsInited.contains(target)) AccessNonInit(target, state2.path).toErrors
               else Errors.empty
@@ -217,10 +214,8 @@ object Checking {
 
         case MethodCall(pot, sym) =>
           pot match {
-            case thisRef @ ThisRef(cls) =>
-              assert(cls == state.thisClass, "unexpected potential " + pot.show)
-
-              val target = resolve(cls, sym)
+            case thisRef: ThisRef =>
+              val target = resolve(state.thisClass, sym)
               if (!target.isOneOf(Flags.Method | Flags.Lazy))
                 check(FieldAccess(pot, target)(eff.source))
               else if (target.isInternal) {
@@ -229,10 +224,8 @@ object Checking {
               }
               else CallUnknown(target, eff.source, state2.path).toErrors
 
-            case SuperRef(thisRef @ ThisRef(cls), supercls) =>
-              assert(cls == state.thisClass, "unexpected potential " + pot.show)
-
-              val target = resolveSuper(cls, supercls, sym)
+            case SuperRef(thisRef: ThisRef, supercls) =>
+              val target = resolveSuper(state.thisClass, supercls, sym)
               if (!target.is(Flags.Method))
                 check(FieldAccess(pot, target)(eff.source))
               else if (target.isInternal) {
@@ -272,17 +265,13 @@ object Checking {
     pot match {
       case MethodReturn(pot1, sym) =>
         pot1 match {
-          case thisRef @ ThisRef(cls) =>
-            assert(cls == state.thisClass, "unexpected potential " + pot.show)
-
-            val target = resolve(cls, sym)
+          case thisRef: ThisRef =>
+            val target = resolve(state.thisClass, sym)
             if (target.isInternal) (thisRef.potentialsOf(target), Effects.empty)
             else Summary.empty // warning already issued in call effect
 
-          case SuperRef(thisRef @ ThisRef(cls), supercls) =>
-            assert(cls == state.thisClass, "unexpected potential " + pot.show)
-
-            val target = resolveSuper(cls, supercls, sym)
+          case SuperRef(thisRef: ThisRef, supercls) =>
+            val target = resolveSuper(state.thisClass, supercls, sym)
             if (target.isInternal) (thisRef.potentialsOf(target), Effects.empty)
             else Summary.empty // warning already issued in call effect
 
@@ -314,17 +303,13 @@ object Checking {
 
       case FieldReturn(pot1, sym) =>
         pot1 match {
-          case thisRef @ ThisRef(cls) =>
-            assert(cls == state.thisClass, "unexpected potential " + pot.show)
-
-            val target = resolve(cls, sym)
+          case thisRef: ThisRef =>
+            val target = resolve(state.thisClass, sym)
             if (sym.isInternal) (thisRef.potentialsOf(target), Effects.empty)
             else (Cold()(pot.source).toPots, Effects.empty)
 
-          case SuperRef(thisRef @ ThisRef(cls), supercls) =>
-            assert(cls == state.thisClass, "unexpected potential " + pot.show)
-
-            val target = resolveSuper(cls, supercls, sym)
+          case SuperRef(thisRef: ThisRef, supercls) =>
+            val target = resolveSuper(state.thisClass, supercls, sym)
             if (target.isInternal) (thisRef.potentialsOf(target), Effects.empty)
             else (Cold()(pot.source).toPots, Effects.empty)
 
@@ -347,19 +332,15 @@ object Checking {
 
       case Outer(pot1, cls) =>
         pot1 match {
-          case ThisRef(cls) =>
-            assert(cls == state.thisClass, "unexpected potential " + pot.show)
-
+          case _: ThisRef =>
+            // all outers for `this` are assumed to be hot
             Summary.empty
 
           case _: Fun =>
             throw new Exception("Unexpected code reached")
 
           case warm: Warm =>
-            (warm.outerFor(cls), Effects.empty)
-
-          case _: Cold =>
-            throw new Exception("Unexpected code reached")
+            (warm.resolveOuter(cls), Effects.empty)
 
           case _ =>
             val (pots, effs) = expand(pot1)

compiler/src/dotty/tools/dotc/transform/init/Effects.scala

@@ -18,10 +18,12 @@ object Effects {
   def show(effs: Effects)(using Context): String =
     effs.map(_.show).mkString(", ")
 
-  /** Effects that are related to safe initialization */
+  /** Effects that are related to safe initialization performed on potentials */
   sealed trait Effect {
-    def size: Int
+    def potential: Potential
+
     def show(using Context): String
+
     def source: Tree
   }
 
@@ -37,48 +39,42 @@ object Effects {
    *  - the selection chain on a potential is too long
    */
   case class Promote(potential: Potential)(val source: Tree) extends Effect {
-    def size: Int = potential.size
-    def show(using Context): String =
-      potential.show + "↑"
+    def show(using Context): String = potential.show + "↑"
   }
 
   /** Field access, `a.f` */
   case class FieldAccess(potential: Potential, field: Symbol)(val source: Tree) extends Effect {
     assert(field != NoSymbol)
 
-    def size: Int = potential.size
-    def show(using Context): String =
-      potential.show + "." + field.name.show + "!"
+    def show(using Context): String = potential.show + "." + field.name.show + "!"
   }
 
   /** Method call, `a.m()` */
   case class MethodCall(potential: Potential, method: Symbol)(val source: Tree) extends Effect {
     assert(method != NoSymbol)
 
-    def size: Int = potential.size
     def show(using Context): String = potential.show + "." + method.name.show + "!"
   }
 
   // ------------------ operations on effects ------------------
 
   extension (eff: Effect) def toEffs: Effects = Effects.empty + eff
 
-  def asSeenFrom(eff: Effect, thisValue: Potential, currentClass: ClassSymbol, outer: Potentials)(implicit env: Env): Effects =
-    trace(eff.show + " asSeenFrom " + thisValue.show + ", current = " + currentClass.show + ", outer = " + Potentials.show(outer), init, effs => show(effs.asInstanceOf[Effects])) { eff match {
+  def asSeenFrom(eff: Effect, thisValue: Potential)(implicit env: Env): Effect =
+    trace(eff.show + " asSeenFrom " + thisValue.show + ", current = " + currentClass.show, init, effs => show(effs.asInstanceOf[Effects])) { eff match {
       case Promote(pot) =>
-        Potentials.asSeenFrom(pot, thisValue, currentClass, outer).promote(eff.source)
+        val pot1 = Potentials.asSeenFrom(pot, thisValue)
+        Promote(pot1)(eff.source)
 
       case FieldAccess(pot, field) =>
-        Potentials.asSeenFrom(pot, thisValue, currentClass, outer).map { pot =>
-          FieldAccess(pot, field)(eff.source)
-        }
+        val pot1 = Potentials.asSeenFrom(pot, thisValue)
+        FieldAccess(pot1, field)(eff.source)
 
       case MethodCall(pot, sym) =>
-        Potentials.asSeenFrom(pot, thisValue, currentClass, outer).map { pot =>
-          MethodCall(pot, sym)(eff.source)
-        }
+        val pot1 = Potentials.asSeenFrom(pot, thisValue)
+        MethodCall(pot1, sym)(eff.source)
     } }
 
-  def asSeenFrom(effs: Effects, thisValue: Potential, currentClass: ClassSymbol, outer: Potentials)(implicit env: Env): Effects =
-    effs.flatMap(asSeenFrom(_, thisValue, currentClass, outer))
+  def asSeenFrom(effs: Effects, thisValue: Potential)(implicit env: Env): Effects =
+    effs.map(asSeenFrom(_, thisValue))
 }

compiler/src/dotty/tools/dotc/transform/init/Env.scala

@@ -20,7 +20,7 @@ import Effects._, Potentials._, Summary._
 
 implicit def theCtx(implicit env: Env): Context = env.ctx
 
-case class Env(ctx: Context, summaryCache: mutable.Map[ClassSymbol, ClassSummary]) {
+case class Env(ctx: Context) {
   private implicit def self: Env = this
 
   // Methods that should be ignored in the checking
@@ -46,12 +46,25 @@ case class Env(ctx: Context, summaryCache: mutable.Map[ClassSymbol, ClassSummary
     sym.isPrimitiveValueClass || sym == defn.StringClass
   }
 
-  /** Summary of a method or field */
+  /** Summary of a class */
+  private val summaryCache = mutable.Map.empty[ClassSymbol, ClassSummary]
   def summaryOf(cls: ClassSymbol): ClassSummary =
     if (summaryCache.contains(cls)) summaryCache(cls)
     else trace("summary for " + cls.show, init, s => s.asInstanceOf[ClassSummary].show) {
       val summary = Summarization.classSummary(cls)
       summaryCache(cls) = summary
       summary
     }
+
+  /** Cache for outer this */
+  private case class OuterKey(warm: Warm, cls: ClassSymbol)
+  private val outerCache: mutable.Map[OuterKey, Potentials] = mutable.Map.empty
+  def resolveOuter(warm: Warm, cls: ClassSymbol)(implicit env: Env): Potentials =
+    val key = OuterKey(warm, cls)
+    if (outerCache.contains(key)) outerCache(key)
+    else {
+      val pots = Potentials.resolveOuter(warm.classSymbol, warm.outer.toPots, cls)
+      outerCache(key) = pots
+      pots
+    }
 }