SI-8708 Fix pickling of LOCAL_CHILD child of sealed classes

When a sealed class or trait has local children, they are not pickled
in as part of the children of the symbol (introduced in 12a2b3b to fix
Aladdin bug 1055). Instead the compiler adds a single child class
named LOCAL_CHILD. The parents of its ClassInfoType were wrong: the
first parent should be a class. For sealed traits, we were using the
trait itself.

Also, the LOCAL_CHILD dummy class was entered as a member of its
enclosing class, which is wrong: it represents a local (non-member)
class, and it's a synthetic dummy anyway.

11 files changed, 122 insertions, 6 deletions

diff --git a/src/compiler/scala/tools/nsc/symtab/classfile/Pickler.scala b/src/compiler/scala/tools/nsc/symtab/classfile/Pickler.scala
index 592c5497b5..34c5a899b7 100644
--- a/src/compiler/scala/tools/nsc/symtab/classfile/Pickler.scala
+++ b/src/compiler/scala/tools/nsc/symtab/classfile/Pickler.scala
@@ -186,7 +186,16 @@ abstract class Pickler extends SubComponent {
             val (locals, globals) = sym.children partition (_.isLocalClass)
             val children =
               if (locals.isEmpty) globals
-              else globals + sym.newClassWithInfo(tpnme.LOCAL_CHILD, List(sym.tpe), EmptyScope, pos = sym.pos)
+              else {
+                // The LOCAL_CHILD was introduced in 12a2b3b to fix Aladdin bug 1055. When a sealed
+                // class/trait has local subclasses, a single <local child> class symbol is added
+                // as pickled child (instead of a reference to the anonymous class; that was done
+                // initially, but seems not to work, as the bug shows).
+                // Adding the LOCAL_CHILD is necessary to retain exhaustivity warnings under separate
+                // compilation. See test neg/aladdin1055.
+                val parents = (if (sym.isTrait) List(definitions.ObjectTpe) else Nil) ::: List(sym.tpe)
+                globals + sym.newClassWithInfo(tpnme.LOCAL_CHILD, parents, EmptyScope, pos = sym.pos)
+              }
 
             putChildren(sym, children.toList sortBy (_.sealedSortName))
           }
diff --git a/src/reflect/scala/reflect/internal/pickling/UnPickler.scala b/src/reflect/scala/reflect/internal/pickling/UnPickler.scala
index 64a1a44722..b808666360 100644
--- a/src/reflect/scala/reflect/internal/pickling/UnPickler.scala
+++ b/src/reflect/scala/reflect/internal/pickling/UnPickler.scala
@@ -290,6 +290,25 @@ abstract class UnPickler {
       def pflags       = flags & PickledFlags
 
       def finishSym(sym: Symbol): Symbol = {
+        /**
+         * member symbols (symbols owned by a class) are added to the class's scope, with a number
+         * of exceptions:
+         *
+         * (.) ...
+         * (1) `local child` represents local child classes, see comment in Pickler.putSymbol.
+         *     Since it is not a member, it should not be entered in the owner's scope.
+         */
+        def shouldEnterInOwnerScope = {
+          sym.owner.isClass &&
+            sym != classRoot &&
+            sym != moduleRoot &&
+            !sym.isModuleClass &&
+            !sym.isRefinementClass &&
+            !sym.isTypeParameter &&
+            !sym.isExistentiallyBound &&
+            sym.rawname != tpnme.LOCAL_CHILD // (1)
+        }
+
         markFlagsCompleted(sym)(mask = AllFlags)
         sym.privateWithin = privateWithin
         sym.info = (
@@ -302,8 +321,7 @@ abstract class UnPickler {
             newLazyTypeRefAndAlias(inforef, readNat())
           }
         )
-        if (sym.owner.isClass && sym != classRoot && sym != moduleRoot &&
-            !sym.isModuleClass && !sym.isRefinementClass && !sym.isTypeParameter && !sym.isExistentiallyBound)
+        if (shouldEnterInOwnerScope)
           symScope(sym.owner) enter sym
 
         sym
@@ -681,10 +699,24 @@ abstract class UnPickler {
       private val p = phase
       protected def completeInternal(sym: Symbol) : Unit = try {
         val tp = at(i, () => readType(sym.isTerm)) // after NMT_TRANSITION, revert `() => readType(sym.isTerm)` to `readType`
-        if (p ne null)
-          slowButSafeEnteringPhase(p) (sym setInfo tp)
+
+        // This is a temporary fix allowing to read classes generated by an older, buggy pickler.
+        // See the generation of the LOCAL_CHILD class in Pickler.scala. In an earlier version, the
+        // pickler did not add the ObjectTpe superclass, it used a trait as the first parent. This
+        // tripped an assertion in AddInterfaces which checks that the first parent is not a trait.
+        // This workaround can probably be removed in 2.12, because the 2.12 compiler is supposed
+        // to only read classfiles generated by 2.12.
+        val fixLocalChildTp = if (sym.rawname == tpnme.LOCAL_CHILD) tp match {
+            case ClassInfoType(superClass :: traits, decls, typeSymbol) if superClass.typeSymbol.isTrait =>
+              ClassInfoType(definitions.ObjectTpe :: superClass :: traits, decls, typeSymbol)
+            case _ => tp
+          } else tp
+
+        if (p ne null) {
+          slowButSafeEnteringPhase(p)(sym setInfo fixLocalChildTp)
+        }
         if (currentRunId != definedAtRunId)
-          sym.setInfo(adaptToNewRunMap(tp))
+          sym.setInfo(adaptToNewRunMap(fixLocalChildTp))
       }
       catch {
         case e: MissingRequirementError => throw toTypeError(e)
diff --git a/test/files/neg/aladdin1055.check b/test/files/neg/aladdin1055.check
new file mode 100644
index 0000000000..41782ae987
--- /dev/null
+++ b/test/files/neg/aladdin1055.check
@@ -0,0 +1,7 @@
+Test_1.scala:2: warning: match may not be exhaustive.
+It would fail on the following input: (_ : this.<local child>)
+  def foo(t: A.T) = t match {
+                    ^
+error: No warnings can be incurred under -Xfatal-warnings.
+one warning found
+one error found
diff --git a/test/files/neg/aladdin1055.flags b/test/files/neg/aladdin1055.flags
new file mode 100644
index 0000000000..e8fb65d50c
--- /dev/null
+++ b/test/files/neg/aladdin1055.flags
@@ -0,0 +1 @@
+-Xfatal-warnings
\ No newline at end of file
diff --git a/test/files/neg/aladdin1055/A.scala b/test/files/neg/aladdin1055/A.scala
new file mode 100644
index 0000000000..862336e30c
--- /dev/null
+++ b/test/files/neg/aladdin1055/A.scala
@@ -0,0 +1,6 @@
+object A {
+  sealed trait T { def f: Int }
+  class TT extends T { def f = 0 }
+
+  def foo = new T { def f = 1 } // local subclass of sealed trait T
+}
diff --git a/test/files/neg/aladdin1055/Test_1.scala b/test/files/neg/aladdin1055/Test_1.scala
new file mode 100644
index 0000000000..39d9b1dc98
--- /dev/null
+++ b/test/files/neg/aladdin1055/Test_1.scala
@@ -0,0 +1,5 @@
+object Test {
+  def foo(t: A.T) = t match {
+    case a: A.TT => 0
+  }
+}
diff --git a/test/files/pos/t8708/Either_1.scala b/test/files/pos/t8708/Either_1.scala
new file mode 100644
index 0000000000..000ed6e7c2
--- /dev/null
+++ b/test/files/pos/t8708/Either_1.scala
@@ -0,0 +1,6 @@
+sealed trait \/[+A, +B]
+
+sealed trait EitherT[F[+_], +A, +B]
+object EitherT {
+  def apply[F[+_], A, B](a: F[A \/ B]): EitherT[F, A, B] = new EitherT[F, A, B] { val run = a }
+}
diff --git a/test/files/pos/t8708/Test_2.scala b/test/files/pos/t8708/Test_2.scala
new file mode 100644
index 0000000000..d0e56b9a37
--- /dev/null
+++ b/test/files/pos/t8708/Test_2.scala
@@ -0,0 +1,13 @@
+import scala.language.higherKinds
+
+trait ClientTypes[M[+_]] {
+  final type Context[+A] = EitherT[M, String, A]
+  object Context {
+    def apply[A](ca: M[String \/ A]): Context[A] = EitherT[M, String, A](ca)
+  }
+
+  final type StatefulContext[+A] = EitherT[Context, String, A]
+  object StatefulContext {
+    def apply[A](state: Context[String \/ A]): StatefulContext[A] = ???
+  }
+}
diff --git a/test/files/run/t8708_b.check b/test/files/run/t8708_b.check
new file mode 100644
index 0000000000..30be62a307
--- /dev/null
+++ b/test/files/run/t8708_b.check
@@ -0,0 +1,8 @@
+Scope{
+  def <init>: <?>;
+  sealed abstract trait T extends ;
+  def foo: <?>
+}
+Scope{
+  def f: <?>
+}
diff --git a/test/files/run/t8708_b/A_1.scala b/test/files/run/t8708_b/A_1.scala
new file mode 100644
index 0000000000..e767420f9e
--- /dev/null
+++ b/test/files/run/t8708_b/A_1.scala
@@ -0,0 +1,8 @@
+package p
+
+class C {
+
+  sealed trait T { def f: Int }
+
+  def foo: T = new T { def f = 1 }
+}
diff --git a/test/files/run/t8708_b/Test_2.scala b/test/files/run/t8708_b/Test_2.scala
new file mode 100644
index 0000000000..c978490609
--- /dev/null
+++ b/test/files/run/t8708_b/Test_2.scala
@@ -0,0 +1,21 @@
+import scala.tools.partest._
+import java.io.{Console => _, _}
+
+object Test extends DirectTest {
+
+  override def extraSettings: String = "-usejavacp -cp " + testOutput.path
+
+  override def code = ""
+
+  override def show(): Unit = {
+    val g = newCompiler()
+    withRun(g)(r => {
+      val c = g.rootMirror.getRequiredClass("p.C")
+      println(c.info.decls)
+      val t = c.info.member(g.newTypeName("T"))
+      // this test ensrues that the <local child> dummy class symbol is not entered in the
+      // scope of trait T during unpickling.
+      println(t.info.decls)
+    })
+  }
+}