Merge pull request #12867 from EnzeXing/non-hot-local-val

Allowing local variables to be initialized with non-hot values in initialization checker

Author: liufengyun

compiler/src/dotty/tools/dotc/Compiler.scala

@@ -64,8 +64,8 @@ class Compiler {
          new CheckStatic,            // Check restrictions that apply to @static members
          new BetaReduce,             // Reduce closure applications
          new InlineVals,             // Check right hand-sides of an `inline val`s
-         new ExpandSAMs,             // Expand single abstract method closures to anonymous classes
-         new init.Checker) ::        // Check initialization of objects
+         new ExpandSAMs) ::          // Expand single abstract method closures to anonymous classes
+    List(new init.Checker) ::        // Check initialization of objects
     List(new ElimRepeated,           // Rewrite vararg parameters and arguments
          new ProtectedAccessors,     // Add accessors for protected members
          new ExtensionMethods,       // Expand methods of value classes with extension methods

compiler/src/dotty/tools/dotc/core/Phases.scala

@@ -290,7 +290,7 @@ object Phases {
     /** If set, implicit search is enabled */
     def allowsImplicitSearch: Boolean = false
 
-     /** List of names of phases that should precede this phase */
+    /** List of names of phases that should precede this phase */
     def runsAfter: Set[String] = Set.empty
 
     /** @pre `isRunnable` returns true */

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

@@ -10,15 +10,16 @@ import dotty.tools.dotc.core._
 import Contexts._
 import Types._
 import Symbols._
+import StdNames._
 
 import dotty.tools.dotc.transform._
-import MegaPhase._
+import Phases._
 
 
 import scala.collection.mutable
 
 
-class Checker extends MiniPhase {
+class Checker extends Phase {
   import tpd._
 
   val phaseName = "initChecker"
@@ -30,9 +31,34 @@ class Checker extends MiniPhase {
   override def isEnabled(using Context): Boolean =
     super.isEnabled && ctx.settings.YcheckInit.value
 
-  override def transformTypeDef(tree: TypeDef)(using Context): tpd.Tree = {
-    if (!tree.isClassDef) return tree
+  override def runOn(units: List[CompilationUnit])(using Context): List[CompilationUnit] =
+    units.foreach { unit => traverser.traverse(unit.tpdTree) }
+    super.runOn(units)
+
+  val traverser = new TreeTraverser {
+    override def traverse(tree: Tree)(using Context): Unit =
+      traverseChildren(tree)
+      tree match {
+        case tdef: MemberDef =>
+          // self-type annotation ValDef has no symbol
+          if tdef.name != nme.WILDCARD then
+            tdef.symbol.defTree = tree
+        case _ =>
+      }
+  }
+
+  override def run(using Context): Unit = {
+    val unit = ctx.compilationUnit
+    unit.tpdTree.foreachSubTree {
+      case tdef: TypeDef if tdef.isClassDef =>
+        transformTypeDef(tdef)
+
+      case _ =>
+    }
+  }
+
 
+  private def transformTypeDef(tree: TypeDef)(using Context): tpd.Tree = {
     val cls = tree.symbol.asClass
     val instantiable: Boolean =
       cls.is(Flags.Module) ||

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

@@ -69,7 +69,7 @@ object Errors {
 
   /** Promote `this` under initialization to fully-initialized */
   case class PromoteError(msg: String, source: Tree, trace: Seq[Tree]) extends Error {
-    def show(using Context): String = "Promote the value under initialization to fully-initialized. " + msg + "."
+    def show(using Context): String = "Cannot prove that the value is fully initialized. " + msg + "."
   }
 
   case class AccessCold(field: Symbol, source: Tree, trace: Seq[Tree]) extends Error {

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

@@ -7,6 +7,7 @@ import Contexts._
 import Symbols._
 import Types._
 import StdNames._
+import NameKinds.OuterSelectName
 
 import ast.tpd._
 import util.EqHashMap
@@ -27,24 +28,20 @@ class Semantic {
    *  Value = Hot | Cold | Warm | ThisRef | Fun | RefSet
    *
    *                 Cold
-   *        ┌──────►  ▲   ◄──┐  ◄────┐
-   *        │         │      │       │
-   *        │         │      │       │
-   *   ThisRef(C)     │      │       │
-   *        ▲         │      │       │
-   *        │     Warm(D)   Fun    RefSet
-   *        │         ▲      ▲       ▲
-   *        │         │      │       │
-   *      Warm(C)     │      │       │
-   *        ▲         │      │       │
-   *        │         │      │       │
-   *        └─────────┴──────┴───────┘
+   *        ┌──────►  ▲  ◄────┐  ◄────┐
+   *        │         │       │       │
+   *        │         │       │       │
+   *        |         │       │       │
+   *        |         │       │       │
+   *   ThisRef(C)  Warm(D)   Fun    RefSet
+   *        │         ▲       ▲       ▲
+   *        │         │       │       │
+   *        |         │       │       │
+   *        ▲         │       │       │
+   *        │         │       │       │
+   *        └─────────┴───────┴───────┘
    *                  Hot
    *
-   *   The most important ordering is the following:
-   *
-   *       Hot ⊑ Warm(C) ⊑ ThisRef(C) ⊑ Cold
-   *
    *   The diagram above does not reflect relationship between `RefSet`
    *   and other values. `RefSet` represents a set of values which could
    *   be `ThisRef`, `Warm` or `Fun`. The following ordering applies for
@@ -194,6 +191,7 @@ class Semantic {
     class PromotionInfo {
       var isCurrentObjectPromoted: Boolean = false
       val values = mutable.Set.empty[Value]
+      override def toString(): String = values.toString()
     }
     /** Values that have been safely promoted */
     opaque type Promoted = PromotionInfo
@@ -300,9 +298,6 @@ class Semantic {
       case (Cold, _) => Cold
       case (_, Cold) => Cold
 
-      case (a: Warm, b: ThisRef) if a.klass == b.klass => b
-      case (a: ThisRef, b: Warm) if a.klass == b.klass => a
-
       case (a: (Fun | Warm | ThisRef), b: (Fun | Warm | ThisRef)) => RefSet(a :: b :: Nil)
 
       case (a: (Fun | Warm | ThisRef), RefSet(refs))    => RefSet(a :: refs)
@@ -495,6 +490,46 @@ class Semantic {
           Result(value2, errors)
       }
     }
+
+    def accessLocal(tmref: TermRef, klass: ClassSymbol, source: Tree): Contextual[Result] =
+      val sym = tmref.symbol
+
+      def default() = Result(Hot, Nil)
+
+      if sym.is(Flags.Param) && sym.owner.isConstructor then
+        // instances of local classes inside secondary constructors cannot
+        // reach here, as those values are abstracted by Cold instead of Warm.
+        // This enables us to simplify the domain without sacrificing
+        // expressiveness nor soundess, as local classes inside secondary
+        // constructors are uncommon.
+        if sym.isContainedIn(klass) then
+          Result(env.lookup(sym), Nil)
+        else
+          // We don't know much about secondary constructor parameters in outer scope.
+          // It's always safe to approximate them with `Cold`.
+          Result(Cold, Nil)
+      else if sym.is(Flags.Param) then
+        default()
+      else
+        sym.defTree match {
+          case vdef: ValDef =>
+            // resolve this for local variable
+            val enclosingClass = sym.owner.enclosingClass.asClass
+            val thisValue2 = resolveThis(enclosingClass, value, klass, source)
+            thisValue2 match {
+              case Hot => Result(Hot, Errors.empty)
+
+              case Cold => Result(Cold, Nil)
+
+              case addr: Addr => eval(vdef.rhs, addr, klass)
+
+              case _ =>
+                 report.error("unexpected defTree when accessing local variable, sym = " + sym.show + ", defTree = " + sym.defTree.show, source)
+                 default()
+            }
+
+          case _ => default()
+        }
   end extension
 
 // ----- Promotion ----------------------------------------------------
@@ -756,7 +791,15 @@ class Semantic {
             res.call(id.symbol, args, superType = NoType, source = expr)
 
       case Select(qualifier, name) =>
-        eval(qualifier, thisV, klass).select(expr.symbol, expr)
+        val qualRes = eval(qualifier, thisV, klass)
+
+        name match
+          case OuterSelectName(_, hops) =>
+            val SkolemType(tp) = expr.tpe
+            val outer = resolveOuterSelect(tp.classSymbol.asClass, qualRes.value, hops, source = expr)
+            Result(outer, qualRes.errors)
+          case _ =>
+            qualRes.select(expr.symbol, expr)
 
       case _: This =>
         cases(expr.tpe, thisV, klass, expr)
@@ -846,7 +889,7 @@ class Semantic {
       case vdef : ValDef =>
         // local val definition
         // TODO: support explicit @cold annotation for local definitions
-        eval(vdef.rhs, thisV, klass).ensureHot("Local definitions may only hold initialized values", vdef)
+        eval(vdef.rhs, thisV, klass, cacheResult = true)
 
       case ddef : DefDef =>
         // local method
@@ -874,24 +917,7 @@ class Semantic {
         Result(Hot, Errors.empty)
 
       case tmref: TermRef if tmref.prefix == NoPrefix =>
-        val sym = tmref.symbol
-
-        def default() = Result(Hot, Nil)
-
-        if sym.is(Flags.Param) && sym.owner.isConstructor then
-          // instances of local classes inside secondary constructors cannot
-          // reach here, as those values are abstracted by Cold instead of Warm.
-          // This enables us to simplify the domain without sacrificing
-          // expressiveness nor soundess, as local classes inside secondary
-          // constructors are uncommon.
-          if sym.isContainedIn(klass) then
-            Result(env.lookup(sym), Nil)
-          else
-            // We don't know much about secondary constructor parameters in outer scope.
-            // It's always safe to approximate them with `Cold`.
-            Result(Cold, Nil)
-        else
-          default()
+        thisV.accessLocal(tmref, klass, source)
 
       case tmref: TermRef =>
         cases(tmref.prefix, thisV, klass, source).select(tmref.symbol, source)
@@ -939,6 +965,40 @@ class Semantic {
 
   }
 
+  /** Resolve outer select introduced during inlining.
+   *
+   *  See `tpd.outerSelect` and `ElimOuterSelect`.
+   */
+  def resolveOuterSelect(target: ClassSymbol, thisV: Value, hops: Int, source: Tree): Contextual[Value] = log("resolving outer " + target.show + ", this = " + thisV.show + ", hops = " + hops, printer, res => res.asInstanceOf[Value].show) {
+    // Is `target` reachable from `cls` with the given `hops`?
+    def reachable(cls: ClassSymbol, hops: Int): Boolean =
+      if hops == 0 then cls == target
+      else reachable(cls.lexicallyEnclosingClass.asClass, hops - 1)
+
+    thisV match
+      case Hot => Hot
+
+      case addr: Addr =>
+        val obj = heap(addr)
+        val curOpt = obj.klass.baseClasses.find(cls => reachable(cls, hops))
+        curOpt match
+          case Some(cur) =>
+            resolveThis(target, thisV, cur, source)
+
+          case None =>
+            report.warning("unexpected outerSelect, thisV = " + thisV + ", target = " + target.show + ", hops = " + hops, source.srcPos)
+            Cold
+
+      case RefSet(refs) =>
+        refs.map(ref => resolveOuterSelect(target, ref, hops, source)).join
+
+      case fun: Fun =>
+        report.warning("unexpected thisV = " + thisV + ", target = " + target.show + ", hops = " + hops, source.srcPos)
+        Cold
+
+      case Cold => Cold
+  }
+
   /** Compute the outer value that correspond to `tref.prefix` */
   def outerValue(tref: TypeRef, thisV: Addr, klass: ClassSymbol, source: Tree): Contextual[Result] =
     val cls = tref.classSymbol.asClass

tests/init/crash/i8892.scala

@@ -0,0 +1,28 @@
+trait Reporter:
+  def report(m: String): Unit
+
+class Dummy extends Reporter:
+  def report(m: String) = ()
+
+  object ABug {
+    sealed trait Nat {
+      transparent inline def ++ : Succ[this.type] = Succ(this)
+
+      transparent inline def +(inline that: Nat): Nat =
+        inline this match {
+          case Zero    => that
+          case Succ(p) => p + that.++
+      }
+    }
+
+    case object Zero extends Nat
+    case class Succ[N <: Nat](p: N) extends Nat
+
+    transparent inline def toIntg(inline n: Nat): Int =
+      inline n match {
+        case Zero    => 0
+        case Succ(p) => toIntg(p) + 1
+      }
+
+    val j31 = toIntg(Zero.++.++.++ + Zero.++)
+  }

tests/init/neg/Desugar.check

@@ -1,7 +1,7 @@
 -- Error: tests/init/neg/closureLeak.scala:11:14 -----------------------------------------------------------------------
 11 |    l.foreach(a => a.addX(this)) // error
    |              ^^^^^^^^^^^^^^^^^
-   |       Cannot prove that the value is fully-initialized. May only use initialized value as arguments.
+   |              Cannot prove that the value is fully-initialized. May only use initialized value as arguments.
    |
-   |       The unsafe promotion may cause the following problem:
-   |       Promote the value under initialization to fully-initialized. May only use initialized value as arguments.
+   |              The unsafe promotion may cause the following problem:
+   |              Cannot prove that the value is fully initialized. May only use initialized value as arguments.

tests/init/neg/InteractiveDriver.check

@@ -1,8 +1,8 @@
 -- Error: tests/init/neg/cycle-structure.scala:3:14 --------------------------------------------------------------------
 3 |    val x = B(this) // error
   |              ^^^^
-  |         Promote the value under initialization to fully-initialized. May only use initialized value as arguments.
+  |              Cannot prove that the value is fully initialized. May only use initialized value as arguments.
 -- Error: tests/init/neg/cycle-structure.scala:9:14 --------------------------------------------------------------------
 9 |    val x = A(this) // error
   |              ^^^^
-  |         Promote the value under initialization to fully-initialized. May only use initialized value as arguments.
+  |              Cannot prove that the value is fully initialized. May only use initialized value as arguments.