Merge pull request #3202 from dotty-staging/fix-#3149

Fix #3149: Fix pickling of child annotations to local classes

Author: odersky

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

@@ -142,15 +142,28 @@ object Annotations {
       apply(defn.AliasAnnot, List(
         ref(TermRef(sym.owner.thisType, sym.name, sym))))
 
-    def makeChild(delayedSym: Context => Symbol)(implicit ctx: Context): Annotation = {
-      def makeChildLater(implicit ctx: Context) = {
-        val sym = delayedSym(ctx)
-        New(defn.ChildAnnotType.appliedTo(sym.owner.thisType.select(sym.name, sym)), Nil)
+    /** Extractor for child annotations */
+    object Child {
+
+      /** A deferred annotation to the result of a given child computation */
+      def apply(delayedSym: Context => Symbol)(implicit ctx: Context): Annotation = {
+        def makeChildLater(implicit ctx: Context) = {
+          val sym = delayedSym(ctx)
+          New(defn.ChildAnnotType.appliedTo(sym.owner.thisType.select(sym.name, sym)), Nil)
+        }
+        deferred(defn.ChildAnnot, implicit ctx => makeChildLater(ctx))
       }
-      deferred(defn.ChildAnnot, implicit ctx => makeChildLater(ctx))
-    }
 
-    def makeChild(sym: Symbol)(implicit ctx: Context): Annotation = makeChild(_ => sym)
+      /** A regular, non-deferred Child annotation */
+      def apply(sym: Symbol)(implicit ctx: Context): Annotation = apply(_ => sym)
+
+      def unapply(ann: Annotation)(implicit ctx: Context): Option[Symbol] =
+        if (ann.symbol == defn.ChildAnnot) {
+          val AppliedType(tycon, (arg: NamedType) :: Nil) = ann.tree.tpe
+          Some(arg.symbol)
+        }
+        else None
+    }
 
     def makeSourceFile(path: String)(implicit ctx: Context) =
       apply(defn.SourceFileAnnot, Literal(Constant(path)))

compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala

@@ -258,7 +258,7 @@ class ClassfileParser(
             ctx.warning(s"no linked class for java enum $sym in ${sym.owner}. A referencing class file might be missing an InnerClasses entry.")
           else {
             if (!(enumClass is Flags.Sealed)) enumClass.setFlag(Flags.AbstractSealed)
-            enumClass.addAnnotation(Annotation.makeChild(sym))
+            enumClass.addAnnotation(Annotation.Child(sym))
           }
         }
       } finally {

compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala

@@ -570,11 +570,21 @@ class TreePickler(pickler: TastyPickler) {
       if (flags is Covariant) writeByte(COVARIANT)
       if (flags is Contravariant) writeByte(CONTRAVARIANT)
     }
-    sym.annotations.foreach(pickleAnnotation)
+    sym.annotations.foreach(pickleAnnotation(sym, _))
   }
 
-  def pickleAnnotation(ann: Annotation)(implicit ctx: Context) =
-    if (ann.symbol != defn.BodyAnnot) { // inline bodies are reconstituted automatically when unpickling
+  private def isUnpicklable(owner: Symbol, ann: Annotation)(implicit ctx: Context) = ann match {
+    case Annotation.Child(sym) => sym.isInaccessibleChildOf(owner)
+      // If child annotation refers to a local class or enum value under
+      // a different toplevel class, it is impossible to pickle a reference to it.
+      // Such annotations will be reconstituted when unpickling the child class.
+      // See tests/pickling/i3149.scala
+    case _ => ann.symbol == defn.BodyAnnot
+      // inline bodies are reconstituted automatically when unpickling
+  }
+
+  def pickleAnnotation(owner: Symbol, ann: Annotation)(implicit ctx: Context) =
+    if (!isUnpicklable(owner, ann)) {
       writeByte(ANNOTATION)
       withLength { pickleType(ann.symbol.typeRef); pickleTree(ann.tree) }
     }

compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala

@@ -11,6 +11,7 @@ import util.Positions._
 import ast.{tpd, Trees, untpd}
 import Trees._
 import Decorators._
+import transform.SymUtils._
 import TastyUnpickler._, TastyBuffer._
 import scala.annotation.{tailrec, switch}
 import scala.collection.mutable.ListBuffer
@@ -663,7 +664,6 @@ class TreeUnpickler(reader: TastyReader, nameAtRef: NameRef => TermName, posUnpi
             def isCodefined =
               roots.contains(companion.denot) == seenRoots.contains(companion)
             if (companion.exists && isCodefined) {
-              import transform.SymUtils._
               if (sym is Flags.ModuleClass) sym.registerCompanionMethod(nme.COMPANION_CLASS_METHOD, companion)
               else sym.registerCompanionMethod(nme.COMPANION_MODULE_METHOD, companion)
             }
@@ -702,6 +702,10 @@ class TreeUnpickler(reader: TastyReader, nameAtRef: NameRef => TermName, posUnpi
       if (!sym.isType) { // Only terms might have leaky aliases, see the documentation of `checkNoPrivateLeaks`
         sym.info = ta.avoidPrivateLeaks(sym, tree.pos)
       }
+      if ((sym.isClass || sym.is(CaseVal)) && sym.isLocal)
+        // Child annotations for local classes and enum values are not pickled, so
+        // need to be re-established here.
+        sym.registerIfChild(late = true)
       tree
     }
 

compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala

@@ -838,7 +838,7 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas
       val start = readIndex
       readNat() // skip reference for now
       target.addAnnotation(
-          Annotation.makeChild(implicit ctx =>
+          Annotation.Child(implicit ctx =>
               atReadPos(start, () => readSymbolRef())))
     }
   }

compiler/src/dotty/tools/dotc/transform/PostTyper.scala

@@ -14,7 +14,7 @@ import SymDenotations._, Symbols._, StdNames._, Annotations._, Trees._, Scopes._
 import util.Positions._
 import Decorators._
 import config.Printers.typr
-import Symbols._, TypeUtils._
+import Symbols._, TypeUtils._, SymUtils._
 import reporting.diagnostic.messages.SuperCallsNotAllowedInline
 
 /** A macro transform that runs immediately after typer and that performs the following functions:
@@ -124,14 +124,9 @@ class PostTyper extends MacroTransform with SymTransformer  { thisTransformer =>
     private def transformAnnot(annot: Annotation)(implicit ctx: Context): Annotation =
       annot.derivedAnnotation(transformAnnot(annot.tree))
 
-    private def registerChild(sym: Symbol, tp: Type)(implicit ctx: Context) = {
-      val cls = tp.classSymbol
-      if (cls.is(Sealed)) cls.addAnnotation(Annotation.makeChild(sym))
-    }
-
     private def transformMemberDef(tree: MemberDef)(implicit ctx: Context): Unit = {
       val sym = tree.symbol
-      if (sym.is(CaseVal, butNot = Method | Module)) registerChild(sym, sym.info)
+      sym.registerIfChild()
       sym.transformAnnotations(transformAnnot)
     }
 
@@ -257,14 +252,6 @@ class PostTyper extends MacroTransform with SymTransformer  { thisTransformer =>
               ctx.compilationUnit.source.exists &&
               sym != defn.SourceFileAnnot)
               sym.addAnnotation(Annotation.makeSourceFile(ctx.compilationUnit.source.file.path))
-
-            // Add Child annotation to sealed parents unless current class is anonymous
-            if (!sym.isAnonymousClass) // ignore anonymous class
-              sym.asClass.classParents.foreach { parent =>
-                val sym2 = if (sym.is(Module)) sym.sourceModule else sym
-                registerChild(sym2, parent)
-              }
-
             tree
           }
           super.transform(tree)

compiler/src/dotty/tools/dotc/transform/SymUtils.scala

@@ -134,4 +134,45 @@ class SymUtils(val self: Symbol) extends AnyVal {
       }
     }
   }
+
+  /** If this symbol is an enum value or a named class, register it as a child
+   *  in all direct parent classes which are sealed.
+   *   @param  @late  If true, register only inaccessible children (all others are already
+   *                  entered at this point).
+   */
+  def registerIfChild(late: Boolean = false)(implicit ctx: Context): Unit = {
+    def register(child: Symbol, parent: Type) = {
+      val cls = parent.classSymbol
+      if (cls.is(Sealed) && (!late || child.isInaccessibleChildOf(cls)))
+        cls.addAnnotation(Annotation.Child(child))
+    }
+    if (self.isClass && !self.isAnonymousClass)
+      self.asClass.classParents.foreach { parent =>
+        val child = if (self.is(Module)) self.sourceModule else self
+        register(child, parent)
+      }
+    else if (self.is(CaseVal, butNot = Method | Module))
+      register(self, self.info)
+  }
+
+  /** Is this symbol defined locally (i.e. at some level owned by a term) and
+   *  defined in a different toplevel class than its supposed parent class `cls`?
+   *  Such children are not pickled, and have to be reconstituted manually.
+   */
+  def isInaccessibleChildOf(cls: Symbol)(implicit ctx: Context) =
+    self.isLocal && !cls.topLevelClass.isLinkedWith(self.topLevelClass)
+
+  /** If this is a sealed class, its known children */
+  def children(implicit ctx: Context): List[Symbol] =
+    self.annotations.collect {
+      case Annotation.Child(child) => child
+    }
+
+  /** Is symbol directly or indirectly owned by a term symbol? */
+  @tailrec def isLocal(implicit ctx: Context): Boolean = {
+    val owner = self.owner
+    if (owner.isTerm) true
+    else if (owner.is(Package)) false
+    else owner.isLocal
+  }
 }

compiler/src/dotty/tools/dotc/transform/patmat/Space.scala

@@ -514,12 +514,7 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
 
   /** Decompose a type into subspaces -- assume the type can be decomposed */
   def decompose(tp: Type): List[Space] = {
-    val children = tp.classSymbol.annotations.filter(_.symbol == ctx.definitions.ChildAnnot).map { annot =>
-      // refer to definition of Annotation.makeChild
-      annot.tree match {
-        case Apply(TypeApply(_, List(tpTree)), _) => tpTree.symbol
-      }
-    }
+    val children = tp.classSymbol.children
 
     debug.println(s"candidates for ${tp.show} : [${children.map(_.show).mkString(", ")}]")
 

compiler/src/dotty/tools/dotc/typer/Namer.scala

@@ -470,7 +470,7 @@ class Namer { typer: Typer =>
     case vdef: ValDef if (isEnumConstant(vdef)) =>
       val enumClass = sym.owner.linkedClass
       if (!(enumClass is Flags.Sealed)) enumClass.setFlag(Flags.AbstractSealed)
-      enumClass.addAnnotation(Annotation.makeChild(sym))
+      enumClass.addAnnotation(Annotation.Child(sym))
     case _ =>
   }
 

tests/pickling/i3149.scala

@@ -0,0 +1,7 @@
+sealed class Foo
+
+class Test {
+  def f = {
+    class Bar extends Foo
+  }
+}

tests/pos/i3149.scala

@@ -0,0 +1,14 @@
+sealed class Foo
+
+class Test {
+  def f = {
+    class Bar extends Foo
+  }
+  class C {
+    class Bar extends Foo
+  }
+  object O {
+    class Bar extends Foo
+  }
+}
+

tests/pos/i3149a.scala

@@ -0,0 +1,14 @@
+sealed class Foo
+
+object Foo {
+  def f = {
+    class Bar extends Foo
+  }
+  class C {
+    class Bar extends Foo
+  }
+  object O {
+    class Bar extends Foo
+  }
+}
+