From 14fa7bef120cbb996d042daba6095530167c49ed Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Tue, 1 Jul 2014 16:19:34 +0200 Subject: [PATCH] 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. --- .../tools/nsc/symtab/classfile/Pickler.scala | 11 ++++- .../reflect/internal/pickling/UnPickler.scala | 42 ++++++++++++++++--- test/files/neg/aladdin1055.check | 7 ++++ test/files/neg/aladdin1055.flags | 1 + test/files/neg/aladdin1055/A.scala | 6 +++ test/files/neg/aladdin1055/Test_1.scala | 5 +++ test/files/pos/t8708/Either_1.scala | 6 +++ test/files/pos/t8708/Test_2.scala | 13 ++++++ test/files/run/t8708_b.check | 8 ++++ test/files/run/t8708_b/A_1.scala | 8 ++++ test/files/run/t8708_b/Test_2.scala | 21 ++++++++++ 11 files changed, 122 insertions(+), 6 deletions(-) create mode 100644 test/files/neg/aladdin1055.check create mode 100644 test/files/neg/aladdin1055.flags create mode 100644 test/files/neg/aladdin1055/A.scala create mode 100644 test/files/neg/aladdin1055/Test_1.scala create mode 100644 test/files/pos/t8708/Either_1.scala create mode 100644 test/files/pos/t8708/Test_2.scala create mode 100644 test/files/run/t8708_b.check create mode 100644 test/files/run/t8708_b/A_1.scala create mode 100644 test/files/run/t8708_b/Test_2.scala diff --git a/src/compiler/scala/tools/nsc/symtab/classfile/Pickler.scala b/src/compiler/scala/tools/nsc/symtab/classfile/Pickler.scala index 592c5497b552..34c5a899b74b 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 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 64a1a4472282..b808666360a8 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 000000000000..41782ae987ee --- /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.) + 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 000000000000..e8fb65d50c20 --- /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 000000000000..862336e30cb1 --- /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 000000000000..39d9b1dc980d --- /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 000000000000..000ed6e7c283 --- /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 000000000000..d0e56b9a3743 --- /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 000000000000..30be62a3078e --- /dev/null +++ b/test/files/run/t8708_b.check @@ -0,0 +1,8 @@ +Scope{ + def : ; + 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 000000000000..e767420f9e88 --- /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 000000000000..c9784906090b --- /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 dummy class symbol is not entered in the + // scope of trait T during unpickling. + println(t.info.decls) + }) + } +}